-
-
Notifications
You must be signed in to change notification settings - Fork 661
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
[hxb] Implement shared string pools #11511
Conversation
I still need to (at least) tackle the " |
Yes I think that's what the "TODO: macro vs non macro" was about. |
Yeah that was me adding that when merging dev with the config changes x) |
Adding to 5.0 preview 1 because I don't remember what the status is here (thought this was merged already..) so I should at least check what's up. |
Huh, I also thought we merged this already... |
So, this lgtm. Rudy CI (well, the parts that I updated) complains a bit about changing IDs but that applies to I see around 33% time reduction on display requests, which is pretty nice (though I'm still barely under 200ms). |
The configuration is still missing, but I'd be fine with adding that "later". |
Opened #11648 for this, I need that small perf boost to work on further improvements :) |
This implements what I'm talking about in #11510. When using
--hxb
, a top-levelStringPool.hxb
is generated, which is then read once and passed to all hxb readers. For now, this is always activated. Once #11507 is merged we can make this configurable. I think the default should probably be off because it's unclear if this actually yields better performance characteristics for completion and such.The cache-aspect of this is not implemented yet. @kLabz Could you handle this? It should be very straightforward because all hxb readers have access to a
cc
instance anyway, so if that carries a string pool we should be good to go. The only part I'm worried about is--hxb
using the data from the cache, but perhaps that is also just a matter of writing the pool from cc to StringPool.hxb.Edit: Note that the cache version should not call
StringPool.finalize
because we never want finalize the persisted pool.