facebook / facebook/folly

AsyncSocket doesn't support movable buffer

Open
#1,467 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
30.5k
Forks
5.9k
PR merge metrics
No merged PRs in 30d

Description

Maybe I am missing something. According to this description from AsyncTransport.h:
```
/**
* When data becomes available, isBufferMovable() will be invoked to figure
* out which API will be used, readBufferAvailable() or
* readDataAvailable(). If isBufferMovable() returns true, that means
* ReadCallback supports the IOBuf ownership transfer and
* readBufferAvailable() will be used. Otherwise, not.
* By default, isBufferMovable() always return false. If
* readBufferAvailable() is implemented and to be invoked, You should
* overwrite isBufferMovable() and return true in the inherited class.
*
* This method allows the AsyncSocket/AsyncSSLSocket do buffer allocation by
* itself until data becomes available. Compared with the pre/post buffer
* allocation in getReadBuffer()/readDataAvailabe(), readBufferAvailable()
* has two advantages. First, this can avoid memcpy. E.g., in
* AsyncSSLSocket, the decrypted data was copied from the openssl internal
* buffer to the readbuf buffer. With the buffer ownership transfer, the
* internal buffer can be directly "moved" to ReadCallback. Second, the
* memory allocation can be more precise. The reason is
* AsyncSocket/AsyncSSLSocket can allocate the memory of precise size
* because they have more context about the available data than
* ReadCallback. Think about the getReadBuffer() pre-allocate 4072 bytes
* buffer, but the available data is always 16KB (max OpenSSL record size).
*/

virtual bool isBufferMovable() noexcept {
return false;
}
```

But even I return true in my ReadCallback object, it never call readBufferAvailable(). Search for the code, it seems that only AsyncPipe supports that. Is that right? I am using v2020.09.07. I also checked the latest version of AsyncSocket, it doesn't call isBufferMovalble() ever.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.