-
Notifications
You must be signed in to change notification settings - Fork 110
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
Don't do alignment computations during BFLATN ser/de #986
Conversation
`AlgebraicTypeLayout` and friends already include full layout information, including properly-aligned offsets for `ProductTypeElementLayout`s. As such, there's no need to do any alignment computation during `serialize_value` or `write_value`. Instead, while traversing a `ProductTypeLayout`, we can use each element's `offset` to update the `curr_offset`.
benchmarks please |
Criterion benchmark resultsCriterion benchmark reportYOU SHOULD PROBABLY IGNORE THESE RESULTS. Criterion is a wall time based benchmarking system that is extremely noisy when run on CI. We collect these results for longitudinal analysis, but they are not reliable for comparing individual PRs. Go look at the callgrind report instead. empty
insert_1
insert_bulk
iterate
find_unique
filter
serialize
stdb_module_large_arguments
stdb_module_print_bulk
remaining
|
Callgrind benchmark resultsCallgrind Benchmark ReportThese benchmarks were run using callgrind, Measurement changes larger than five percent are in bold. In-memory benchmarkscallgrind: empty transaction
callgrind: filter
callgrind: insert bulk
callgrind: iterate
callgrind: serialize_product_value
callgrind: update bulk
On-disk benchmarkscallgrind: empty transaction
callgrind: filter
callgrind: insert bulk
callgrind: iterate
callgrind: serialize_product_value
callgrind: update bulk
|
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.
full-scan time: [59.269 ms 59.444 ms 59.639 ms]
change: [-28.386% -28.129% -27.845%] (p = 0.00 < 0.05)
Performance has improved.
full-join time: [210.61 µs 210.80 µs 211.01 µs]
change: [-13.686% -13.497% -13.312%] (p = 0.00 < 0.05)
Performance has improved.
I'll review this on Monday :) |
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.
🚀
Description of Changes
AlgebraicTypeLayout
and friends already include full layout information, including properly-aligned offsets forProductTypeElementLayout
s. As such, there's no need to do any alignment computation duringserialize_value
orwrite_value
.Instead, while traversing a
ProductTypeLayout
, we can use each element'soffset
to update thecurr_offset
.API and ABI breaking changes
N/a
Expected complexity level and risk
3 - minor change, but in close contact with some
unsafe
code.Testing
table
tests pass, including proptests.core
tests and smoketests.