Use esbuild for javascript bundling. #5829

bmd3k · 2022-07-29T11:10:31Z

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)

esbuild.

else.

JamesHollyer

+64 lines -442 lines!!! This is awesome! Thanks for figuring this out!

I had a few questions and one minor nit suggestion. It looks great!

JamesHollyer · 2022-07-29T18:53:15Z

tensorboard/defs/defs.bzl

@@ -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")


Can you move this back down below these other loads for git blame purposes? Or is there a reason this was moved up?

I moved it up to sort the imports alphabetically.

JamesHollyer · 2022-07-29T19:04:17Z

yarn.lock

@@ -3588,7 +3480,7 @@ [email protected]:
  dependencies:
    sourcemap-codec "^1.4.4"

-magic-string@^0.25.0, magic-string@^0.25.7:


What did "magic-string@^0.25.7" even do?

I don't know but I guess rollup or terser depended on it.

JamesHollyer · 2022-07-29T19:07:57Z

tensorboard/defs/defs.bzl

+    #
+    # 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


Did you have to do anything besides specifying .mjs extensions to get ts_library to work?

That's all I was certain was ts_library-specific.

The change to use the esbuild bundler (#5829) broke the projector plugin (#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

The change to use the esbuild bundler (tensorflow#5829) broke the projector plugin (tensorflow#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

The change to use the esbuild bundler (#5829) broke the projector plugin (#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

The change to use the esbuild bundler (tensorflow#5829) broke the projector plugin (tensorflow#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

* 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)

The change to use the esbuild bundler (tensorflow#5829) broke the projector plugin (tensorflow#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

* 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)

The change to use the esbuild bundler (tensorflow#5829) broke the projector plugin (tensorflow#5924). The root cause is in the 'numeric' library, which we use for calculating the PCA. The library requires that the symbol 'numeric' is available in the global scope when its operations are executed by other modules. See, for example, how the definition of some of its operations refer to the string 'numeric' in the Function definition, unmodifiable by the bundler/minification code: https://github.com/sloisel/numeric/blob/656fa1254be540f428710738ca9c1539625777f1/src/numeric.js#L696 The esbuild bundler does not keep 'numeric' in global scope and instead renames it as part of bundling/minification. We work around this by manually adding it to global scope. We introduce the wrapper tensorboard/webapp/third_party/numeric.ts to add 'numeric' to the global scope and require that tensorboard code import that wrapper rather than importing numeric directly. We add a CI check to ensure that numeric is not imported directly.

Few changes: - upgrade rules_nodejs to 5.7.0 (this will extend the nodejs versions available for installation, while we are installing the default nodejs version here with the 5.7.0 rules_nodejs toolchain) - following this, upgrade all bazel toolchain to 5..7.0 - correspondingly upgrade rules_sass version to 1.55.0 (I can only find rules_sass github codebase with given version tag, but not sure how to find its hash..etc. Here we are basically following tensorboard team's current setup) - upgrade angular (and all relative packages) to v14, correspondingly upgrade ngrx to v14, and bump up rxjs. - upgrade typescript to 4.7.4 - upgrade zone.js to 0.12.0 - import ts_library (used for bazel build) from concatjs instead of typescript, as the module is moved to concatjs. (Also added concatjs into data field of `tsc_wrapped_with_angular` target) - added name field to the output of `rollup.config.js`, as it is a required field now for iife output type - Added `node_modules` into `include_paths` of sass_binary build rule when building the `styles.css` file for frontend. This is because angular v14 changed to import angular/cdk from using a relative path to an new one like `@angular/cdk`, so we need to include the `node_modules` int the the searching paths to enable sassCompiler to find the theming file. **Notes** Since then, the tensorboard_plugin_profiler build process mostly follows tensorboard's setup, but we are diverging moving forward as following: (1) The tensorboard team changed to use esbuild to bundle the js file instead of rollup (see tensorflow/tensorboard#5829). We may consider switching to esbuild in the future as well given its potential better performance, but now since it's not blocking us, we remain to use rollup. (2) The tensorboard team uses angular teams internal toolchain to build angular with bazel for angular v13+ (see tensorflow/tensorboard#6049). Since we turns out don't have the issue tensorboard is facing when upgrading angular, we are fine to go forward without changing our bundling rules. And we tend not to because the toolchain is not publicly used and there's no guarantee on supporting in the future. PiperOrigin-RevId: 533374699

Few changes: - upgrade rules_nodejs to 5.7.0 (this will extend the nodejs versions available for installation, while we are installing the default nodejs version here with the 5.7.0 rules_nodejs toolchain) - following this, upgrade all bazel toolchain to 5..7.0 - correspondingly upgrade rules_sass version to 1.55.0 (I can only find rules_sass github codebase with given version tag, but not sure how to find its hash..etc. Here we are basically following tensorboard team's current setup) - upgrade angular (and all relative packages) to v14, correspondingly upgrade ngrx to v14, and bump up rxjs. - upgrade typescript to 4.7.4 - upgrade zone.js to 0.12.0 - import ts_library (used for bazel build) from concatjs instead of typescript, as the module is moved to concatjs. (Also added concatjs into data field of `tsc_wrapped_with_angular` target) - added name field to the output of `rollup.config.js`, as it is a required field now for iife output type - Added `node_modules` into `include_paths` of sass_binary build rule when building the `styles.css` file for frontend. This is because angular v14 changed to import angular/cdk from using a relative path to an new one like `@angular/cdk`, so we need to include the `node_modules` int the the searching paths to enable sassCompiler to find the theming file. - yarn_install's exports_directories_only property now defaults to True. We must set this to False in order to remain compatible with ts_library See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#exports_directories_only - No longer need to override node_version as rules_nodejs default version of node is now 16.12.0. See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#updated-defaults We were overriding this only because rules_nodejs defaul version of node was too old. **References** - migrating to rules_nodejs 5.0: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0 - Instructions for 5.7.0 specifically: https://github.com/bazelbuild/rules_nodejs/releases/tag/5.7.0 - Tensorboard team's angular upgrade notes: No longer need to override node_version as rules_nodejs default version of node is now 16.12.0. See: https://github.com/bazelbuild/rules_nodejs/wiki/Migrating-to-5.0#updated-defaults We were overriding this only because rules_nodejs defaul version of node was too old. - other relative tensorboard upgrade PRs (tons of information in the PR messasge): tensorflow/tensorboard#5977 tensorflow/tensorboard#6063 tensorflow/tensorboard#6066 **Notes** Since then, the tensorboard_plugin_profiler build process mostly follows tensorboard's setup, but we are diverging moving forward as following: (1) The tensorboard team changed to use esbuild to bundle the js file instead of rollup (see tensorflow/tensorboard#5829). We may consider switching to esbuild in the future as well given its potential better performance, but now since it's not blocking us, we remain to use rollup. (2) The tensorboard team uses angular teams internal toolchain to build angular with bazel for angular v13+ (see tensorflow/tensorboard#6049). Since we turns out don't have the issue tensorboard is facing when upgrading angular, we are fine to go forward without changing our bundling rules. And we tend not to because the toolchain is not publicly used and there's no guarantee on supporting in the future. PiperOrigin-RevId: 537957341

bmd3k added 9 commits July 28, 2022 16:09

Use esbuild for tb_webapp_binary (prod and dev).

6c8d8d6

Use esbuild for polymer3_lib_binary (prod and dev).

6d1759d

Implement tf_js_binary using esbuild and migrate other usages of

6e5d9fa

esbuild.

Get monaco source editor working by loading require.js after everything

40f602d

else.

Closer to done.

90f6e5d

Cleanup after corrupt rebase.

c86b42a

Cleanup for PR.

b06e4fe

Another cleanup pass.

08fb3fc

buildifier.

750e402

bmd3k requested a review from JamesHollyer July 29, 2022 14:08

JamesHollyer approved these changes Jul 29, 2022

View reviewed changes

bmd3k merged commit 0df21fc into tensorflow:master Aug 1, 2022

rileyajones mentioned this pull request Sep 14, 2022

Projector hangs on Computing PCA when using Tensorboard2.10 with Pytorch #5924

Closed

bmd3k mentioned this pull request Sep 22, 2022

projector: Fix esbuild-bundled projector. #5944

Merged

copybara-service bot mentioned this pull request Jun 1, 2023

Upgrade angular from v12 to v14 tensorflow/profiler#628

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use esbuild for javascript bundling. #5829

Use esbuild for javascript bundling. #5829

bmd3k commented Jul 29, 2022

JamesHollyer left a comment

JamesHollyer Jul 29, 2022

bmd3k Aug 1, 2022

JamesHollyer Jul 29, 2022

bmd3k Aug 1, 2022

JamesHollyer Jul 29, 2022

bmd3k Aug 1, 2022

Use esbuild for javascript bundling. #5829

Use esbuild for javascript bundling. #5829

Conversation

bmd3k commented Jul 29, 2022

JamesHollyer left a comment

Choose a reason for hiding this comment

JamesHollyer Jul 29, 2022

Choose a reason for hiding this comment

bmd3k Aug 1, 2022

Choose a reason for hiding this comment

JamesHollyer Jul 29, 2022

Choose a reason for hiding this comment

bmd3k Aug 1, 2022

Choose a reason for hiding this comment

JamesHollyer Jul 29, 2022

Choose a reason for hiding this comment

bmd3k Aug 1, 2022

Choose a reason for hiding this comment