-
Notifications
You must be signed in to change notification settings - Fork 111
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
Implements patch list as a single contiguous array of reusable PatchPoint instances. #521
Conversation
Here are more benchmark results from a different usage pattern: Results from benchmarking a write of data equivalent to a stream of 59155 nested binary Ion values(3 forks, 2 warmups, 2 iterations, preallocation 1).
Results from benchmarking a write of data equivalent to a single nested binary Ion value(3 forks, 2 warmups, 2 iterations, preallocation 1).
Results from benchmarking a write of data equivalent to a stream of 194617 nested binary Ion value(3 forks, 2 warmups, 2 iterations, preallocation 1).
|
6a1cba1
to
a4cb237
Compare
a4cb237
to
973b026
Compare
Codecov ReportAttention:
... and 1 file with indirect coverage changes 📢 Thoughts on this report? Let us know! |
e087fba
to
cb769f5
Compare
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.
I think it would be interesting to do a performance comparison with the recycling queue both with and without auto-flush. I wonder if auto-flush amplifies the benefits of this change by limiting the size of the recycling queue.
Issue #, if available:
N/A
Description of changes:
This PR introduces optimizations to improve memory usage and avoid unnecessary allocations. The following changes are included in this PR:
IonRawBinaryWriter
, we only keep theIonRawBinaryWriter
level patch list (this patch list is composed by the patch list for containers within the current scope)during the writing process. Previously, we needed to append the child container's patch list after the parent container to maintain the accurate sequence of the patch points. With the change, we use a patch point placeholder for the parent container's patch information. If there is a patch point created for the current container, we override the placeholder with the real data.Here are the performance comparison results before and after the change: Benchmark a write of data equivalent to the a stream of stream of 194617 nested binary data using IonWriter(binary). The output data will write into an in-memory buffer.
Note: This implementation is built on top of removing the patch buffer, so we will compare the performance of the library without any changes, with the change of removing the patch buffer and with the change of recycling queue implementation.
Preallocation 0:
No Change:
Patch Buffer Removal:
Recycling Queue:
Preallocation 1:
No change:
Patch Buffer Removal:
Recycling Queue:
Preallocation 2:
No Change:
Patch Buffer Removal:
Recycling Queue:
For more benchmark results generated from other dataset, please visit here.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.