Repository navigation
Share a single cache for all type relationships and propagate results hierarchically - #64808
Daniel Rosenwasser (DanielRosenwasser) wants to merge 4 commits into
Conversation
|
TypeScript Bot (@typescript-bot) test top1000 |
There was a problem hiding this comment.
🟡 Changes recommended
Directional failure propagation can create unreachable identity entries that incorrectly consume the identity comparison budget.
1 open finding
What changed in this PR
Shares and packs checker relationship caches, with hierarchical result propagation to reduce memory and redundant comparisons.
Changes:
- Packs five relationship results into one shared cache entry.
- Propagates successful and failed relationship implications.
- Adds unit tests and a storage benchmark.
| File | Description |
|---|---|
tsc/internal/checker/relater.go |
Implements packed shared caching and propagation. |
tsc/internal/checker/relater_test.go |
Tests packing, propagation, precedence, and storage. |
tsc/internal/checker/checker.go |
Initializes the shared relationship cache. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| if result&RelationComparisonResultFailed != 0 && result&RelationComparisonResultOverflow == 0 { | ||
| for implied, shift := shiftedResult>>packedRelationResultBits, int(r.packedShift)-packedRelationResultBits; implied != 0; implied, shift = implied>>packedRelationResultBits, shift-packedRelationResultBits { | ||
| packed = r.packedResults.setPackedResult(packed, implied, uint8(shift), true) | ||
| } |
There was a problem hiding this comment.
That is pretty annoying, and all of this does make it harder to report results. I'm not sure how accurately address this in a performant way other than distinguishing between filled-in entries and lookups that initially had no value but caused work to actually occur. I would actually say the latter is more what we're interested in in the checker anyway though.
There was a problem hiding this comment.
Actually, what we could do is not propagate until we actually care. Basically
- Fetch a cache entry - if it's 0, nothing's available. Relate the types, fill in the blank, update the count.
- Check your own flags. Stop if you've got anything.
- If not, something else has checked the current types.
- Clear all bits below, and while bits are set, shift right by 6, check if the lower 6 are set, and if so check for a (usable!) false result.
1. If one is set, propagate.
2. Otherwise do work
3. Update the count - Basically do the right corresponding thing for positive results in the lower bits.
With this we won't update counts unnecessarily, but we will sort of "miscount" how much work happens. But I think that is okay.
We could also just keep the values completely disjoint when fetching and avoid this issue.
|
Daniel Rosenwasser (@DanielRosenwasser) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
lspComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Daniel Rosenwasser (@DanielRosenwasser) Here are the results of running the top 1000 repos with tsc comparing Everything looks good! |
|
TypeScript Bot (@typescript-bot) perf test this |
|
Daniel Rosenwasser (@DanielRosenwasser) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
lspComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
TypeScript Bot (@typescript-bot) perf test this |
|
Daniel Rosenwasser (@DanielRosenwasser) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
lspComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
startupComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||

This change brings back an optimization I was really hoping to explore a while back, but I implemented it in a slightly naive way in the Strada codebase. The inspiration is hierarchical caching, but it is now applied in a manner where cache results are all packed into the same
uint32in the same shared map.Today we have a separate cache for every type relationship. This keeps things clean, but it comes with a few downsides. One is that there is some amount of overlap between the caches. For example, when I had an agent analyze the overlap between caches, it found the following results.
VS Code cache overlap analysis
Ranking all ten pairs on the VS Code
srcworkload, using Jaccard overlap (shared keys / unique keys across both caches) as the primary metric:“Potential entry reduction” is
shared / (left size + right size): the percentage of existing map entries eliminated if the two maps shared one key while retaining separate relation results.Ranking by raw duplicate count
If you care only about the absolute number of duplicated keys rather than normalized similarity, the order is:
The practical conclusion is that subtype/strict-subtype is the only major overlap pair. Subtype/assignable is a distant second, and every other normalized overlap is below 1%. Identity comparisons are additionally imperfect here because identity keys normalize source/target order, while the other relations are directional.
The other downside to these distinct caches is that there's additional bookkeeping and waste for each separate type relationship - every result takes up 3 more bytes than is required to represent the actual comparison result. In fact, each result only needs 6 bits to represent the result, and we only have 5 type relationships: comparable, assignable, subtype, strict-subtype, and identity.
With this change, we use a single shared cache for every type relationship. Cache results share the same underlying
uint32, and type relationships keep track of where in theuint32they live. Stricter cache values live in the lower bits (i.e. identity starts at the0th bit), while weaker cache values live in the higher bits.Additionally, cached positive results are proactively propagated for weaker type relationships, while cached negative results are propagated for stronger type relationships. This propagation is cheap to propagate and requires no extra memory if no other type relationship is ever interested in the result.
For example, let's say we have types
FooandBar. If we check whetherFoois comparable toBar, the result will be cached. If the result is negative, we can propagate that information to stronger type relationships. We know that if something is not comparable, it is not assignable, nor a subtype, nor a strict subtype, and certainly not identical.I tested a few different variations of this. One of them tried to limit sharing between only the subtype and strict subtype relationships. That was good, but I could restore the special case there; but the current implementation has some more optimizations that make the code cleaner and still more efficient overall. Turning off propagation entirely resulted in slightly better perf (0.4% faster), while turning it back on was slightly more memory efficient (0.6% less memory), but both of these amounts technically fell in the noise threshold.
But full propagation between all type relationships also created fewer objects in the checker on VS Code: 124 fewer symbols, 98 fewer types, 2976 fewer instantiations.
In compiling VS Code with
--noEmit --checkers 1, this nets a 1.2% speedup plus a 35MiB reduction in memory usage. When I ran on multiple checkers, here's what I got:I might still want to do some refactoring, but we might as well see how this goes in a perf run.