THRIFT-5114: Simplified reallocation of TMemoryBuffer #2030
Conversation
|
There's also #2007 ... may I ask to cross-review each others change, resolving any potential interferences? |
The two changes are unrelated. My change is foremost for better readability of the code. My change also introduces a difference in the allocation size of buffer enlargement. The current implementation computes this based on a relatively hard to understand formula. My change always allocates the next larger power of two, so essentially it doubles the buffer whenever the size is exceeded. I can not say that my allocation policy is generally better. But it is hopefully easier to understand and maintain. |
|
I also agree #2007 is unrelated. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. |
|
Getting pushed back again, sorry. |
|
This issue is no longer stale. Thank you for your contributions. |
emmenlau
force-pushed
the
bda_simplify_tmemorybuffer_alloc
branch
from
74e182d to
5d60e81
Compare
|
Pushed an update, and rebased on latest master. |
emmenlau
force-pushed
the
bda_simplify_tmemorybuffer_alloc
branch
from
5d60e81 to
2156a3c
Compare
| } | ||
| avail = available_write() + (static_cast<uint32_t>(new_size) - bufferSize_); | ||
| const uint32_t current_used = bufferSize_ - avail; | ||
| const uint32_t required_buffer_size = len + current_used; |
There was a problem hiding this comment.
It's one of my clean-code-quirks: I use const wherever possible. I think it makes the code more robust, because none of these variables are expected to change in the remainder of the code.
Would you prefer to remove my strong consting? Then I will not continue to use it in Thrift.
There was a problem hiding this comment.
No, my fault, I was under the impression that since avail may change the value can't hardly be const but it is not exactly the way I assumed it is re initialization of the const. So const is fine.
3 participants
This PR makes a change to how the new buffer size of TMemoryBuffer is computed. In the current implementation, the new size of TMemoryBuffer is computed in a loop, which makes it less readable. This PR replaces the computation of the new size with an explicit formula.
[skip ci]at the end of your pull request to free up build resources.