Assign files to checkers using balanced import affinity - #4313
Conversation
The checker pool previously assigned files to checkers with simple round-robin striping. That balances file count, but adjacent files and resolved import neighbors often share generic instantiations and symbol/type links, so the same work is repeated across checker-local caches. Start with deterministic weighted blocks to preserve program-order locality, then run a small FM-style graph refinement over the resolved import graph. A file can move toward the checker containing more of its import neighbors only when the target remains under a strict static cost cap, keeping parallel work balanced. Perf on ~/work/vscode/src with tsgo -p . --noEmit --extendedDiagnostics, 5-run average versus main: Memory allocs 25,210,536 -> 24,028,267 (-1,182,268, -4.7%). Memory used 4,473,325K -> 4,242,826K (-230,500K, -5.2%). Check time 4.741s -> 4.294s (-0.447s, -9.4%). Total time 5.370s -> 4.969s (-0.401s, -7.5%).
|
TypeScript Bot (@typescript-bot) perf test this faster |
There was a problem hiding this comment.
Pull request overview
Updates the compiler’s checker pooling strategy to assign source files to checkers using a weight-balanced, import-graph-aware affinity algorithm, aiming to improve type-check performance by keeping related files on the same checker while maintaining load balance.
Changes:
- Added a file-to-checker association algorithm that assigns contiguous file blocks by estimated “work” (text + node + symbol counts).
- Refined those associations using an undirected import adjacency graph, moving files to increase import-neighbor locality while respecting a balance cap.
- Added unit tests covering the block sizing, initial weight-based association, and graph refinement behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/compiler/checkerpool.go | Implements weight-based checker associations and import-graph refinement, and uses the resulting mapping when creating fileAssociations. |
| internal/compiler/checkerpool_test.go | Adds unit tests for the new association/refinement helpers. |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Daniel Rosenwasser (DanielRosenwasser)
left a comment
There was a problem hiding this comment.
This seems really neat. I think the algorithm is mostly understandable, but would prefer some more comments at each point to explain what's happening (e.g. at each loop, it's not immediately clear what we're about to do next)
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Weird. Simplifying the algorithm to just use text made it a lot worse in time? |
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
That is the result I'm expecting, FWIW. Is this PR explained enough? |
|
I like the improvements, even if it mostly seems to be around memory consumption. I'm surprised the initial compile time improvement for VS Code evaporated. Any idea why? |
|
It was my last change; the weird count stuff is apparently load bearing in vscode. I'm going to have to think about a more principled way to weight files... |
|
Hm, so it seems like the effect from the "complexity" estimation is more important than I thought. The import graph still matters, but just shifting files around by how complicated they look does seem to matter even more. I had copilot do a big matrix of different things and am about to push a new one which changes the algorithm again to, which it described as: Phase 1: Work-balanced initial assignmentUse LPT, or longest-processing-time-first scheduling. This directly targets the critical path: wall-clock check time is bounded by the slowest checker bucket. Phase 2: Import-graph locality refinementBuild an undirected graph where two files are adjacent if one imports the other. The load cap is roughly SummaryThe ordering is the important part: balance checker work first, then improve import locality within that balance budget. LPT gives a good max-load baseline, and graph refinement recovers checker cache locality without being allowed to create a new overloaded checker. |
Assign files to checkers with longest-processing-time-first scheduling before running the import-graph refinement pass. The estimated work for a file is now its node count plus one unit per 100 bytes of source text, which better tracks checker work than source text size alone. Measured on ~/work/vscode/src with tsgo -p . --noEmit --extendedDiagnostics, 5-run average versus the current text+graph assignment: - current text+graph: Check time 9.168s, Total time 10.298s, Symbols 6,407,330, Types 2,217,598, Memory 4,394,421K, Allocs 24,807,475. - pre-text old-weight+graph: Check time 8.740s (-4.7%), Total time 10.028s (-2.6%), Symbols -0.5%, Types -0.8%, Memory -0.5%, Allocs -0.2%. - LPT node+text+graph: Check time 8.457s (-7.8%), Total time 9.587s (-6.9%), Symbols +0.9%, Types +1.2%, Memory +0.5%, Allocs +0.7%. - LPT old-weight+graph: Check time 8.485s (-7.5%), Total time 9.632s (-6.5%), Symbols ~, Types +0.3%, Memory -0.2%, Allocs +0.2%. The import graph remains useful: removing graph refinement from the LPT variants preserved some check-time improvement but increased symbols, types, memory, and allocations by roughly 4-8% on this benchmark.
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Tested this on our large monorepo (~17M lines of TS) -- seems to be a great improvement! Both runs were with #4309 reverted since it causes a major ram regression. Before~2m 15s After~2m 1s so ~10% faster and 10% more efficient ram use, love it! |
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
I tested the method from #4856 and it was worse; the FENNEL approach here does a better job across workloads, mui, etc. |
|
It looks like Copilot did a parameter sweep for all the constant factors except this one - give it a go? const checkerAssociationTextWeightDivisor = 100Code LGTM, I like how isolated this is |
|
It did actually sweep that; I did just have it retry and 100 is still the best in what projects I have. |
|
I will let it update some comments afterward, though, so will probably push soon when it's done benchmarking |
Right now, we assign checkers round-robin. I've always suspected that there's a better packing that could be used to make us faster and use less memory.
This PR is the result of letting Copilot churn reading papers, testing things out, experimenting; it eventually found "FENNEL", a MS Research concept (linked below) and adapted it into something that appears to produce a good result for all of the benchmarks without regression.
I then gave it more test cases that it hadn't seen, and those were neutral to positive too.
Notion, Slack, others have tested this and seen similar improvements, roughly 10% faster with 10% less memory. Benchmark run is here as well: #4313 (comment)
As another experiment, I also tried out another branch that used a PRNG to assign files; with #4820 now in place, I could not find any seed on any of our benchmarks that caused new diagnostics, which gives me hope that changing the checker association algorithm is fine.
Copilot-produced algorithm description
The assignment problem is modeled as balanced graph partitioning:
Related files benefit from sharing a checker because checking one file often populates semantic state used by its dependencies and importers. However, placing too many expensive roots on one checker increases the parallel critical path.
The implementation uses a weighted adaptation of FENNEL, a streaming graph-partitioning algorithm. For each file, it chooses the checker maximizing:
A hard, best-effort load bound prevents graph affinity from creating substantial estimated imbalance.
Estimated work
The base weight is:
Import fanout adds a project-normalized weight. Imports collectively contribute approximately the same weight as local syntax, avoiding a project-specific absolute constant.
Stream order and empirical safeguards
FENNEL is sensitive to vertex order, as discussed in work such as Prioritized Restreaming Algorithms for Balanced Graph Partitioning. This matters especially for the checker because a small source file can trigger large amounts of nonlocal semantic work.
Experiments on VS Code, TypeScript, MUI docs, and XState at 2, 4, and 8 checkers showed that equal estimated loads can still produce badly skewed checker completion times. The final policy therefore uses three calibrated regimes:
The source-dominated threshold and the
4x,12x, and16xsafety factors were selected through cross-project sweeps. They are intended to compensate for semantic work that the static proxy cannot observe, rather than being constants derived from the FENNEL paper.