Conversation
c2575c7 to
44c8f16
Compare
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 9203e0f:
|
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/46832/ |
| @@ -0,0 +1,39 @@ | |||
| const { transformSync } = require("@babel/core"); | |||
There was a problem hiding this comment.
We deviates from the conventional Gulp stream approach because:
- Passing the source code to sub process is more expensive than passing the path
- Passing the transformed code to main process in order to written via
gulp.destis expensive and ultimately counters the performance gain from multiple process
So we end up creating our own gulp-babel + gulp-newer in this file.
| ); | ||
|
|
||
| gulp.task("build-babel", () => buildBabel(/* exclude */ libBundles)); | ||
| gulp.task("build-babel", () => buildBabel(true, /* exclude */ libBundles)); |
There was a problem hiding this comment.
guess rename this to ignore?
| gulp.task("build-babel", () => buildBabel(true, /* exclude */ libBundles)); | |
| gulp.task("build-babel", () => buildBabel(true, /* ignore */ libBundles)); |
| } | ||
| } | ||
| const srcStat = statSync(src); | ||
| return srcStat.mtimeMs > destStat.mtimeMs; |
There was a problem hiding this comment.
were you able to test watching/incremental, wondering how effective this is? wondering whether we can reuse chokidar here too or is that slow?
There was a problem hiding this comment.
We can combine it with an incremental watch pipeline, so only changed files are compiled. I can land it in a separate PR.
The stat comparison here is essentially a replacement for gulp-newer so when we run gulp build-no-bundle when we already have lib/**, we don't re-compile the compiled sources.
| numWorkers, | ||
| exposedMethods: ["transform"], | ||
| }); | ||
| worker.getStdout().on("data", chunk => process.stdout.write(chunk)); |
There was a problem hiding this comment.
Does worker.getStdout().pipe(process.stdout) work?
f76e1aa to
71fd2e9
Compare
|
Note: This PR is also considered a prototype of future |
| function buildBabel(exclude) { | ||
| const base = monorepoRoot; | ||
| function createWorker(useWorker) { | ||
| const numWorkers = require("os").cpus().length / 2 - 1; |
There was a problem hiding this comment.
I imagine we can/might want to make this configurable at some point? (Like the --maxWorkers flag in jest)
There was a problem hiding this comment.
Yeah. Currently the default value is assuming the CPU has hyper-threading so it havles and minus the main thread. I tested different values locally, based on our scenario, increasing the numWorkers more than that does not yield faster performance.
Co-authored-by: Brian Ng <bng412@gmail.com>
This PR replaces
gulp-babelby ajest-workerpowered Babel farm. On a 4C8U machine, the time spent onyarn gulp build-no-bundlehas improved from 12 seconds to 8.5 seconds.Note: I also tried the worker-thread based piscinia on this branch. However the worker_threads method is slower (16 seconds) than single thread, which is expected as the build process contains many filesystem IO.