[ty] Avoid storing redundant reachability indexes#25453
Merged
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 91.94%. The percentage of expected errors that received a diagnostic held steady at 87.09%. The number of fully passing files held steady at 92/134. |
Memory usage reportSummary
Significant changesClick to expand detailed breakdownflake8
trio
sphinx
prefect
|
|
charliermarsh
force-pushed
the
charlie/compact-retained-semantic-arrays
branch
2 times, most recently
from
55b7221 to
bedff0d
Compare
charliermarsh
force-pushed
the
charlie/elide-reachability-rank-metadata
branch
from
7356b26 to
d9dea8c
Compare
charliermarsh
changed the title
charliermarsh
changed the base branch from
charlie/compact-retained-semantic-arrays
to
main
|
charliermarsh
changed the title
Member
Author
|
Beyond the memory, 4% perf on Pydantic, 2% on anyio (1% regression on some micro benchmarks). |
charliermarsh
requested review from
AlexWaygood,
carljm,
dcreager,
ibraheemdev and
sharkdp
as code owners
Member
|
(the pydantic benchmark seems to have become flaky in the last week or so, regularly reporting speedups or regressions of up to 5% on PRs that couldn't plausibly have had that impact. I suspect some "perf nondeterminism" somewhere... not a knock on your PR, though :-) |
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
3 participants
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.
Summary
When all reachability nodes for a scope are retained, their IDs already match their positions in the stored node array. In that case, this PR avoids storing an extra bit vector for mapping node IDs back to the array. We only allocate that mapping when unused intermediate nodes were removed.
This reduces retained memory for reachability constraints while preserving the existing compaction for scopes that need it. It also moves node lookup into
ReachabilityConstraints::get_interior_node, so both paths use the same lookup logic.