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
Faster babel build #13423
Faster babel build #13423
Changes from 6 commits
03ba9e3
9adecfb
925303d
5095936
9ed7880
71fd2e9
9203e0f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -5,11 +5,8 @@ import { fileURLToPath } from "url"; | |||||
import plumber from "gulp-plumber"; | ||||||
import through from "through2"; | ||||||
import chalk from "chalk"; | ||||||
import newer from "gulp-newer"; | ||||||
import babel from "gulp-babel"; | ||||||
import fancyLog from "fancy-log"; | ||||||
import filter from "gulp-filter"; | ||||||
import revertPath from "gulp-revert-path"; | ||||||
import gulp from "gulp"; | ||||||
import { rollup } from "rollup"; | ||||||
import { babel as rollupBabel } from "@rollup/plugin-babel"; | ||||||
|
@@ -21,6 +18,8 @@ import rollupReplace from "@rollup/plugin-replace"; | |||||
import { terser as rollupTerser } from "rollup-plugin-terser"; | ||||||
import _rollupDts from "rollup-plugin-dts"; | ||||||
const { default: rollupDts } = _rollupDts; | ||||||
import { Worker as JestWorker } from "jest-worker"; | ||||||
import glob from "glob"; | ||||||
|
||||||
import rollupBabelSource from "./scripts/rollup-plugin-babel-source.js"; | ||||||
import formatCode from "./scripts/utils/formatCode.js"; | ||||||
|
@@ -75,13 +74,6 @@ function getIndexFromPackage(name) { | |||||
} | ||||||
} | ||||||
|
||||||
function compilationLogger() { | ||||||
return through.obj(function (file, enc, callback) { | ||||||
fancyLog(`Compiling '${chalk.cyan(file.relative)}'...`); | ||||||
callback(null, file); | ||||||
}); | ||||||
} | ||||||
|
||||||
function errorsLogger() { | ||||||
return plumber({ | ||||||
errorHandler(err) { | ||||||
|
@@ -222,33 +214,46 @@ function getFiles(glob, { include, exclude }) { | |||||
return stream; | ||||||
} | ||||||
|
||||||
function buildBabel(exclude) { | ||||||
const base = monorepoRoot; | ||||||
function createWorker(useWorker) { | ||||||
const numWorkers = require("os").cpus().length / 2 - 1; | ||||||
if (numWorkers === 0 || !useWorker) { | ||||||
return require("./babel-worker.cjs"); | ||||||
} | ||||||
const worker = new JestWorker(require.resolve("./babel-worker.cjs"), { | ||||||
numWorkers, | ||||||
exposedMethods: ["transform"], | ||||||
}); | ||||||
worker.getStdout().pipe(process.stdout); | ||||||
worker.getStderr().pipe(process.stderr); | ||||||
return worker; | ||||||
} | ||||||
|
||||||
return getFiles(defaultSourcesGlob, { | ||||||
exclude: exclude && exclude.map(p => p.src), | ||||||
}) | ||||||
.pipe(errorsLogger()) | ||||||
.pipe(newer({ dest: base, map: mapSrcToLib })) | ||||||
.pipe(compilationLogger()) | ||||||
.pipe( | ||||||
babel({ | ||||||
caller: { | ||||||
// We have wrapped packages/babel-core/src/config/files/configuration.js with feature detection | ||||||
supportsDynamicImport: true, | ||||||
}, | ||||||
}) | ||||||
) | ||||||
.pipe( | ||||||
// gulp-babel always converts the extension to .js, but we want to keep the original one | ||||||
revertPath() | ||||||
) | ||||||
.pipe( | ||||||
// Passing 'file.relative' because newer() above uses a relative | ||||||
// path and this keeps it consistent. | ||||||
rename(file => path.resolve(file.base, mapSrcToLib(file.relative))) | ||||||
) | ||||||
.pipe(gulp.dest(base)); | ||||||
async function buildBabel(useWorker, ignore = []) { | ||||||
const worker = createWorker(useWorker); | ||||||
const files = await new Promise((resolve, reject) => { | ||||||
glob( | ||||||
defaultSourcesGlob, | ||||||
{ | ||||||
ignore: ignore.map(p => `./${p.src}/**`), | ||||||
}, | ||||||
(err, files) => { | ||||||
if (err) reject(err); | ||||||
resolve(files); | ||||||
} | ||||||
); | ||||||
}); | ||||||
|
||||||
const promises = []; | ||||||
for (const file of files) { | ||||||
// @example ./packages/babel-parser/src/index.js | ||||||
const dest = "./" + mapSrcToLib(file.slice(2)); | ||||||
promises.push(worker.transform(file, dest)); | ||||||
} | ||||||
return Promise.all(promises).finally(() => { | ||||||
if (worker.end !== undefined) { | ||||||
worker.end(); | ||||||
} | ||||||
}); | ||||||
} | ||||||
|
||||||
/** | ||||||
|
@@ -517,7 +522,7 @@ gulp.task( | |||||
gulp.series("copy-dts", () => buildRollupDts(dtsBundles)) | ||||||
); | ||||||
|
||||||
gulp.task("build-babel", () => buildBabel(/* exclude */ libBundles)); | ||||||
gulp.task("build-babel", () => buildBabel(true, /* exclude */ libBundles)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. guess rename this to ignore?
Suggested change
|
||||||
|
||||||
gulp.task( | ||||||
"build", | ||||||
|
@@ -536,7 +541,10 @@ gulp.task( | |||||
|
||||||
gulp.task("default", gulp.series("build")); | ||||||
|
||||||
gulp.task("build-no-bundle", () => buildBabel()); | ||||||
// First build on worker processes for complilation speed | ||||||
JLHwung marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
gulp.task("build-no-bundle", () => buildBabel(true)); | ||||||
// Incremental builds take place in main process | ||||||
gulp.task("build-no-bundle-watch", () => buildBabel(false)); | ||||||
|
||||||
gulp.task( | ||||||
"build-dev", | ||||||
|
@@ -556,7 +564,7 @@ gulp.task( | |||||
gulp.task( | ||||||
"watch", | ||||||
gulp.series("build-dev", function watch() { | ||||||
gulp.watch(defaultSourcesGlob, gulp.task("build-no-bundle")); | ||||||
gulp.watch(defaultSourcesGlob, gulp.task("build-no-bundle-watch")); | ||||||
gulp.watch( | ||||||
babelStandalonePluginConfigGlob, | ||||||
gulp.task("generate-standalone") | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
const { transformSync } = require("@babel/core"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We deviates from the conventional Gulp stream approach because:
So we end up creating our own gulp-babel + gulp-newer in this file. |
||
const { mkdirSync, statSync, readFileSync, writeFileSync } = require("fs"); | ||
const { dirname } = require("path"); | ||
const chalk = require("chalk"); | ||
const fancyLog = require("fancy-log"); | ||
|
||
function needCompile(src, dest) { | ||
let destStat; | ||
try { | ||
destStat = statSync(dest); | ||
} catch (err) { | ||
if (err.code === "ENOENT") { | ||
return true; | ||
} else { | ||
throw err; | ||
} | ||
} | ||
const srcStat = statSync(src); | ||
return srcStat.mtimeMs > destStat.mtimeMs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
exports.transform = function (src, dest) { | ||
mkdirSync(dirname(dest), { recursive: true }); | ||
if (!needCompile(src, dest)) { | ||
return; | ||
} | ||
fancyLog(`Compiling '${chalk.cyan(src)}'...`); | ||
const content = readFileSync(src, { encoding: "utf8" }); | ||
const { code } = transformSync(content, { | ||
filename: src, | ||
caller: { | ||
// We have wrapped packages/babel-core/src/config/files/configuration.js with feature detection | ||
supportsDynamicImport: true, | ||
name: "babel-worker", | ||
}, | ||
}); | ||
|
||
writeFileSync(dest, code, "utf8"); | ||
}; |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.