Skip to content
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

Use Bazel and ts_library to build core and backend-cpu #5133

Merged
merged 101 commits into from Jul 15, 2021

Conversation

mattsoulanille
Copy link
Member

@mattsoulanille mattsoulanille commented May 27, 2021

This PR builds tfjs-core and tfjs-backend-cpu using Bazel and makes the build outputs available to the other packages in the monorepo.

New Bazel tools directory

This PR adds a new tools directory that includes Bazel rules and macros for building TensorFlow.js

tools/defaults.bzl

Defines default versions of ts_library and esbuild that are specific to the tfjs repo.

  • ts_library is set to use es2017 and has its tsconfig.json file set to tsconfig_ts_library.json.
  • esbuild has the correct esbuild binary for the user's platform pre-selected. esbuild is used for tests since it is very fast.

tools/tfjs_bundle.bzl

tfjs_bundle is a macro to build bundles for tfjs. It defines several rollup_bundle targets and their minified variants:

  • [name].es2017[.min].js: A UMD es2017 bundle.
  • [name][.min].js: A UMD es5 bundle.
  • [name].fesm[.min].js: An es2017 flat es modules bundle.
  • [name].node[.min].js: A commonjs es5 node bundle. (Note: This may be removed)

Rollup plugins and options are defined in the rollup_template.config.js file.

tools/tfjs_web_test.bzl

tfjs_web_test is a macro that defines several karma_web_test targets, most of which are run on BrowserStack:

  • [name]: A local karma_web_test target that runs using chrome.
  • browserstack_[browser_name]_[name]: A karma_web_test that runs on BrowserStack using [browser_name].
    • Browsers are defined in tools/karma_template.conf.js
    • The macro allows selecting which browsers to test with via the browsers keyword argument.

tools/copy_to_dist

copy_to_dist is a rule that creates symlinks for a given tree of files in another directory. It is used to copy the ts_library compilation results from src to dist to support creating an npm package with outputs in dist. It is also used to copy bundles from tfjs_bundle to dist (and any other files that need to be in dist).

To see the logs from the Cloud Build CI, please join either our discussion or announcement mailing list.

Building tfjs-core and tfjs-backend-cpu with Bazel

There are several steps to building tfjs-core. While tfjs-core itself does not depend on a backend, its tests require tfjs-backend-cpu, so we build them separately to avoid a circular dependency.

The following Bazel targets build tfjs-core without tests:

  • //tfjs-core/src:tfjs-core_src_lib: Builds all of tfjs-core excluding tests and the index.ts entrypoint.
    • Uses module_name = "@tensorflow/tfjs-core/dist" so that its files can be imported relative to that path. e.g. import @tensorflow/tfjs-core/dist/public/chained_ops/register_all_chained_ops.
  • //tfjs-core/src:tfjs-core_lib: Builds index.ts entrypoint and depends on //tfjs-core/src:tfjs-core_src_lib.
    • Uses module_name = "@tensorflow/tfjs-core" so it can be imported as @tensorflow/tfjs-core.

After tfjs-core is built, tfjs-backend-cpu can be built by the following rules:

  • //tfjs-backend-cpu/src:tfjs-backend-cpu_src_lib: Like with core, builds all of tfjs-backend-cpu excluding tests and the index.ts entrypoint. Depends on //tfjs-core/src/tfjs-core_lib.
  • //tfjs-backend-cpu/src:tfjs-backend-cpu_lib: Like with core, builds the index.ts entrypoint. These steps are similarly separated so that module_name can be set differently for each.

With tfjs-backend-cpu built, tfjs-core can be tested:

  • //tfjs-core/src/tfjs-core_test_lib: Builds all the test files.
  • //tfjs-core:tfjs-core_node_test: Runs tfjs-core_test_lib tests in node.
  • //tfjs-core/src:tfjs-core_test_bundle: Builds a browser bundle for tests.
  • //tfjs-core:tfjs-core_test: Runs web tests using the bundle.
  • There are several other tests, like snippets tests and async backends tests.

With tfjs-core tests build, tfjs-backend-cpu can be tested:

  • //tfjs-backend-cpu/src:tfjs-backend-cpu_test_lib: Builds cpu tests.
  • //tfjs-backend-cpu:tfjs-backend-cpu_test: Runs cpu tests in node.

Creating NPM packages

The pkg_npm rule is used to create an npm package for tfjs-core, but there are a few steps that need to be done first:

  • //tfjs-core:tf-core: Bundles //tfjs-core/src:tfjs-core_lib using tfjs_bundle.
  • //tfjs-core:copy_bundles: Copies bundles to dist/ (in bazel's output folder).
  • //tfjs-core:copy_src_to_dist: Copies the compiled results of //tfjs-core/src:tfjs-core[_src, _test]_lib to dist.
  • There are a few other copy rules to copy things like the miniprogram files and test snippets code.

Once bundles are built and everything is copied, the npm package can be created:

  • //tfjs-core:tfjs-core_pkg: Packages everything into an npm package.

A similar process to the above is used to package tfjs-backend-cpu.

Inter-operating with existing packages

Packages in the monorepo that don't yet use Bazel are linked to each other via link:../package-name dependencies in their package.json files. These link dependencies expect the outputs of building a package to appear in the package's directory, but Bazel doesn't allow that. Instead, Bazel places build artifacts in a location completely separate from the source files. This output directory can change, but there is a symlink to it named dist for easy access.

This means we can just change the link:../ dependencies to point to ../dist/bin/tfjs-backend-cpu/tfjs-backend-cpu_pkg, right? Unfortunately, we can't, because of how node resolve dependencies. Suppose we're building tfjs-layers, which depends on tfjs-backend-cpu. If we link to ../dist/bin/tfjs-backend-cpu/tfjs-backend-cpu_pkg, then when tfjs-backend-cpu imports / requires @tensorflow/tfjs-core, it moves up the dist/bin/tfjs-backend-cpu directory tree and looks in the root node_modules of the tfjs repo. Due to some specifics of how ts_library works, this node_modules contains an incomplete copy of @tensorflow/tfjs-core that is created by Bazel for running node tests, so the import fails.

We can't replace link:../ with file:../ because that would result in multiple imports of multiple copies of the same code, which creates several copies of things that are supposed to be singletons, like io handlers.

To work around these issues, this PR creates a new top-level link-package that provides a place where bazel-built packages can be link:../ed from. Its package.json currently lists @tensorflow/tfjs-core and @tensorflow/tfjs-backend-cpu as file:../ dependencies copied from ../dist/bin/.... With this new link package, tfjs-layers and others can now list @tensorflow/tfjs-backend-cpu as link: ../link-package/node_modules/@tensorflow/tfjs-backend-cpu. Now, when tfjs-backend-cpu looks for tfjs-core, it will find the correct tfjs-core, since the link package has both installed in its node_modules.

The link package should never be published to npm, and will be removed once the monorepo fully builds with Bazel.


This change is Reviewable

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @lina128, @mattsoulanille, and @pyu10055)


tfjs-backend-cpu/package.json, line 6 at r6 (raw file):

Previously, mattsoulanille (Matthew Soulanille) wrote…

We currently distribute our .js outputs as esmodules and a separate node bundle, however, with Bazel, we'll distribute the .js files as commonjs and new.mjs files as esmodules. That way, we can skip bundling for node because node can just use the commonjs files.

Also, since Bazel rewrites the commonjs module imports to use module names instead of relative paths, this change was required to make webgl karma tests pass. Webgl would import a file from @tensorflow/tfjs-core/dist/... which would then import @tensorflow/tfjs-core (since bazel changed relative imports to module name imports), but since the package.json listed the node bundle as the main entrypoint, it would import the bundle instead of tfjs-core/dist/index.js, which would result in duplicate definitions of classes. Specifically, this broke router_registry_test.ts.

Thank you for the detailed explanation!

@mattsoulanille
Copy link
Member Author

tensorflow/tfjs-models#757 should probably be merged before this PR. The current version of parcel-bundler that we use in models demos seems to have problems building with the Bazel outputs. Parcel 2 has no issues and neither does esbuild.

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this reminds me, what's the testing plan for release with the new Bazel build? Will you use Verdaccio for release testing? Any change to publish-tfjs-ci.sh?

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @lina128, @mattsoulanille, and @pyu10055)

@mattsoulanille
Copy link
Member Author

Release tests should run the same as before when we release, but I'll take a look (and try running them) to make sure. I've updated the release script to take into account how tfjs-core and tfjs-backend-cpu are published (I need to add this to the BAZEL_MIGRATION.md doc).

Copy link
Collaborator

@lina128 lina128 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

e2e/scripts/publish-tfjs-ci.sh uses yarn build-npm for-publish and npm publish, this probably needs to be changed?

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @lina128, @mattsoulanille, and @pyu10055)

Copy link
Member Author

@mattsoulanille mattsoulanille left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I changed publish-tfjs-ci.sh to be aware of Bazel. Here's a branch where I tested verdaccio release tests and here's them passing on GCP.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @lina128, @mattsoulanille, and @pyu10055)

Copy link
Collaborator

@pyu10055 pyu10055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 2 of 1 approvals obtained (waiting on @lina128 and @mattsoulanille)

@mattsoulanille mattsoulanille merged commit e745d42 into tensorflow:master Jul 15, 2021
Adopt Bazel automation moved this from In progress to Done Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants