-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: consider speeding up compile of large ast package in microsoft/typescript-go #73044
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
Comments
Just to note it, our full main suite of tests (78k in To run the entire repo's tests takes only 11 seconds on my machine (packages in parallel, most tests are in the testrunner package). Most of the bottleneck is just the Go compiler or #72992. |
With all builds cached (using gotestsum for brevity and test counting): $ gotestsum --hide-summary=skipped --format-hide-empty-pkg -- -count=1 ./...
<snip>
DONE 105775 tests, 1051 skipped in 11.748s But if I do $ gotestsum --hide-summary=skipped --format-hide-empty-pkg -- -count=1 ./...
<snip>
DONE 105775 tests, 1051 skipped in 117.253s 100s are just compile time. So, it's likely disabling some optimizations could help us go faster, probably. Of course, improving things without turning off optimizations would be better 😄 |
Just to show it for the "total end to end" calculus, having cleaned my build cache, adding $ gotestsum --hide-summary=skipped --format-hide-empty-pkg -- -gcflags='-l -N' -count=1 ./...
<snip>
DONE 105775 tests, 1051 skipped in 59.638s And a rerun is only 16s or so. Despite turning off optimizations, our large number of tests still run pretty fast. |
Depending on the hardware, my perhaps optimistic hope is that if you ask for more cmd/compile concurrency ( Whether some of the others I included above end up helping you in practice depends on how long the tests take, etc., though it seems plausible it could help in some cases.
I know that was just a quick test (and I think part of the point you were making is that you applied it broadly), but just in case it helps, including the The goal is not to make it complicated for you, but you could scope the flags to specific packages based on what seems to give a useful tradeoff of build time vs. resulting execution time. For example, to just specify that the
Or to say all packages underneath
You can also have different options for different packages (with the last match winning). There's a little more on that here (ctrl-f for
One other quick tip is using |
Ah, I didn't even put together that |
Go version
go version go1.24.1 linux/amd64
What did you do?
Compile the
ast
package of the Go-based TypeScript compiler (https://github.com/microsoft/typescript-go).What did you see happen?
If the changes in #72815 land, it looks like the large
typescript-go/internal/ast
package then becomes the long pole for the overall typescript-go compilation.There might be some opportunities within cmd/compile or cmd/go to speed it up. There might also be some workarounds that could be used today, which might in turn hint towards possible changes within the Go toolchain.
To get some starting data points, I tried compiling the typescript-go
ast
package with Go 1.24.1 to see what might speed it up:(The adjustments here are cumulative. The reported seconds are just for compiling the
ast
package itself, and were measured via-gcflags='-bench=<file>.txt'
and then summarized with benchstat. Peak RSS was measured viatime
).Possible changes to the Go toolchain might include:
Perhaps the default cmd/compile concurrency (
-gcflags=-c=N
) could be increased by default or increased via some heuristic or threshold, such as perhaps a trivial threshold like number of input bytes in a package or function count or ____. (I think I recall older comments maybe by Josh saying that as HW evolves it might make sense to adjust the default of 4 upwards. Some related discussion in cmd/go,cmd/compile: let cmd/compile choose how much concurrency to use #48497).Perhaps cmd/compile could be more aggressive about adjusting
GOGC
. (Probably part of the reason it didn't help too much to disable the GC here is that the compiler is already somewhat aggressive about this. See for example https://go.dev/cl/436235 by David Chase, which introduces a fairly aggressive adjustment toGOGC
, but perhaps that could be revisited for some modest benefit).Perhaps other changes based on examining the current performance.
In terms of workarounds, some of the above flags might be useful today for the TypeScript team (for example, perhaps something like
-gcflags='-c=16'
is useful now if the TypeScript team is not already adjusting that).Others, such as disabling inlining, might waste total wall clock time if for example the immediate next step is to then kick off multiple minutes of regression tests, but some chance it could make sense in other cases if for example the next step is to instead run, say, ~500ms of tests via
go test -short
or similar. (Separately, maybe someone will find it interesting to look at cmd/compile performance with and without inlining enabled for this package -- maybe the resulting performance delta is "expected", or maybe it shows something more interesting).What did you expect to see?
Ideally, it would be faster.
Additional Details
Here is a high-level breakdown of wall clock time spent overall (as reported by
-gcflags=-bench=<file>.txt
) comparing the starting point vs. the final experiment above (and then below that are individual results for each step).$ benchstat 1-base.bench 2-add-c16.bench
$ benchstat 2-add-c16.bench 3-add-disable-gc.bench
$ benchstat 3-add-disable-gc.bench 4-add-disable-inline.bench
$ benchstat 4-add-disable-inline.bench 5-add-disable-opt.bench
CC @jakebailey, @RyanCavanaugh
The text was updated successfully, but these errors were encountered: