[Feature request] Add flag for optionally disabling tree-shaking

[mattem]

Feature request

A command line flag to opt-out of tree-shaking (or opt-in to tree-shaking, that defaults true). An alternative could also to only enable tree-shaking if the --minify option is also present.

Motivation

Attempting to use esbuild as a replacement for concat_js / Webpack when using Karma, the test runner for Angular.
When Angular compiles templates to TS, it marks the generated metadata functions as pure as these aren't needed in production builds, however in some situations they are needed during tests.

Using esbuild to generate a testing bundle will currently remove the function calls that are marked as pure (rightly so), but for this use case they are needed.

[kzc]

Related issue: #371

Although it'd be preferable to have no optimizations or minification take place in the absence of the --minify option, introducing a new flag like --no-opt would be a second choice.

[vladmandic]

this is actually required for some modules that define number of functions, but do not reference them at all and instead register them for usage during runtime.

example is @tensorflow/tfjs-backend-cpu, @tensorflow/tfjs-backend-webgl, @tensorflow/tfjs-backend-wasm - they all implement same math functions, but implemented differently (using CPU, WebGL or WASM) and active one gets registered for current usage.

at the moment esbuild drops all those functions from bundle.

(this also means it's not a feature request, it's becomes a bug).

[evanw]

at the moment esbuild drops all those functions from bundle.

I'd like to get to the root of the problem and understand why this is happening instead of just adding a flag to ignore the issue. I have so far found these issues that come up when using esbuild with that package:

• @tensorflow/tfjs-core defines "main": "dist/tf-core.node.js" and "module": "dist/index.js" but then marks dist/index.js as having side effects and dist/tf-core.node.js as having no side effects.

  This exposed a bug with esbuild's automatic main field picker where module was incorrectly marked as having no side effects because main was marked as having no side effects. I fixed this bug in the commit mentioned above.

• It looks like esbuild's handling of side effects is incorrect for re-exported symbols (the export * as syntax). I will fix that bug too.

  Edit: Actually, I looked at this again and I think what esbuild is doing here is correct. The exports are unused and the module that registers the WebAssembly backend is marked as having no side effects, so it seems correct to me for a bundler to remove it if all of the exports are unused. In other words this seems like a bug with the package itself.

[vladmandic]

In other words this seems like a bug with the package itself.

I'm already having a conversation with package maintainers trying to change it, there are several issue with that package definition (for example, nodejs specific requires in a browser package) - so far, the reply has been "since it works with webpack, it must be issue with your bundler"

[evanw]

Yeah I'm still not totally sure if it's an issue with esbuild or not. The sideEffects thing is something Webpack invented, so in a sense they are the specification. I'll have to dig around in Webpack's internals to figure out what exactly they are doing. A file is marked as having side effects that re-exports everything from a file that is marked as not having side effects. If none of those exports are used, then I thought that meant the file marked as not having side effects could be removed. But perhaps not. It's going to take me a while to dig through Webpack's code though.

[vladmandic]

it's also very inconsistent, for example:

first import is tfjs bundle-of-bundles which includes tfjs-core, tfjs-data, tfjs-layers, tfjs-converter, tfjs-backend-cpu, tfjs-backend-webgl:

import * as tf from '@tensorflow/tfjs/dist/tf.es2017.js'; // works
import * as tf from '@tensorflow/tfjs/dist/index.js'; // doesn't work

and then load wasm module after that, but the opposite works:

import { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/index.js'; // works
import { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/tf-backend-wasm.es2017.js'; // doesn't work

(this inconsistency is nothing to do with esbuild, it's the same when i use it in script tag with type module)

[evanw]

Wait I'm confused. It doesn't even work with Webpack. Here's the example from their readme:

// Import @tensorflow/tfjs or @tensorflow/tfjs-core
import * as tf from '@tensorflow/tfjs';
// Adds the WASM backend to the global backend registry.
import '@tensorflow/tfjs-backend-wasm';
// Set the backend to WASM and wait for the module to be ready.
tf.setBackend('wasm').then(() => main());

Bundling that with webpack ./example.js and running the bundle in the browser with a script tag throws Error: Backend name 'wasm' not found in registry. Maybe it only works with some custom Webpack configuration?

[vladmandic]

the webpack config tfjs quotes is documented here: https://github.com/tensorflow/tfjs/tree/master/tfjs-backend-wasm/starter/webpack

but regarding code, at the very minimum you need to check if .wasm file is loaded correctly when calling tf.setBackend('wasm') - is there a http request that completes without error?

best is to call tf.setWasmPaths('your_path_here'); excplicitly before calling tf.setBackend('wasm')

but in general, not found in registry is an error that happens when tree shaking removes functions so they don't get registered.
and strictly, those functions are not used - they are just defined - and get registered dynamically when backend is activated.

see my first note: example is @tensorflow/tfjs-backend-cpu, @tensorflow/tfjs-backend-webgl, @tensorflow/tfjs-backend-wasm - they all implement same math functions, but implemented differently (using CPU, WebGL or WASM) and active one gets registered for current usage.

[vladmandic]

i've created a simple test and seems like latest version you just published fixes tree shaking issues, all variations now work - great stuff!

what remains is the minify problem #538

https://gist.github.com/vladmandic/3097052d32a902239e9b855af8fd8425

[vladmandic]

actually, there is one more border case where tree shaking is overly aggressive.

this works:

import * as tf from '@tensorflow/tfjs/dist/index.js';
import { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/index.js';

but if i create a tf.js wrapper simply containing

export * as tf from '@tensorflow/tfjs/dist/index.js';
export { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/index.js';

and then use it in rest of the code as:

import { tf, setWasmPaths } from './tf.js'

it appears to load, but it will fail on first tensor operation as 'not registered'
(and i can see the bundle size dropping from 2.1M to 1.8M, so tree shaking drops code)

but then when i do this, it works again

export * as tf from '@tensorflow/tfjs/dist/index.js';
export * as wasm from '@tensorflow/tfjs-backend-wasm/dist/index.js';

basically, indirectly exporting single function makes esbuild think all other functions are not used and drops them.
i don't think esbuild does anything wrong here, it's just something to note.

[evanw]

I can't reproduce those results. Here's what I tried:

// index.js
import * as tf from '@tensorflow/tfjs/dist/index.js';
import { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/index.js';

// index2.js
import { tf, setWasmPaths } from './tf.js'

// tf.js
export * as tf from '@tensorflow/tfjs/dist/index.js';
export { setWasmPaths } from '@tensorflow/tfjs-backend-wasm/dist/index.js';

Versions:

@tensorflow/tfjs: 2.7.0
@tensorflow/tfjs-backend-wasm: 2.7.0
esbuild: 0.8.9

Commands:

esbuild --bundle index.js --outfile=out.js
esbuild --bundle index2.js --outfile=out2.js

Both files out.js and out2.js are byte-wise identical and have 2,353,824 bytes.

It could be different if there is more in the file that actually uses tf. Currently esbuild special-cases syntax like import * as tf when all uses of tf are statically-known property accesses within the same file. In that case esbuild removes the * as tf and substitutes the set of used property names as imports instead. This is equivalent and allows for better tree shaking since unused imported properties can be removed. For example if the only use of tf is to access tf.prop esbuild will behave as if you had written import {prop} from ... instead. However, this does not happen if you then re-export tf. Basically tracking this is possible but it would require analyzing arbitrary chains of property accesses across files which is not done for performance reasons. So that could explain the difference.

[vladmandic]

there's something weird, i also can't reproduce with a test app, but clearly see bundle size difference on my real app.

in any case, i like the behavior you describe with export * as ....
i just noted this is something strange - consider this as resolved from my side (i'm not the creator of this issue, so can't close it).

btw, thanks for amazing support!

[evanw]

A flag to disable respecting tree shaking annotations has now been released: --tree-shaking=ignore-annotations.