Uglifyjs: Inlining functions that are non-constant expressions causes perf regression

[rafeca]

Bug report or feature request?

Feature request? (Sub-optimal code)

ES5 or ES6+ input?

ES5

Uglify version (uglifyjs -V)

>=3.1.7

JavaScript input

function nonTrivialFn(pre) {
  return pre + Math.random();
}
function hotFunctionCalledALotOfTimes(num) {
  return nonTrivialFn(num + Math.random());
}
hotFunctionCalledALotOfTimes(3);
hotFunctionCalledALotOfTimes(5);

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

{
  inline: true,
  reduce_vars: true,
  side_effects: true,
  toplevel: true,
  unused: true,
}

JavaScript output or error produced.

function hotFunctionCalledALotOfTimes(num){
  return function(pre) {
    return pre + Math.random();
  }(num + Math.random())
}
hotFunctionCalledALotOfTimes(3);
hotFunctionCalledALotOfTimes(5);

Description

In metro bundler (the JS packager used by React Native) we've observed a noticeable performance regression (in terms of TTI and memory increase) after upgrading from [email protected] to [email protected] when running React Native apps.

After doing a bisect, we've found that the causing commit is https://github.com/mishoo/UglifyJS2/commit/5b4b07e9a7d67e593c6ae8d54dc77d174afd25ac, which inlines functions that are non-constant expressions, causing some functions in very hot codepaths to be constantly re-created.

Would it be possible to have an option to disable the inlining of functions that are not constant expressions?

[alexlamsl]

Pull Request is welcomed.

[kzc]

See: PR #2465
See: PR #2466

[kzc]

I thought Node/V8 was representative of how the other major JS engines performed with function expressions, but this is not the case.

FireFox 56 and Safari 9.1.3 produce consistent timings regardless of whether a top level function declaration or an in-loop function expression is used. Chrome 62 is significantly slower for function expressions within hot loops (f2.js):

// f1.js 
var start = new Date().getTime();
function foo(x, y, z) {
    return x < y ? x * y + z : x * z - y;
};
var sum = 0;
for (var i = 0; i < 1e8; ++i) {
    sum += foo(i, i+1, i*3);
}
console.log(sum);
var end = new Date().getTime();
console.log("elapsed: " + (end - start) / 1000.0 + "s");

/*
Chrome 62:    0.298s
FireFox 56:   0.197s
Safari 9.1.3: 0.535s
node 8.0.0:   0.386s
node 6.9.0:   0.329s
node 4.2.1:   0.352s
*/

// f2.js 
var start = new Date().getTime();
var sum = 0;
for (var i = 0; i < 1e8; ++i) {
    sum += function foo(x, y, z) {
        return x < y ? x * y + z : x * z - y;
    }(i, i+1, i*3);
}
console.log(sum);
var end = new Date().getTime();
console.log("elapsed: " + (end - start) / 1000.0 + "s");

/*
Chrome 62:    1.435s
FireFox 56:   0.198s
Safari 9.1.3: 0.540s
node 8.0.0:   4.350s
node 6.9.0:   2.917s
node 4.2.1:   2.361s
*/

@alexlamsl How do IE11, Edge and ChakraCore behave with these benchmarks?

/cc @bmeurer

[alexlamsl]

I thought Node/V8 was representative of how the other major JS engines performed with function expressions, but this is not the case.

Now I see why you think this is a bigger deal than I do.

How do IE11, Edge and ChakraCore behave with these benchmarks?

IE11 and ChakraCore show no performance difference just like FireFox or Safari in your test.

Haven't tested Edge, but it should be using almost the same JavaScript engine as ChakraCore.

[alexlamsl]

We can rename reduce_funcs to chrome if you want :wink:

[kzc]

IE11 and ChakraCore show no performance difference just like FireFox or Safari in your test.
Now I see why you think this is a bigger deal than I do.

True enough. Uglify shouldn't have to make Chrome specific optimizations even if it has the biggest browser market share.