Repository navigation
Checker affinity tweaks - #64870
Open
Jake Bailey (jakebailey) wants to merge 4 commits into
Open
Checker affinity tweaks#64870Jake Bailey (jakebailey) wants to merge 4 commits into
Jake Bailey (jakebailey) wants to merge 4 commits into
Conversation
Each checker builds its own caches for whatever types its files reach,
so files that share a neighborhood (a package's own types, the
framework declarations it uses) should land on the same checker.
Assigning one file at a time did a poor job of that. Direct imports
are only a small part of what neighboring files have in common, so
related files were scattered across checkers, each rebuilding the same
types. Balance suffered too: declaration files excluded by skipLibCheck
still counted toward a checker's load even though no checker spends
time on them, so the real work per checker was uneven.
Now the partition leaves out files that are not type checked and
places whole directories: the outermost directory weighing at most a
quarter of a checker's load is placed as one unit, heaviest units
first. This replaces the calibrated source/declaration policies, which
existed to work around the same problems. Diagnostics are unchanged on
vscode, mui-docs, the TypeScript compiler, and xstate.
--checkers 4:
│ b4.txt │ a4.txt │
│ sec/op │ sec/op vs base │
Tsc/bluesky 2.684 ± 4% 2.411 ± 2% -10.17% (p=0.002 n=6)
Tsc/sentry 10.45 ± 4% 10.32 ± 3% ~ (p=0.180 n=6)
Tsc/t3code-server 9.234 ± 4% 8.306 ± 2% -10.04% (p=0.002 n=6)
Tsc/mui-docs 7.426 ± 2% 6.714 ± 2% -9.59% (p=0.002 n=6)
geomean 6.623 6.103 -7.85%
│ b4.txt │ a4.txt │
│ rss-B │ rss-B vs base │
Tsc/bluesky 1.184Gi ± 3% 1.167Gi ± 2% ~ (p=0.065 n=6)
Tsc/sentry 3.526Gi ± 6% 3.393Gi ± 6% ~ (p=0.132 n=6)
Tsc/t3code-server 2.816Gi ± 6% 2.690Gi ± 8% ~ (p=0.132 n=6)
Tsc/mui-docs 2.246Gi ± 2% 2.015Gi ± 2% -10.25% (p=0.002 n=6)
geomean 2.267Gi 2.153Gi -5.03%
│ b4.txt │ a4.txt │
│ heap-B │ heap-B vs base │
Tsc/bluesky 933.1Mi ± 0% 915.0Mi ± 0% -1.94% (p=0.002 n=6)
Tsc/sentry 2.605Gi ± 0% 2.479Gi ± 0% -4.84% (p=0.002 n=6)
Tsc/t3code-server 2.281Gi ± 0% 2.179Gi ± 0% -4.50% (p=0.002 n=6)
Tsc/mui-docs 1.717Gi ± 0% 1.562Gi ± 0% -9.05% (p=0.002 n=6)
geomean 1.746Gi 1.657Gi -5.12%
--checkers 8:
│ b8.txt │ a8.txt │
│ sec/op │ sec/op vs base │
Tsc/bluesky 2.036 ± 4% 1.915 ± 2% -5.97% (p=0.004 n=6)
Tsc/sentry 8.352 ± 3% 7.233 ± 1% -13.40% (p=0.002 n=6)
Tsc/t3code-server 7.864 ± 3% 6.880 ± 5% -12.52% (p=0.002 n=6)
Tsc/mui-docs 6.367 ± 3% 5.856 ± 2% -8.04% (p=0.002 n=6)
geomean 5.402 4.860 -10.03%
│ b8.txt │ a8.txt │
│ rss-B │ rss-B vs base │
Tsc/bluesky 1.530Gi ± 2% 1.470Gi ± 2% -3.93% (p=0.002 n=6)
Tsc/sentry 4.214Gi ± 9% 4.011Gi ± 3% -4.84% (p=0.009 n=6)
Tsc/t3code-server 4.027Gi ± 4% 3.129Gi ± 2% -22.31% (p=0.002 n=6)
Tsc/mui-docs 3.029Gi ± 9% 2.573Gi ± 2% -15.07% (p=0.002 n=6)
geomean 2.978Gi 2.625Gi -11.87%
│ b8.txt │ a8.txt │
│ heap-B │ heap-B vs base │
Tsc/bluesky 1.206Gi ± 0% 1.155Gi ± 0% -4.17% (p=0.002 n=6)
Tsc/sentry 3.116Gi ± 0% 2.892Gi ± 0% -7.18% (p=0.002 n=6)
Tsc/t3code-server 3.052Gi ± 0% 2.502Gi ± 0% -18.02% (p=0.002 n=6)
Tsc/mui-docs 2.478Gi ± 0% 1.917Gi ± 0% -22.66% (p=0.002 n=6)
geomean 2.309Gi 2.001Gi -13.34%
A file's estimated work counts each of its imports. A barrel that
re-exports thousands of modules is then estimated as one of the most
expensive files in the program although it is among the cheapest to
check: MUI's icon index re-exports 10,750 modules and takes 20ms, yet
carried 14% of the docs project's estimated work. Its checker was
treated as full, and import affinity pulled every icon onto it too, so
one of four checkers spent the check phase on cheap files while the
other three shared all of the real work.
Counting at most 256 imports per file leaves ordinary files alone (the
largest import count in bluesky, sentry and t3code is 358) and lets
that checker take a full share. Using the fourth checker costs memory
on mui-docs, since it now builds the MUI types as well.
--checkers 4:
│ b4.txt │ a4.txt │
│ sec/op │ sec/op vs base │
Tsc/bluesky 2.394 ± 5% 2.369 ± 7% ~ (p=0.240 n=6)
Tsc/sentry 10.42 ± 3% 10.20 ± 3% ~ (p=0.180 n=6)
Tsc/t3code-server 8.336 ± 3% 8.184 ± 2% ~ (p=0.132 n=6)
Tsc/mui-docs 6.898 ± 2% 6.537 ± 1% -5.23% (p=0.002 n=6)
geomean 6.154 5.996 -2.57%
│ b4.txt │ a4.txt │
│ rss-B │ rss-B vs base │
Tsc/bluesky 1.160Gi ± 3% 1.167Gi ± 1% ~ (p=0.093 n=6)
Tsc/sentry 3.402Gi ± 3% 3.323Gi ± 4% ~ (p=0.937 n=6)
Tsc/t3code-server 2.776Gi ± 6% 2.820Gi ± 6% ~ (p=0.699 n=6)
Tsc/mui-docs 2.017Gi ± 3% 2.299Gi ± 5% +14.00% (p=0.002 n=6)
geomean 2.168Gi 2.239Gi +3.30%
│ b4.txt │ a4.txt │
│ heap-B │ heap-B vs base │
Tsc/bluesky 914.8Mi ± 0% 914.9Mi ± 0% ~ (p=1.000 n=6)
Tsc/sentry 2.478Gi ± 0% 2.483Gi ± 0% +0.18% (p=0.004 n=6)
Tsc/t3code-server 2.178Gi ± 0% 2.178Gi ± 0% ~ (p=0.699 n=6)
Tsc/mui-docs 1.562Gi ± 0% 1.732Gi ± 0% +10.87% (p=0.002 n=6)
geomean 1.657Gi 1.701Gi +2.66%
--checkers 8:
│ b8.txt │ a8.txt │
│ sec/op │ sec/op vs base │
Tsc/bluesky 1.980 ± 3% 1.947 ± 2% ~ (p=0.180 n=6)
Tsc/sentry 7.520 ± 4% 7.402 ± 1% -1.56% (p=0.041 n=6)
Tsc/t3code-server 7.042 ± 2% 6.837 ± 2% -2.92% (p=0.004 n=6)
Tsc/mui-docs 5.859 ± 2% 5.727 ± 1% -2.25% (p=0.009 n=6)
geomean 4.978 4.874 -2.10%
│ b8.txt │ a8.txt │
│ rss-B │ rss-B vs base │
Tsc/bluesky 1.470Gi ± 2% 1.481Gi ± 3% ~ (p=0.310 n=6)
Tsc/sentry 4.015Gi ± 3% 4.016Gi ± 3% ~ (p=0.699 n=6)
Tsc/t3code-server 3.156Gi ± 3% 3.123Gi ± 1% ~ (p=0.310 n=6)
Tsc/mui-docs 2.561Gi ± 7% 2.698Gi ± 7% +5.38% (p=0.041 n=6)
geomean 2.628Gi 2.661Gi +1.24%
│ b8.txt │ a8.txt │
│ heap-B │ heap-B vs base │
Tsc/bluesky 1.155Gi ± 0% 1.155Gi ± 0% ~ (p=0.699 n=6)
Tsc/sentry 2.893Gi ± 0% 2.881Gi ± 0% -0.40% (p=0.002 n=6)
Tsc/t3code-server 2.502Gi ± 0% 2.502Gi ± 0% ~ (p=0.818 n=6)
Tsc/mui-docs 1.917Gi ± 0% 2.043Gi ± 0% +6.58% (p=0.002 n=6)
geomean 2.001Gi 2.031Gi +1.51%
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The scheduling rewrite needs human confirmation of diagnostic stability and workload balance, with direct regression coverage still missing.
1 open issue
What changed in this PR
Refines checker assignment to improve locality and balance parallel type-checking work.
Changes:
- Groups files by directory and assigns heavier groups first.
- Excludes skipped declaration/JSON files and caps import-based weights.
- Replaces obsolete policy tests with directory-grouping tests.
| File | Description |
|---|---|
| tsc/internal/compiler/checkerpool.go | Implements directory-based grouping and revised workload estimates. |
| tsc/internal/compiler/checkerpool_test.go | Tests grouping and updates partitioning test constants. |
Review effort: Balanced
Comment on lines
+314
to
+318
| if (file.IsDeclarationFile || ast.IsJsonSourceFile(file)) && p.program.SkipTypeChecking(file, false) { | ||
| continue | ||
| } | ||
| baseWeights[i] = getCheckerAssociationBaseWeight(file.NodeCount, len(file.Text())) | ||
| importCounts[i] = min(len(file.Imports()), checkerAssociationMaxImports) |
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Checking takes as long as the slowest checker. Estimated work can be
well off for a whole directory, and with groups of up to a quarter of
a checker's load, a single misjudged group could leave one checker
seconds behind the rest. The perf run on the PR showed a large VS Code
check-time regression this way, while other VS Code snapshots
improved, which fits a result that hinges on where one group lands.
Cap groups at a sixteenth of a checker's load instead. Packages still
stay together in most projects, but no single group can unbalance the
checkers as much.
Wall time at 4 checkers, compared with the previous commit
(vscode-older is the July snapshot from the benchmarking repo):
│ /tmp/old.txt │ /tmp/new.txt │
│ sec/op │ sec/op vs base │
Tsc/vscode 14.60 ± 4% 14.49 ± 1% ~ (p=0.589 n=6)
Tsc/vscode-older 7.773 ± 5% 7.725 ± 3% ~ (p=0.589 n=6)
Tsc/mui-docs 8.800 ± 3% 7.981 ± 4% -9.31% (p=0.002 n=6)
geomean 9.995 9.630 -3.66%
│ /tmp/old.txt │ /tmp/new.txt │
│ rss-B │ rss-B vs base │
Tsc/vscode 5.805Gi ± 3% 5.907Gi ± 2% ~ (p=0.093 n=6)
Tsc/vscode-older 3.840Gi ± 2% 3.866Gi ± 3% ~ (p=0.589 n=6)
Tsc/mui-docs 3.655Gi ± 6% 3.639Gi ± 6% ~ (p=0.818 n=6)
geomean 4.335Gi 4.364Gi +0.67%
│ /tmp/old.txt │ /tmp/new.txt │
│ heap-B │ heap-B vs base │
Tsc/vscode 4.921Gi ± 0% 4.937Gi ± 0% +0.33% (p=0.002 n=6)
Tsc/vscode-older 3.205Gi ± 0% 3.213Gi ± 0% +0.24% (p=0.002 n=6)
Tsc/mui-docs 2.746Gi ± 0% 2.732Gi ± 0% -0.52% (p=0.002 n=6)
geomean 3.512Gi 3.512Gi +0.01%
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
An import's weight is the total syntax weight divided by the number of
imports, so that imports and syntax contribute equally. A program with
almost no imports, such as the pre-modules TypeScript compiler, which is
built from namespaces, has 12 imports in total, making each one worth
14 times an average file. sys.ts was then estimated at more than twice
checker.ts, got a checker to itself, and the checker with checker.ts
also had to take a share of the remaining files.
An import is now worth at most twice the average weighted file. Every
other measured project has imports worth well under that, so their
assignments do not change.
Measured with --checkers 4 (n=6):
│ before.txt │ after.txt │
│ sec/op │ sec/op vs base │
Tsc/ts-pre-modules 730.0m ± 4% 653.0m ± 3% -10.55% (p=0.002 n=6)
Tsc/mui-docs 7.781 ± 1% 7.723 ± 1% ~ (p=0.121 n=6)
Tsc/bluesky 2.915 ± 4% 2.909 ± 1% ~ (p=0.513 n=6)
Tsc/sentry 11.90 ± 3% 11.85 ± 2% ~ (p=0.937 n=6)
Tsc/t3code-server 15.35 ± 10% 15.33 ± 4% ~ (p=0.565 n=6)
Tsc/botvscode 7.429 ± 0% 7.518 ± 3% +1.20% (p=0.004 n=6)
geomean 5.312 5.211 -1.89%
│ before.txt │ after.txt │
│ check-sec │ check-sec vs base │
Tsc/ts-pre-modules 618.5m ± 3% 539.5m ± 3% -12.77% (p=0.002 n=6)
Tsc/mui-docs 7.232 ± 1% 7.174 ± 1% ~ (p=0.100 n=6)
Tsc/bluesky 2.620 ± 3% 2.615 ± 2% ~ (p=0.974 n=6)
Tsc/sentry 11.03 ± 3% 10.99 ± 2% ~ (p=1.000 n=6)
Tsc/t3code-server 14.89 ± 10% 14.90 ± 4% ~ (p=0.818 n=6)
Tsc/botvscode 6.453 ± 0% 6.530 ± 3% +1.19% (p=0.026 n=6)
geomean 4.812 4.703 -2.28%
│ before.txt │ after.txt │
│ rss-B │ rss-B vs base │
Tsc/ts-pre-modules 262.5Mi ± 2% 271.7Mi ± 2% +3.51% (p=0.002 n=6)
Tsc/mui-docs 3.700Gi ± 7% 3.544Gi ± 8% ~ (p=0.699 n=6)
Tsc/bluesky 1.401Gi ± 4% 1.378Gi ± 2% ~ (p=0.394 n=6)
Tsc/sentry 4.473Gi ± 7% 4.418Gi ± 5% ~ (p=0.589 n=6)
Tsc/t3code-server 3.865Gi ± 2% 3.851Gi ± 2% ~ (p=0.937 n=6)
Tsc/botvscode 3.785Gi ± 2% 3.834Gi ± 2% ~ (p=1.000 n=6)
geomean 2.105Gi 2.095Gi -0.47%
│ before.txt │ after.txt │
│ heap-B │ heap-B vs base │
Tsc/ts-pre-modules 198.5Mi ± 0% 206.8Mi ± 0% +4.18% (p=0.002 n=6)
Tsc/mui-docs 2.732Gi ± 0% 2.732Gi ± 0% ~ (p=0.589 n=6)
Tsc/bluesky 1.105Gi ± 0% 1.106Gi ± 0% ~ (p=1.000 n=6)
Tsc/sentry 3.400Gi ± 0% 3.400Gi ± 0% ~ (p=1.000 n=6)
Tsc/t3code-server 3.168Gi ± 0% 3.169Gi ± 0% ~ (p=0.132 n=6)
Tsc/botvscode 3.212Gi ± 0% 3.212Gi ± 0% ~ (p=0.589 n=6)
geomean 1.651Gi 1.662Gi +0.69%
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
Contributor
Contributor
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This branch has not been deployed
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.
See the two commits.