Repository navigation
Compute signatures of dependent files in parallel on incremental rebuilds - #64659
Gadzhi Gadzhiev (resure) wants to merge 6 commits into
Conversation
After a shared dependency changes, the incremental builder computed the declaration signatures of the files referencing it one at a time. Each referencing file now gets its own task in the work group, with at most GOMAXPROCS signatures in flight so peak memory stays where it was. The cached path of updateShapeSignature returned false when another traversal had already computed the file's signature, which made the result depend on which changed file was processed first: a changed file that affects the global scope could fail to invalidate every file. It now reports whether that signature changed.
typescript-automation
Bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Root changed-file signature computations bypass the new concurrency limit, allowing unbounded concurrent declaration emits.
Review effort: Balanced
Findings: 1
What changed in this PR
Parallelizes incremental declaration-signature computation while preserving deterministic invalidation behavior.
Changes:
- Queues dependent signature computations concurrently with deduplication.
- Corrects cached signature-change reporting.
- Adds regression tests, baselines, and a performance benchmark.
| File | Description |
|---|---|
tsc/internal/execute/incremental/affectedfileshandler.go |
Implements parallel signature traversal and caching changes. |
tsc/internal/execute/incremental/affectedfileshandler_test.go |
Adds an incremental rebuild benchmark. |
tsc/internal/execute/incremental/affectedfileshandler_internal_test.go |
Tests cached signature-change behavior. |
tsc/internal/execute/tsctests/tsc_test.go |
Adds incremental and watch regression scenarios. |
tsc/testdata/baselines/reference/tsc/incremental/shared-dependency-with-inferred-types.js |
Records incremental compiler output. |
tsc/testdata/baselines/reference/tscWatch/incremental/shared-dependency-with-inferred-types.js |
Records watch-mode output. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
The limit only covered files reached through references, so a rebuild with many directly changed files still started all of their declaration emits at once. Take the semaphore inside computeDtsSignature instead.
There was a problem hiding this comment.
It's cute to use the existing workgroup for this - but actually, if we need to limit parallelism for performance, why not limit the original workgroup size instead of taking a semaphore for just this path? Feels like we should be able to swap the singleThreaded parameter to a concurrency parameter that takes a max-parallel-threads count (0=infinity, I suppose) and do the limiting in the core concurrency construct.
|
Makes sense, done in 7f0e5e1. The limit is per group now, not process-wide, so with Time and memory on the reproducer are the same as with the semaphore, about 1.5 s and 375 MiB. |
After an edit to a widely imported file, the incremental builder computes the declaration signature of every file that references it, to find what is affected. It did that one file at a time, so this phase ran on about one core. Each referencing file is now its own task in the existing work group.
This is the second half of #64469 (for #64464), which removed the checker scan that took most of the time.
Most of this change was generated with AI tooling; I have hit this problem myself and have reviewed the result.
The 6,000-leaf reproducer from #64464, first rebuild after a comment-only edit to
hub.tswithnoEmitandincremental. 32-core Linux, median of 5 runs,mainalready includes #64469.The
tsbuildinfoafter the rebuild is byte-identical to main's. The gain is smaller on a real project: on n8n'spackages/nodes-basethe same kind of edit to a file that 433 files reference goes from 5.4 s to 4.5 s, and an edit to its most referenced file takes about 2.5 s either way.Signatures in flight are capped at
GOMAXPROCSto keep memory down: after an edit to all 6,000 leaves the rebuild peaks at 377 MiB against 456 MiB on main, in the same 1.5 s. The cap lives in the work group.core.NewWorkGroupnow takes a concurrency instead ofsingleThreaded(0 = unlimited), which is why so many call sites change. The existing ones pass 1 or 0 and behave as before, only the two groups incollectAllAffectedFilesget a cap. It is per group, sotsc -bbuilding several projects at once can exceed it in total. main has no cap there at all.The speedup also depends on the number of checkers, since a signature needs its file's checker. The default is four, and with
--checkers 8the rebuild takes 1.2 s. There is nothing to gain underisolatedModules, where referencing files are not walked.Separately,
updateShapeSignaturereturned false when another traversal had already computed the file's signature. It now returns whether the signature changed. On main the old answer makes the result depend on which changed file is processed first: in a case like the one inTestUpdateShapeSignatureCachedResult, with--singleThreaded20 of 40 runs wrote a differenttsbuildinfothan the rest, and withassumeChangesOnlyAffectDirectDependencies23 of 40 missed the resulting error. This does not depend on the parallelism, so I can split it out if that is easier to review.