-
Notifications
You must be signed in to change notification settings - Fork 812
Some of the changes to improve --build memory footprint #2604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adjusts the --build pipeline to significantly reduce memory usage and make concurrency behavior more predictable, especially for large project graphs. The main themes are tightening what gets cached, pooling writer resources safely, and centralizing build/work concurrency control behind the --builders option.
Changes:
- Rework the build orchestrator to drive all per-project work (watch updates, incremental checks, build/clean) through a shared
rangeTaskscheduler that uses--builders(default 4) or--singleThreadedto determine concurrency. - Refine parsing caches so they are keyed on comparable types, used only where needed (e.g.,
.d.ts/.jsonand resolved project references), and tweak how cached values are reused. - Ensure the emit writer pool in
Program.Emitno longer repeatedly captures state in a way that can retainPrograminstances unnecessarily, and align diagnostics/help text with the new default builder behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/tsoptions/declsbuild.go |
Updates the --builders option’s default value description to “4, unless --singleThreaded is passed”, matching the new concurrency default enforced by the orchestrator. |
internal/execute/build/parseCache.go |
Introduces a parseCache keyed on comparable types with a loadOrStore API that can optionally reuse non-zero cached values, forming the basis for the build host’s source-file and config caches. |
internal/execute/build/orchestrator.go |
Removes the old semaphore-based concurrency control and single-/multi-threaded build entrypoints, replacing them with a unified rangeTask helper that drives updateWatch, DoCycle, and buildOrClean according to SingleThreaded and BuildOptions.Builders. |
internal/execute/build/host.go |
Adjusts the build host to only involve the cache for .d.ts and .json source files and to use the new parseCache.loadOrStore for both source files and resolved project references. |
internal/execute/build/buildtask.go |
Drops direct dependence on an orchestrator-level buildSemaphore, so project compilation/emission concurrency is now governed centrally by the orchestrator’s rangeTask scheduling. |
internal/diagnostics/extraDiagnosticMessages.json |
Removes the extra diagnostic entry for “all, unless --singleThreaded is passed.”, aligning diagnostics with the updated default and avoiding an outdated help string. |
internal/compiler/program.go |
Caches the newline string once per Emit call and uses that in the writer pool’s factory, reducing the risk of the pool retaining Program state and improving memory behavior during emit. |
Notable issues identified (already left as PR comments):
rangeTaskcurrently usesfor range numRoutines, which is not valid Go syntax and will prevent the code from compiling; this needs to be replaced with a counted loop that queuesnumRoutinesworkers.host.GetSourceFilepassesallowNonZero = falseintoparseCache.loadOrStore, which means cached.d.ts/.jsonentries are never actually reused, defeating the stated caching intent and causing unnecessary re-parsing.
|
Would you mind pushing a beta build? I’d love to give it a spin. |
|
We don't have a way to do that; you'd have to build from source |
|
I gave the local build a try, but I'm still running into OOM. tsgo seems to be consuming significantly more memory compared to the previous tsc. |
|
@Zzzen is it possible to share repro. What does extendedDiagnostics show.. |
Sorry there were some mistakes when i restructured code. can you please give it a try. if it still repros, can you please share the repro steps, pprof dump, result with extendedDiagnostics so i can look into it some more |
|
It's a proprietary project, so I can't share the actual code. In TypeScript 5.9 my function export function f(input: DeepReadonly<LargeType>) { … }gets emitted cleanly like this in the .d.ts: export function f(input: DeepReadonly<LargeType>): void;But with tsgo, export function f(input: { prop1: { propA: … }, prop2: ……… }): void;
|
|
Can you try building #2459 and see if that fixes your issue? |
Fixes #1622 (probably)