Skip to content

Commit

Permalink
Use esbuild for javascript bundling. (tensorflow#5829)
Browse files Browse the repository at this point in the history
* Motivation for features / changes

  The upcoming Angular 13 upgrade breaks our custom tf_dev_js_binary rule, which we used for fast, unminified bundling of the tensorboard:dev target.

* Technical description of changes

  One straighforward solution is to move all of our javascript bundling to the esbuild bundler. It is just as fast as our custom bundling in tf_dev_js_binary and much faster than the rollup+terser combination we've been using for bundling our production build. The bundles it generates also tend to be smaller than the bundles we are currently generating.

  By using esbuild in our tf_js_binary rule we can migrate all bundling to this single rule and eliminate tf_dev_js_binary rule. The only difference between a "prod" and "dev" target is whether we choose to minify or not. We can also remove rollup and terser from our project.

  See: https://esbuild.github.io/

  Note that bazel documents how to use esbuild with ts_project but not with ts_library. We managed to get it to work but its long-term support is unknown.

  https://www.npmjs.com/package/@bazel/esbuild

* Detailed steps to verify changes work correctly (as executed by you)

  For core TensorBoard app:
  * `bazel run tensorboard` and `bazel run tensorboard:dev`.
  * Simple check of 'Time Series', 'Scalars', 'Images', 'Graphs', 'Distributions', 'Histograms', 'Text', and 'Hparams'. 
  * Set 'pagination limit' in the settings and ensure it is honored by the 'Time Series' and 'Images' plugins - this ensures the angular/polymer bridge is working.
  * Set 'ignore outliers in chart scaling' in both 'Time Series' and 'Scalars' plugins and refresh and verify the changes are honored. - this ensures integration with local storage is still working.
  * Check the 'Debugger' plugin and the Monaco code editor is working within it.
  
  I also verified that other binaries using tf_js_binary rule still seem to work:
  * chart_worker.js
  * tf_graph_app
  * vz_projector
  * tensor_widget (could only verify it builds, couldn't figure out how to render it)

  I checked that bundle times are noticeably faster.
  * for the prod build of tb_webapp_binary.js, the rollup+terser bundling could be 20s to 30s. but with esbuild it is just 1s.
  * for the dev build of tb_webapp_binary.js, there is not really any noticeable performance gain.

  I checked bundle sizes. They actually all end up being smaller:
  * At commit 0f4d6e9
  * For the production app with minified binaries:
    * bazel-bin/tensorboard/webapp/tb_webapp_binary.js 4378795 (vs 4464436)
    * bazel-bin/tensorboard/components/polymer3_lib_binary.js 3166751 (vs 3177827)
    * bazel-bin/tensorboard/webapp/index.js 7631949 (vs 7728765)
  * For the dev app with unminified binaries:
    * bazel-bin/tensorboard/webapp/dev_assets/tb_webapp_binary.js 7190752 (vs 26059122)
    * bazel-bin/tensorboard/webapp/dev_assets/polymer3_lib_binary_dev.js 6124163 (vs 8121074)
  • Loading branch information
bmd3k authored and dna2github committed May 1, 2023
1 parent 9ba9fd0 commit 5c36d11
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 442 deletions.
6 changes: 6 additions & 0 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ yarn_install(
yarn_lock = "//:yarn.lock",
)

# Load esbuild rules for bazel.
# https://bazelbuild.github.io/rules_nodejs/esbuild.html
load("@build_bazel_rules_nodejs//toolchains/esbuild:esbuild_repositories.bzl", "esbuild_repositories")

esbuild_repositories(npm_repository = "npm")

http_archive(
name = "io_bazel_rules_sass",
sha256 = "ee6d527550d42af182673c3718da98bb9205cabdeb08eacc0e3767fa3f2b051a",
Expand Down
7 changes: 1 addition & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,10 @@
"@angular/compiler": "^12.2.0",
"@angular/compiler-cli": "^12.2.0",
"@bazel/concatjs": "^4.6.1",
"@bazel/esbuild": "^4.6.2",
"@bazel/ibazel": "^0.15.9",
"@bazel/jasmine": "^4.6.1",
"@bazel/rollup": "^4.6.1",
"@bazel/terser": "^4.6.1",
"@bazel/typescript": "^4.6.1",
"@rollup/plugin-commonjs": "^20.0.0",
"@rollup/plugin-node-resolve": "^13.0.4",
"@types/d3": "5.7.2",
"@types/jasmine": "^3.8.2",
"@types/lodash": "^4.14.172",
Expand All @@ -58,8 +55,6 @@
"prettier": "2.4.1",
"prettier-plugin-organize-imports": "2.3.4",
"requirejs": "^2.3.6",
"rollup": "^2.56.2",
"terser": "^5.14.2",
"tslib": "^2.3.0",
"typescript": "4.5.4",
"yarn-deduplicate": "^5.0.0"
Expand Down
5 changes: 0 additions & 5 deletions tensorboard/defs/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ tb_proto_library(
],
)

exports_files([
"rollup_config.js",
"terser_config.json",
])

ts_library(
name = "strict_types",
srcs = ["strict_type_check.d.ts"],
Expand Down
89 changes: 37 additions & 52 deletions tensorboard/defs/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,11 @@
# limitations under the License.
"""External-only delegates for various BUILD rules."""

load("@npm//@bazel/rollup:index.bzl", "rollup_bundle")
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
load("@npm//@bazel/concatjs:index.bzl", "karma_web_test_suite")
load("@npm//@bazel/esbuild:index.bzl", "esbuild")
load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_library")
load("@io_bazel_rules_sass//:defs.bzl", "npm_sass_library", "sass_binary", "sass_library")
load("@npm//@bazel/terser:index.bzl", "terser_minified")
load("//tensorboard/defs/internal:js.bzl", _tf_dev_js_binary = "tf_dev_js_binary")

tf_dev_js_binary = _tf_dev_js_binary

def tensorboard_webcomponent_library(**kwargs):
"""Rules referencing this will be deleted from the codebase soon."""
Expand All @@ -29,7 +26,6 @@ def tensorboard_webcomponent_library(**kwargs):
def tf_js_binary(
name,
compile,
deps,
visibility = None,
dev_mode_only = False,
includes_polymer = False,
Expand All @@ -39,61 +35,50 @@ def tf_js_binary(
Args:
name: Name of the target.
compile: whether to compile when bundling. Only used internally.
deps: dependencies of the js_binary.
visibility: visibility of the target.
dev_mode_only: whether the binary is for development. When True, it will
omit the Terser.
omit the minification step.
includes_polymer: whether this binary contains Polymer. Only used
internally.
**kwargs: keyword arguments to rollup_bundle. Please refer to
https://bazelbuild.github.io/rules_nodejs/Built-ins.html#rollup_bundle
**kwargs: Other keyword arguments to esbuild(). Typically used for
entry_point and deps. Please refer to https://esbuild.github.io/api/
for more details.
"""

# `compile` option is used internally but is not used by rollup_bundle.
# Discard it.
internal_rollup_name = name + "_rollup_internal_dbg"
rollup_bundle(
name = internal_rollup_name,
config_file = "//tensorboard/defs:rollup_config.js",
# Must pass `true` here specifically, else the input file argument to
# Rollup (appended by `rollup_binary`) is interpreted as a value for
# the preceding option.
args = ["--failAfterWarnings", "true", "--silent", "true"],
deps = deps + [
"@npm//@rollup/plugin-commonjs",
"@npm//@rollup/plugin-node-resolve",
],
format = "iife",
sourcemap = "false",
visibility = ["//visibility:private"],
**kwargs
)

if dev_mode_only:
internal_result_name = internal_rollup_name
else:
internal_result_name = name + "_terser_internal_min"
terser_minified(
name = internal_result_name,
src = internal_rollup_name,
# Notes about the terser config:
# compress.passes - this is set to '1' to workaround issue with
# terser and threejs. In practice it (surprisingly) generates
# smaller results than when it was previously set to '3'.
config_file = "//tensorboard/defs:terser_config.json",
visibility = ["//visibility:private"],
sourcemap = False,
)

# For some reason, terser_minified is not visible from other targets. Copy
# or re-export seems to work okay.
native.genrule(
# esbuild is a fast JavaScript bundler[1] appropriate for both production
# and development builds.
#
# Bazel documents[2] how to use esbuild bundling with ts_project but we use
# the not-quite-deprecated ts_library rule instead of ts_project. We've
# managed to get esbuild working with ts_library but its long-term support
# is unknown.
#
# [1]: https://esbuild.github.io/
# [2]: https://www.npmjs.com/package/@bazel/esbuild
esbuild(
name = name,
srcs = [internal_result_name],
outs = [name + ".js"],
visibility = visibility,
cmd = "cat $(SRCS) > $@",
# Use "iife" format instead of "esm" because "esm" writes symbols at
# the global level and tends to overwrite `window` functions. "iife" is
# just a thin wrapper around "esm" (it adds 11 bytes) and doesn't
# suffer from the same overwriting problem.
format="iife",
minify= False if dev_mode_only else True,
args = {
# Must specify that 'mjs' extensions are preferred, since that is
# the extension that is used for es2015/esm code generated by
# ts_library.
# https://github.com/bazelbuild/rules_nodejs/issues/2691#issuecomment-846429871
"resolveExtensions": [".mjs", ".js"],
# The reasoning for these particular mainFields values are lost to
# history. These come from the old rollup bundler configuration.
# We do know that the esbuild default values for mainFields do not
# work for us. In particular we ran into problems with
# esbuild pulling in "node"-specific versions of some libraries that
# are incompatible with browsers.
"mainFields": ["browser", "es2015", "module", "jsnext:main", "main"],
},
**kwargs
)

def tf_ts_config(**kwargs):
Expand Down
149 changes: 0 additions & 149 deletions tensorboard/defs/internal/js.bzl

This file was deleted.

56 changes: 0 additions & 56 deletions tensorboard/defs/rollup_config.js

This file was deleted.

18 changes: 0 additions & 18 deletions tensorboard/defs/terser_config.json

This file was deleted.

0 comments on commit 5c36d11

Please sign in to comment.