From 2548a6cd6c9e234207b631bc6da6b4dafe9e4f24 Mon Sep 17 00:00:00 2001 From: Takuto Ikuta Date: Tue, 1 Feb 2022 21:10:02 +0900 Subject: [PATCH] introduce devtools_fast_bundle build config Proposal: http://go/devtools-fast-bundle This CL introduces build flag switching bundler from rollup.js to esbuild by * adding esbuild to npm without downloading binary packages * making devtools_plugin for rollup.js re-usable to esbuild On 24C/48T Z840 Linux machine, this shows following performance difference by using ``` devtools_skip_typecheck = true is_debug = false ``` as base build config. esbuild (devtools_fast_bundle = true) $ time ninja -C out/Default/ ... real 0m21.174s user 2m47.513s sys 0m38.549s rollup.js (devtools_fast_bundle = false) $ time ninja -C out/Default/ ... real 1m28.286s user 30m19.220s sys 5m36.392s So esbuild is 3.2x faster and use only 9.6% of machine resouce (user + sys) compared to rollup.js. refs: * https://esbuild.github.io/plugins/#on-resolve * https://rollupjs.org/guide/en/#resolveid Bug: 1278663 Cq-Include-Trybots: luci.devtools-frontend.try:devtools_frontend_linux_blink_light_rel_fastbuild,devtools_frontend_linux_dbg_fastbuild Change-Id: If6b2e774f48091b0fe9c959e7ed1ed9bc2b0847c Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3401984 Reviewed-by: Tim Van der Lippe Commit-Queue: Takuto Ikuta --- scripts/build/devtools_plugin.js | 10 ++++ scripts/build/esbuild.js | 62 +++++++++++++++++++ scripts/build/ninja/bundle.gni | 100 ++++++++++++++++++++++--------- 3 files changed, 144 insertions(+), 28 deletions(-) create mode 100644 scripts/build/esbuild.js diff --git a/scripts/build/devtools_plugin.js b/scripts/build/devtools_plugin.js index 0d47d7fae7..4c9fa03dd1 100644 --- a/scripts/build/devtools_plugin.js +++ b/scripts/build/devtools_plugin.js @@ -42,6 +42,16 @@ function devtoolsPlugin(source, importer) { if (!importer) { return null; } + + if (source === '../../lib/codemirror' || source === 'fs') { + // These are imported via require(...), but we don't use + // @rollup/plugin-commonjs. So this check is not necessary for rollup. But + // need to have this for esbuild as it doesn't ignore require(...). + return { + external: true, + }; + } + const currentDirectory = path.normalize(dirnameWithSeparator(importer)); const importedFilelocation = path.normalize(path.join(currentDirectory, source)); const importedFileDirectory = dirnameWithSeparator(importedFilelocation); diff --git a/scripts/build/esbuild.js b/scripts/build/esbuild.js new file mode 100644 index 0000000000..ee5771a7df --- /dev/null +++ b/scripts/build/esbuild.js @@ -0,0 +1,62 @@ +// Copyright 2022 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @ts-check + +const path = require('path'); + +const devtools_paths = require('../devtools_paths.js'); +const devtools_plugin = require('./devtools_plugin.js'); + +// esbuild module uses binary in this path. +process.env.ESBUILD_BINARY_PATH = path.join(devtools_paths.devtoolsRootPath(), 'third_party', 'esbuild', 'esbuild'); + +const entryPoints = [process.argv[2]]; +const outfile = process.argv[3]; + +const outdir = path.dirname(outfile); + +const plugin = { + name: 'devtools-plugin', + setup(build) { + // https://esbuild.github.io/plugins/#on-resolve + build.onResolve({filter: /.*/}, args => { + const res = devtools_plugin.devtoolsPlugin(args.path, args.importer); + if (!res) { + return null; + } + + if (res.external && res.id) { + return { + external: res.external, + path: './' + path.relative(outdir, res.id), + }; + } + + if (res.external) { + return { + external: true, + }; + } + + return { + path: res.id, + }; + }); + }, +}; + +require('esbuild') + .build({ + entryPoints, + outfile, + bundle: true, + format: 'esm', + platform: 'browser', + plugins: [plugin], + }) + .catch(err => { + console.error('failed to run esbuild:', err); + process.exit(1); + }); diff --git a/scripts/build/ninja/bundle.gni b/scripts/build/ninja/bundle.gni index 0f82b4bf31..94cc8e3974 100644 --- a/scripts/build/ninja/bundle.gni +++ b/scripts/build/ninja/bundle.gni @@ -5,43 +5,87 @@ import("./node.gni") import("./vars.gni") +declare_args() { + # If this is enabled, devtools build uses esbuild instead of rollup.js to + # bundle JavaScript files. + devtools_fast_bundle = false +} + +assert(!(devtools_fast_bundle && is_official_build), + "Official build should not bundle with esbuild") + template("bundle") { assert(defined(invoker.entrypoint), "You must define the 'entrypoint' for a bundle target") - node_action(target_name) { - script = "node_modules/rollup/dist/bin/rollup" + if (devtools_fast_bundle) { + node_action(target_name) { + script = "scripts/build/esbuild.js" + forward_variables_from(invoker, + [ + "visibility", + "deps", + "public_deps", + ]) - forward_variables_from(invoker, - [ - "visibility", - "deps", - "public_deps", - ]) + inputs = [ + invoker.entrypoint, + devtools_location_prepend + "scripts/build/devtools_plugin.js", + devtools_location_prepend + "scripts/devtools_paths.js", + ] - inputs = [ - invoker.entrypoint, - devtools_location_prepend + "scripts/build/rollup.config.js", - ] + _esbuild = devtools_location_prepend + "third_party/esbuild/esbuild" + if (host_os == "win") { + inputs += [ _esbuild + ".exe" ] + } else { + inputs += [ _esbuild ] + } - args = [ - # TODO(crbug.com/1098074): We need to hide warnings that are written stderr, - # as Chromium does not process the returncode of the subprocess correctly - # and instead looks if `stderr` is empty. - "--silent", - "--config", - rebase_path(devtools_location_prepend + "scripts/build/rollup.config.js", - root_build_dir), - "--input", - rebase_path(invoker.entrypoint, root_build_dir), - "--file", - rebase_path(invoker.output_file_location, root_build_dir), - ] + args = [ + rebase_path(invoker.entrypoint, root_build_dir), + rebase_path(invoker.output_file_location, root_build_dir), + ] - if (!devtools_dcheck_always_on) { - args += [ "--configDCHECK" ] + outputs = [ invoker.output_file_location ] } + } else { + node_action(target_name) { + script = "node_modules/rollup/dist/bin/rollup" - outputs = [ invoker.output_file_location ] + forward_variables_from(invoker, + [ + "visibility", + "deps", + "public_deps", + ]) + + inputs = [ + invoker.entrypoint, + devtools_location_prepend + "scripts/build/rollup.config.js", + devtools_location_prepend + "scripts/build/devtools_plugin.js", + devtools_location_prepend + "scripts/devtools_paths.js", + ] + + args = [ + # TODO(crbug.com/1098074): We need to hide warnings that are written stderr, + # as Chromium does not process the returncode of the subprocess correctly + # and instead looks if `stderr` is empty. + "--silent", + "--config", + rebase_path( + devtools_location_prepend + "scripts/build/rollup.config.js", + root_build_dir), + "--input", + rebase_path(invoker.entrypoint, root_build_dir), + "--file", + rebase_path(invoker.output_file_location, root_build_dir), + ] + + if (!devtools_dcheck_always_on) { + args += [ "--configDCHECK" ] + } + + outputs = [ invoker.output_file_location ] + } } }