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

build: use a single top-level tsc -b for all packages #3090

Merged
merged 7 commits into from
Nov 24, 2022
Merged

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Nov 23, 2022

This PR is an attempt to speed up our build process using tsc with TypeScript project references.

Note
All numbers listed in this PR are from my Apple M1 Max MacBook.

Motivation

In order to build Electron Forge, we currently run two separate build steps:

  • For type declarations (.d.ts): we run tsc --emitDeclarationsOnly on each package in the monorepo with lerna exec.
  • For JavaScript: we run SWC on each package in the monorepo with lerna exec. This is aliased to build:fast for JS-only compilation.

Note
Because TypeScript declaration generation requires a full type checker, it is impossible to perform with SWC as of November 2022. However, this seems to be somewhere on the SWC roadmap (ref swc-project/swc#657).

The problem here is that tsc is slow despite swc being fast, meaning that although it takes about 1-2 seconds to compile our TypeScript to JavaScript, it also takes over 10 seconds to add the associated type declarations.

Changes

The good: adding project references

In order to speed up our tsc usage, I decided to try adding TypeScript project references to our monorepo setup. I loosely followed the sample project from RyanCavanaugh/learn-a to get set up with Lerna + project references.

Ultimately, this means that we can run tsc -b once and get incremental builds! From my testing, this drastically speeds up the full build process for warm builds.

Note
As a bonus, you can now run yarn build:watch to get a watch mode for development!

The bad: removing SWC

Unfortunately, it seems like it's impossible to run tsc -b with --emitDeclarationsOnly as of TypeScript 4.9, which means that we can't actually generate type declarations only with the quicker tsc method, meaning that an extra SWC step becomes redundant.

(However, an unreleased PR microsoft/TypeScript#51241 has landed that will enable us to test this out in the future.)

Note
Aside: I tried looking up if SWC supported project references at all so we could avoid using lerna exec and the answer was "no for now" (ref swc-project/swc#2156). However, since the SWC step isn't a bottleneck, I didn't really look too deep into it.

Testing

To give a clearer picture of the perf numbers, I wanted to run some janky benchmarks. YMMV on how useful these numbers are beyond my specific machine, though.

I tested three separate commands:

  • The current yarn build (tsc + SWC)
  • This PR's tsc -b command (tsc only)
  • The current yarn build:fast (only tested for the warm builds because it blows tsc -b out the water for cold build)

I also tested three different scenarios:

  1. Cold build: running the build command from a clean checkout.
  2. Warm build (small PR/4 LOC change): running the build command after alternating between fix(publisher-ers): properly publish non-default flavours #3079 and the trunk.
  3. Warm build (medium PR/139 LOC change): running the build command after alternating between feat(publisher-gcs): add Google Cloud Storage publisher #2100 and the trunk.

I then ran each combination of command x scenario 10 times:

1 2 3
build tsc -b build build:fast tsc -b build build:fast tsc -b
30.50s 25.21s 13.46s 1.78s 1.74s 14.33s 1.44s 3.75s
28.59s 24.40s 14.49s 1.44s 1.65s 13.57s 1.50s 2.61s
30.63s 23.40s 14.96s 1.47s 1.55s 13.59s 1.50s 3.62s
28.77s 23.34s 13.27s 1.50s 1.55s 13.73s 1.48s 3.62s
28.59s 23.18s 14.11s 1.53s 1.65s 14.18s 1.49s 2.58s
28.58s 23.45s 13.45s 1.45s 1.60s 14.13s 1.45s 3.60s
28.51s 22.78s 14.54s 1.51s 1.58s 14.41s 1.46s 2.54s
28.42s 22.70s 12.92s 1.46s 1.58s 13.76s 1.48s 3.63s
30.22s 22.56s 13.79s 1.49s 1.57s 14.51s 1.43s 2.60s
29.75s 22.55s 13.67s 1.44s 1.65s 12.83s 1.50s 3.60s
Average 29.15s 23.45s 13.89s 1.51s 1.61s 14.23s 1.47s 3.17s

Conclusion

Some conclusions I can derive from the numbers:

  • Cold builds are a bit faster.
  • Incremental builds help a lot. You get almost 10x build speeds for single-digit LOC changes.
  • As the size of changes goes up, tsc -b speed lags behind SWC only, but also includes full type declaration generation.

I guess whether or not this PR should land or not should depend on how we value usage of build:fast vs having way quicker full builds.

@erickzhao erickzhao changed the title Stream "Anti-Hero" by Taylor Swift build: use tsc -b Nov 23, 2022
@erickzhao erickzhao changed the title build: use tsc -b build: use a tsc -b for all commands Nov 23, 2022
@erickzhao erickzhao changed the title build: use a tsc -b for all commands build: use a single top-level tsc -b for all packages Nov 23, 2022
@erickzhao erickzhao marked this pull request as ready for review November 23, 2022 19:44
@erickzhao erickzhao requested a review from a team November 23, 2022 19:51
@@ -4,7 +4,7 @@
"target": "es2019",
"outDir": "dist",
"lib": ["dom", "es2019"],
"sourceMap": true,
Copy link
Member Author

Choose a reason for hiding this comment

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

This matches the behaviour from the .swcrc that we used in production.

name: pkg.name,
manifest: pkg,
});
const subDirPath = path.resolve(PACKAGES_DIR, subDir);
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to assume that anything in the packages/ folder was a folder as well. Here, I add a little isDirectory() check to make sure that we don't assume package/foo is a folder.

Copy link
Member

@malept malept left a comment

Choose a reason for hiding this comment

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

Sad to see swc go, but the numbers speak for themselves.

@erickzhao erickzhao merged commit 51fcc92 into main Nov 24, 2022
@erickzhao erickzhao deleted the heavens-on-fire branch November 24, 2022 18:01
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