Skip to content
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

Multithreaded compilation via Node workers and threads option or CIVET_THREADS environment variable; enable Node compiler cache; improve unplugin caching #1646

Merged
merged 8 commits into from
Dec 18, 2024

Conversation

edemaine
Copy link
Collaborator

@edemaine edemaine commented Dec 15, 2024

The main goal was to make Civet build faster. On my CPU (Core i9-14900KF), this normally takes about 10 seconds. With this PR, it gets down to 3.2 seconds.

Improve unplugin caching

The unplugin has a cache that avoids recompiling .civet files if they haven't changed. I enabled this cache in source/esbuild.civet. I wonder if it should be enabled by default?

This should have helped for ESM + CommonJS + browser builds, as we do when building Civet. But in an esbuild context, all of these builds attempt to run in parallel, so all three builds typically start before any one completes. So the cache didn't get utilized.

So I modified the cache to set immediately when the job starts, with a Promise to await if a second job attempts to compile the same file. This avoids the repeated compilation work.

Already the Civet build time reduces to 7.4 seconds. Because of a bug with esbuild alias, we can't use the unplugin yet for the browser build. But I also fixed this bug, so once we upgrade the Civet embedded in Civet, we will reduce the build time to 5.2 seconds!

I wondered if we should similar caching in Hera's esbuild plugin, but currently each Hera compilation costs only ~15ms, so this would only save us ~30ms. It might be important once we add Civet support to Hera's code blocks.

Parallel compilation via Node workers

That's what I should have done first, but what I actually spent most of my time was making Civet's compile function automatically parallelize by spawning the desired number of Node workers. This feature can be enabled in two ways:

  • CIVET_THREADS environment variable. If set to a positive number, enables up to that number of Node workers, spawned as needed.
  • threads compiler option, which can be set in the config file or passed into compile.

With this, the build time (via CIVET_THREADS=n yarn build) reduces a bit more:

threads time
1 5.2 seconds
2 3.6 seconds
3 3.3 seconds
4 3.2 seconds
5+ ~3.2 seconds

Unfortunately, the parallelism seems to max out around here. I think at this point we're bottlenecked by the largest Civet source file.

As some evidence in this direction, here are some big chunks of the build time that we're not trying to optimize here:

  • terser (which we use for civet.dev) takes about 1.4 seconds.
  • TypeScript (which we use to build the unplugin's .d.ts declaration files) takes about 0.4 seconds, only 0.2 of which can be done in parallel with Civet builds.

Remaining question

Should we turn on the unplugin's cache option by default? I can't think of a bad case for this. Previously it wasn't helpful except in watch mode, but now I think it's generally quite helpful, for anyone doing multiple bundles from the same source code. And in watch mode, I think you'd want it on too.

Copy link
Contributor

@STRd6 STRd6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Defaulting to cache: true sounds good as well. Thanks!

@edemaine edemaine merged commit 74e14a1 into main Dec 18, 2024
3 checks passed
@edemaine edemaine deleted the speed branch December 18, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants