Uglifyjs: Compressor with sequences=false fails

Created on 8 Nov 2017  路  5Comments  路  Source: mishoo/UglifyJS

Bug report or feature request?

Bug

ES5 or ES6+ input?

ES5

Uglify version (uglifyjs -V)

  • 3.1.8 and 3.1.7 (not reproducible with 3.1.6)

  • Node version 8.9.0

JavaScript input

balls.js

The uglifyjs CLI command executed or minify() options used.

./node_modules/uglify-es/bin/uglifyjs balls.js -c sequences=false -o out.js

Also reproducible using uglifyjs-webpack-plugin.

JavaScript output or error produced.

ERROR: Cannot read property 'transform' of null
    at eval (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:4589:31)
    at AST_SimpleStatement.eval [as transform] (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:4562:21)
    at AST_SimpleStatement._clone (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:456:25)
    at AST_SimpleStatement.clone (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:465:21)
    at TreeTransformer.eval [as before] (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:458:33)
    at AST_SimpleStatement.eval [as transform] (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:4558:35)
    at eval (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:4577:25)
    at doit (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:134:23)
    at MAP (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:160:52)
    at do_list (eval at <anonymous> (/home/gibson/node_modules/uglify-es/tools/node.js:21:1), <anonymous>:4576:16)
bug

All 5 comments

uglify-js has the same bug on the ES5 file.

@buu700 Here's the thing - your JS input file has a ton of asm.js code in it without "use asm" directives. As such some browsers will not implement specialized asm.js optimizations and will just rely on their regular JIT engine optimizations.

Uglify disables compress transforms in "use asm" scopes. Without that directive Uglify will make those functions subject to Uglify compress transforms which could make them slower at runtime in certain browsers.

Even if the sequences=false bug is fixed I'd recommend one of the following workarounds because it will produce faster code:

Workaround A - disable the compress option reduce_vars:

$ time bin/uglifyjs balls.js -m -c sequences=false,reduce_vars=false | gzip | wc -c
  527434

real    0m23.638s
user    0m24.746s
sys 0m0.322s

reduce_vars will inline single-use functions, which would make asm.js generated code without "use asm" directives significantly slower.

Workaround B - disable compress altogether:

$ time bin/uglifyjs balls.js -m | gzip | wc -c
  531980

real    0m6.239s
user    0m6.399s
sys 0m0.171s

Workaround B is only 5K larger gzipped and is preferable in my opinion. It also uglifies 4X faster than Workaround A.

Thanks for the advice! Everything you discovered is actually completely by design*, however. Out of curiosity, does it look like this issue is related to the asm.js, or is that just something you happened to notice while investigating?

(The need for sequences=false in my project is unrelated to all of this, and just a temporary workaround until we reimplement some old post-processing code to hook into webpack.)


*: libsodium.js, ntru.js, and a small set of similar libraries all preferentially use WebAssembly with -O3 optimization (as embedded base64) and include asm.js only as a fallback.

Since the asm.js is just a fallback that we don't care too much about, "use asm" is stripped to prevent AOT compilation (which would block the thread even in clients that support wasm) and it's emitted with -Oz and run through the uglify compressor to make it as small as possible. Incidentally, it sounds like stripping "use asm" is unintentionally improving the efficiency of that last bit, as I wouldn't have guessed that uglify was taking special care not to interfere with it.

See https://github.com/jedisct1/libsodium.js/pull/106 for context.

@kzc any luck reducing the test case? I'll need to catch up on sleep so will take another stab at this over the weekend.

Essentially something turns g=We(o|0,g|0,3)|0; into new AST_Statement() { body: null }, which in turn chokes when reduce_vars tries to clone the encapsulating function Ge(r,i,o,g):
https://github.com/mishoo/UglifyJS2/blob/4c0b0177b6fa556c31f0099c6d52a4ad4f670ba3/lib/compress.js#L4260-L4267

Since the asm.js is just a fallback that we don't care too much about, "use asm" is stripped to prevent AOT compilation (which would block the main thread even in clients that support wasm)

@buu700 I see. Very interesting.

I just ran uglifyjs -b on the file to see why it took so long to runuglifyjs -c and I noticed the asm.js code without the directive.

See #818 for more info on how uglify handles "use asm". You might leave the directives in place just for the uglify "use asm" compress support, and then run sed or equivalent on the result to strip "use asm";.

any luck reducing the test case?

@alexlamsl It's somewhere in the asm.js code. The bug disappears as unrelated functions are removed. I think it'd be easier to add debug statements within uglify to isolate it.

Essentially something turns g=We(o|0,g|0,3)|0; into new AST_Statement() { body: null }

I think it was an AST_SimpleStatement judging by the stack trace in the top post - not that it matters. :-)

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pvdz picture pvdz  路  3Comments

PinkyJie picture PinkyJie  路  3Comments

uiteoi picture uiteoi  路  5Comments

Jimbly picture Jimbly  路  4Comments

alexlamsl picture alexlamsl  路  4Comments