Repository navigation
perf: forget cached lookups per name, not per context (0158) - #473
Merged
Merged
Conversation
A name added by `set`, a loop body scope cleared after each pass and a loop's slots bound or unbound started a new lookup epoch, so every cached lookup of the context missed once afterwards. They now drop only the entries of the names concerned: - LookupCache groups its 128 entries in 32 buckets of 4 by name hash, and NewSlot(hash) gives a name expression an entry of its name's bucket, so forgetting a name clears 4 keys (kept in their own array). The hit path is unchanged. - Each scope records the buckets of the names added to it, so clearing or leaving it forgets them without hashing its names. - BindSlot and UnbindSlots take the slot names' hashes. Callgrind against 39dc889: many_tags -9.6%, macros -3.8%, chat_mistral -0.6%, config_file -0.5%; mitsuhiko_table +0.7%, for_filter_if +0.6%, inheritance +0.6% (details in docs/tasks/0158). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AmFfzfwMghRTWPifzgmANC
The loop and its lazy filter reloaded the target's slot and hash after each item; the first scope chunk zero-filled masks Push sets anyway. Pays back the bucket clears per loop run: every render case is now within +0.5% of base, mitsuhiko_table +0.24%. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AmFfzfwMghRTWPifzgmANC
rmorozov
marked this pull request as ready for review
October 12, 2026 00:16
rmorozov
pushed a commit
that referenced
this pull request
Oct 12, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMcWcpGwhiaEs33YjAgEV4
rmorozov
added a commit
that referenced
this pull request
Oct 12, 2026
Claude-Session: https://claude.ai/code/session_01KMcWcpGwhiaEs33YjAgEV4 Co-authored-by: Claude <noreply@anthropic.com>
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.
Requested by Ruslan · project thread
Before: a
set, a loop body scope cleared after each pass, or a loop or macro slot bound or unbound started a new lookup epoch for the context. After that, every cached name lookup missed once, including lookups of names the change could not affect. On many_tags every rootsetdid this. On chat_mistral 62 of the 71 misses per render came from these events.After: those events drop only the cache entries of the names they touch. Other names stay cached. Rendered output does not change.
How:
LookupCachegroups its 128 entries into 32 buckets of 4, by name hash.NewSlot(hash)places a name expression in its name's bucket, so forgetting a name clears 4 keys. The keys sit in their own array, so this is two stores. The hit path does not change.ScopeStack, the buckets of the names added to it.ClearandExitScopeforget those buckets without hashing names.BindSlotandUnbindSlotstake the slot names' hashes.InternalValueMapa node map, because the cache keeps pointers to scope values while other names are added.Pushsets anyway. This pays back the extra bucket clears per loop run.Instruction counts after the second round (callgrind, Release, against 39dc889):
Allocations are unchanged. Every render case is now within the house bar of +0.5%.
Exceptions to the task's +0.3% bar:
NewSlotcomputes the name's bucket and uses that bucket's counter. This costs a few instructions once per distinct name in a template, so it is a real cost and not layout.Nested contexts still take a context epoch. A follow-up note on this is in docs/tasks/0158.
Roles: the verifier ran 1 round on the first commit. It found 5 clang-tidy hits and no correctness issue: the Debug cache cross-check passed on about 10k generated and hand-written templates, and the mutation checks were caught. ASan+UBSan, C++20 and preflight pass. The second-round commit passed 4747 tests in Debug (with the cache cross-check) and preflight. Red pushes so far: 0.
🤖 Generated with Claude Code
https://claude.ai/code/session_01AmFfzfwMghRTWPifzgmANC
Generated by Claude Code