-
Notifications
You must be signed in to change notification settings - Fork 726
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
Eliminate extensions allocations #2840
base: master
Are you sure you want to change the base?
Conversation
Seems to have failed for an unrelated reason: #2842 |
Oh wow, thanks for opening this PR and sorry for the delay in responding. I haven't gone over the benchmarks yet (I'd like to rerun them on a less noisy machine...), but they seem to be very promising: Benchmarking results
(This was after I ran the same set of benchmarks on the |
Signed-off-by: Alex Saveau <[email protected]>
Thanks for working on this, this is something I had been planning on doing for a long time but never had the time for. I'm really excited to review this PR! |
Signed-off-by: Alex Saveau <[email protected]>
Signed-off-by: Alex Saveau <[email protected]>
Did a bit of cleanup, the PR should be in a nicer state now. |
Fixes #428
What does this do?
Pools allocations of extensions (implicitly using the fact that AnyMap is stored in a pool) by reusing Boxes.
Why can't this be safe code?
It'd be nice to do this:
Sadly you can't do that because the dyn is unsized. Instead, we manually re-implement the vtable in unsafe code.
Miri has been run on the entire repo with this change:
RUSTFLAGS="--cfg nightly" MIRIFLAGS=-Zmiri-backtrace=full cargo miri t --workspace
Performance
Tested on https://github.com/SUPERCILEX/ftzz.
Before:
After:
The increased fragmentation is a little surprising and a bummer, but that might just be in my application as there were no other allocations to remove.
Random notes
There's no need to implement cache eviction because the HashMap is keyed by types which means the maximum amount of memory used is pre-determined at compile time.
I'm using strict provenance APIs for miri testing as I need to use usize to ptr casts which is no bueno.