Lock-free root fast paths for hygiene queries [-0.06%, below significance] - #65
Draft
xmakro wants to merge 1 commit into
Draft
Lock-free root fast paths for hygiene queries [-0.06%, below significance]#65xmakro wants to merge 1 commit into
xmakro wants to merge 1 commit into
Conversation
outer_expn_is_descendant_of and normalize_to_macros_2_0/_macro_rules took the HygieneData lock to compute answers that are statically known for the root syntax context, which is the case for the majority of identifiers in non-macro code and is hit several times per name resolution. The root context's data is written once in HygieneData::new and never mutated: alloc_ctxt can never reserve index 0, update_dollar_crate_names touches only dollar_crate_name, and both metadata and on-disk-cache decoding short-circuit raw id 0 before allocating.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
outer_expn_is_descendant_ofandnormalize_to_macros_2_0/_macro_rulestook the HygieneData lock (thread-local + RefCell, or a real mutex under the parallel front end) to compute answers that are statically known for the root syntax context — the common case for non-macro code, hit several times per name resolution.Correctness argument, verified exhaustively: the root context's data is written once in
HygieneData::newand never mutated afterwards.alloc_ctxtcan never reserve index 0 (the vec starts with one element),update_dollar_crate_nameswrites onlydollar_crate_name, and both the metadata and on-disk-cache decoders short-circuitraw_id == 0before allocating.is_descendant_of(_, root)is unconditionally true — the existing sibling fast path checksancestor == rootbefore its cross-crate guard, so foreign expansions are correctly descendants.Honest measurement: -0.06% geomean, zero cells clearing 0.25%. Real but below what a perf run would flag as significant. Submitted because it is provably equivalent and removes lock traffic that matters more under
-Zthreads>1, not because the number justifies it on its own.Known trade-off: a nested
HygieneData::withbug would now abort only when the context happens to be non-root, rather than deterministically.Measured in isolation against a clean base (ThinLTO stage2, glibc, instructions:u, full scenario, 5 crates x 3 profiles = 15 cells). This change was measured alone, not as part of a batch.
Reviewed adversarially before submission; findings that survived are reflected in the code and described above.