polyfuse icon indicating copy to clipboard operation
polyfuse copied to clipboard

Reuse buffer for request

Open sum12 opened this issue 3 years ago • 5 comments

commit 1 replace unsafe set_len with a new struct SubVec

Here all that is needed is restricted access only the data tha is actually been read. This extra implementation allows the code to go without unsafe code.

commit 2 improve performance by reusing request buffer allocation

A Vec allocated is used read the incoming request from the kernel and stored inside the request. Once the request is serviced this buffer gets dropped. Allocation can be expensive in a heavily loaded systems, doing multiple read and writes and probably concurrently.

Another bottleneck: this allocaiton is done on the main thread which holds the session further hampering response times.

This patch add a new Mutexd VecDeque of Vec<u8> to the SessionInner. Which is shared across the threads through Arc thus there is only one copy of this vector. This VecDeque acts as cache hold pointers the to already allocated buffers, which are no longer in use.

In order the retain the buffer, this patch add a Drop implementation for the Request struct. Once the request is about to be dropped its assigned buffer is moved to session's VecDeque via mem::replace.

Thus each time there is a new request there is check with the session's cache of buffers and if there is none only then allocate.

sum12 avatar Mar 01 '21 20:03 sum12

 ➜  find mnt | wc -l                                                           
19102                                                                         
                                                                              
# with patch                                                                  
➜  time ls mnt/* > /dev/null                                                  
ls --color=tty mnt/* > /dev/null  0.08s user 0.03s system 36% cpu 0.283 total 
                                                                              
# withou patch                                                                
➜  time ls mnt/* > /dev/null                                                  
ls --color=tty mnt/* > /dev/null  0.05s user 0.01s system 13% cpu 0.458 total 

sum12 avatar Mar 01 '21 20:03 sum12

I have no objection to the first commit that avoids the unsafe set_len calls.

For the second commit, I agree with the idea of reusing the buffers for receiving FUSE requests. When rewriting to 0.4.0, I intentionally ignored it for ease of implementation.

However, I disagree with the current implementation that returns automatically the used buffers back to the queue. I would like to able to allow users to add such buffer management mechanisms as needed, rather than incorporating them into Session itself.

ubnt-intrepid avatar Mar 02 '21 05:03 ubnt-intrepid

rough sketch

let session = Arc::new(session);
let buffers = Arc::new(buffers);

// worker thread
loop {
    let mut buf: Vec<u8> = buffers.acquire().unwrap(); // will block this thread
    let request = session.recv_request(&mut buf).unwrap();
    // ...omit
    buffers.send(buf).unwrap();
}

ubnt-intrepid avatar Mar 02 '21 05:03 ubnt-intrepid

okay I will move the second commit to a new PR for now. Thanks for rough sketch, Question: I dont see recv_request on session I guess you mean next_request (which can be modified to take an param ? this will be a breaking change) or should there be a new api to recv_request ?

sum12 avatar Mar 07 '21 08:03 sum12

@ubnt-intrepid any inputs here please ?

sum12 avatar Mar 25 '21 14:03 sum12