-
Notifications
You must be signed in to change notification settings - Fork 314
wire: Optimize writes to bytes.Buffer #3589
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
84a65f2 to
230fb69
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
158b29e to
a2cbcdf
Compare
|
Now rebased over #3584. Below is the performance improvement with both PRs, relative to master: |
2cda86f to
a182543
Compare
0ea4d52 to
396b860
Compare
davecgh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall. This help further reduce the number of allocations the GC has to deal with.
I've identified a few nits inline.
c3cafff to
d9752dc
Compare
This special cases writes to bytes.Buffer, which is always the writer type written to by WriteMessageN. There are several optimizations that can be implemented by special casing this type: First, pulling temporary short buffers from binary freelist can be skipped entirely, and instead the binary encoding of integers can be appended directly to its existing capacity. This avoids the synchronization cost to add and remove buffers from the free list, and for applications which only ever write wire messages with WriteMessageN, the allocation and ongoing garbage collection scanning cost to for these buffers can be completely skipped. Second, special casing the buffer type in WriteVarString saves us from creating a temporary heap copy of a string, as the buffer's WriteString method can be used instead. Third, special casing the buffer allows WriteMessageN to calculate the serialize size and grow its buffer so all remaining appends for writing block and transactions will not have to reallocate the buffer's backing allocation. This same optimization can be applied to other messages in the future.
d9752dc to
1c43c08
Compare
davecgh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. Looks great.
This special cases writes to bytes.Buffer, which is always the writer type written to by WriteMessageN. There are several optimizations that can be implemented by special casing this type:
First, pulling temporary short buffers from binary freelist can be skipped entirely, and instead the binary encoding of integers can be appended directly to its existing capacity. This avoids the synchronization cost to add and remove buffers from the free list, and for applications which only ever write wire messages with WriteMessageN, the allocation and ongoing garbage collection scanning cost to for these buffers can be completely skipped.
Second, special casing the buffer type in WriteVarString saves us from creating a temporary heap copy of a string, as the buffer's WriteString method can be used instead.
Third, special casing the buffer allows WriteMessageN to calculate the serialize size and grow its buffer so all remaining appends for writing block and transactions will not have to reallocate the buffer's backing allocation. This same optimization can be applied to other messages in the future.