fix(symbols): memoize ShortNamed per index generation - #562
Merged
Merged
Conversation
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
…t comparisons Co-Authored-By: jason.han <hanhuijun@gmail.com>
Merged
6 tasks
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.
What and why
The membership-import pruning that bounds
importPrefixAvailable(#541) asksIndex.ShortNamedwhether any symbol under a given last-name segment is registered under a short name. That answer was recomputed on every call by walking every qualified name ending in the segment and every symbol under each, so a model with many membership imports — the Apollo 11 example has hundreds — spent ~25% of a whole-model-validateinShortNamed,lastSeparatorand map lookups (0.44 s → 0.55 s).ShortNamedis now memoized per index generation, following the same pattern aslookupDirectChildren:The cache lives beside
directChildrenCacheand is reset with it inresetDirectChildrenCachesLocked, so any layer write (everyset/delbumps the generation) invalidates it. The resolver-sideReadSegmentdependency tracking is untouched.How it was verified
TestShortNamedFollowsIndexGeneration: false on an empty index, true after addingpart def <s> Y;, stable on repeat, false again after the document is replaced and after it is removed.go test -count=1 ./internal/semantic/... ./internal/check/... ./tests/resolve;go test -race ./internal/semantic/symbols ./internal/semantic/resolve;go build ./...,go vet,gofmt -l .empty;python3 scripts/changelog.py check.-validate, 5 interleaved runs, medians: before fix(resolve): skip membership imports that cannot surface a looked-up name #541 0.43 s, develop 0.55 s, this branch 0.45 s. The many-exposeview that fix(resolve): skip membership imports that cannot surface a looked-up name #541 fixed still validates in 0.02 s.Checklist
make testandmake lintpass locallychanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelog