-
Notifications
You must be signed in to change notification settings - Fork 244
Use VarULE baked storage in PackedPatterns #7309
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
base: main
Are you sure you want to change the base?
Conversation
|
Build time measurements of the Before: After: |
|
Benchmark measurements: Benchmarks that say "format only" are unchanged. Ones that say "construct and format" have a regression of somewhere between 1% and 9% depending on how my computer is feeling; the consensus seems to be somewhere around 5-6%. To reproduce, run: |
|
If I mark the ZeroFrom impl as inline, the results are still noisy but seem to go down to about 3-4% regression. |
|
Finally, code size. Before: Details-rwxr-xr-x 1 sffc primarygroup 52750 Dec 11 17:57 and_list.wasm -rwxr-xr-x 1 sffc primarygroup 41593 Dec 11 17:57 casemapping.wasm -rwxr-xr-x 1 sffc primarygroup 276517 Dec 11 17:57 chrome_transliterators.wasm -rwxr-xr-x 1 sffc primarygroup 3409535 Dec 11 17:57 chrono_jiff.wasm -rwxr-xr-x 1 sffc primarygroup 36041 Dec 11 17:57 code_line_diff.wasm -rwxr-xr-x 1 sffc primarygroup 47052 Dec 11 17:57 date_try_from_fields.wasm -rwxr-xr-x 1 sffc primarygroup 9306 Dec 11 17:57 derives.wasm -rwxr-xr-x 1 sffc primarygroup 22222 Dec 11 17:57 elevator_floors.wasm -rwxr-xr-x 1 sffc primarygroup 415 Dec 11 17:57 experimental_segmenter.wasm -rwxr-xr-x 1 sffc primarygroup 14863 Dec 11 17:57 filter_langids.wasm -rwxr-xr-x 1 sffc primarygroup 12318 Dec 11 17:57 first_weekday_for_region.wasm -rwxr-xr-x 1 sffc primarygroup 1492 Dec 11 17:57 iso_date_manipulations.wasm -rwxr-xr-x 1 sffc primarygroup 10836 Dec 11 17:57 language_names_hash_map.wasm -rwxr-xr-x 1 sffc primarygroup 16187 Dec 11 17:57 language_names_lite_map.wasm -rwxr-xr-x 1 sffc primarygroup 28387 Dec 11 17:57 litemap_bincode.wasm -rwxr-xr-x 1 sffc primarygroup 16921 Dec 11 17:57 litemap_postcard.wasm -rwxr-xr-x 1 sffc primarygroup 104287 Dec 11 17:57 make_var.wasm -rwxr-xr-x 1 sffc primarygroup 8052 Dec 11 17:57 make.wasm -rwxr-xr-x 1 sffc primarygroup 8994 Dec 11 17:57 permyriad.wasm -rwxr-xr-x 1 sffc primarygroup 31682 Dec 11 17:57 syntatically_canonicalize_locales.wasm -rwxr-xr-x 1 sffc primarygroup 2712024 Dec 11 17:57 timezone_picker.wasm -rwxr-xr-x 1 sffc primarygroup 341108 Dec 11 17:57 tui.wasm -rwxr-xr-x 1 sffc primarygroup 9156 Dec 11 17:57 unicode_bmp_blocks_selector.wasm -rwxr-xr-x 1 sffc primarygroup 23988 Dec 11 17:57 unread_emails.wasm -rwxr-xr-x 1 sffc primarygroup 306727 Dec 11 17:57 work_log.wasm -rwxr-xr-x 1 sffc primarygroup 19494 Dec 11 17:57 writeable_message.wasm -rwxr-xr-x 1 sffc primarygroup 3420 Dec 11 17:57 yoke_derive.wasm -rwxr-xr-x 1 sffc primarygroup 3415 Dec 11 17:57 zf_derive.wasm -rwxr-xr-x 1 sffc primarygroup 559 Dec 11 17:57 zv_serde.wasm After: Details-rwxr-xr-x 1 sffc primarygroup 52750 Dec 11 17:58 and_list.wasm -rwxr-xr-x 1 sffc primarygroup 41593 Dec 11 17:58 casemapping.wasm -rwxr-xr-x 1 sffc primarygroup 276873 Dec 11 17:58 chrome_transliterators.wasm -rwxr-xr-x 1 sffc primarygroup 3737310 Dec 11 17:58 chrono_jiff.wasm -rwxr-xr-x 1 sffc primarygroup 36041 Dec 11 17:58 code_line_diff.wasm -rwxr-xr-x 1 sffc primarygroup 47052 Dec 11 17:58 date_try_from_fields.wasm -rwxr-xr-x 1 sffc primarygroup 9306 Dec 11 17:58 derives.wasm -rwxr-xr-x 1 sffc primarygroup 22222 Dec 11 17:58 elevator_floors.wasm -rwxr-xr-x 1 sffc primarygroup 415 Dec 11 17:58 experimental_segmenter.wasm -rwxr-xr-x 1 sffc primarygroup 14863 Dec 11 17:58 filter_langids.wasm -rwxr-xr-x 1 sffc primarygroup 12318 Dec 11 17:58 first_weekday_for_region.wasm -rwxr-xr-x 1 sffc primarygroup 1492 Dec 11 17:58 iso_date_manipulations.wasm -rwxr-xr-x 1 sffc primarygroup 10836 Dec 11 17:58 language_names_hash_map.wasm -rwxr-xr-x 1 sffc primarygroup 16187 Dec 11 17:58 language_names_lite_map.wasm -rwxr-xr-x 1 sffc primarygroup 28387 Dec 11 17:58 litemap_bincode.wasm -rwxr-xr-x 1 sffc primarygroup 16921 Dec 11 17:58 litemap_postcard.wasm -rwxr-xr-x 1 sffc primarygroup 104287 Dec 11 17:58 make_var.wasm -rwxr-xr-x 1 sffc primarygroup 8052 Dec 11 17:58 make.wasm -rwxr-xr-x 1 sffc primarygroup 8994 Dec 11 17:58 permyriad.wasm -rwxr-xr-x 1 sffc primarygroup 31682 Dec 11 17:58 syntatically_canonicalize_locales.wasm -rwxr-xr-x 1 sffc primarygroup 2712024 Dec 11 17:58 timezone_picker.wasm -rwxr-xr-x 1 sffc primarygroup 335574 Dec 11 17:58 tui.wasm -rwxr-xr-x 1 sffc primarygroup 9156 Dec 11 17:58 unicode_bmp_blocks_selector.wasm -rwxr-xr-x 1 sffc primarygroup 23988 Dec 11 17:58 unread_emails.wasm -rwxr-xr-x 1 sffc primarygroup 301186 Dec 11 17:58 work_log.wasm -rwxr-xr-x 1 sffc primarygroup 19494 Dec 11 17:58 writeable_message.wasm -rwxr-xr-x 1 sffc primarygroup 3420 Dec 11 17:58 yoke_derive.wasm -rwxr-xr-x 1 sffc primarygroup 3415 Dec 11 17:58 zf_derive.wasm -rwxr-xr-x 1 sffc primarygroup 559 Dec 11 17:58 zv_serde.wasm Differences:
|
|
Things to highlight:
|
|
🎉 All dependencies have been resolved ! |
|
Please review the whole PR; the commits got messy due to merges, and there isn't that much code being changed. |
|
@Manishearth Do you think the metrics posted above justify the change? |
|
Yes! I think the performance slowdown is basically noise and the build time improvement is important for one of our larger baked data crates. Edit: hadn't really realized the size of the codesize loss in the chrono test. ignore! |
robertbastian
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.
The numbers don't look very conclusive to me, I don't think build time is that important given that runtime performance will suffer.
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.
should this change?
|
Yeah, we looked closer at this and determined we shouldn't land this, but that this was a good proof of concept. It was important to learn that depending on the data architecture, data deduplication may suffer with this type of optimization (which explains the large chrono_jiff codesize) |
Depends on #7310
This should have been done after #5230 but got dropped.