support for optimize-js optimizations

[arnoudb]

I see the code execution becomes slower after applying uglify. it could become fast again if some basic rules are applied:

see https://github.com/nolanlawson/optimize-js

just wanted to bring this to your attention as it would be a great to incorporate this in uglify.

[kzc]

Just use -c negate_iife=false on the original JS to achieve the same effect.

Uglify could change the default going forward, but it's surprising that some browsers don't detect the negated iife pattern and optimize for it considering its widespread use.

[arnoudb]

Thnx for the fast reply!

We already use that flag in our project and it helps indeed. So we are very happy this option is already available. And yes you are right that browsers should optimise this themselves. But unfortunately they don't do such a great job in this respect.

But there are 2 other things that speed things up that are mentioned and done in optimize-js. For example passing a function expression to a function call like:

document.addEventListener('click', function(){/*handle click*/});

the optimized version would be:

document.addEventListener('click', (function(){/*handle click*/}));

It would be awesome if these could be supported as well as these code constructs are used much used practice an therefor the speed improvements are considerable. I looked a bit into the uglify source code but i have no idea where to add this myself.

[kzc]

This function expression paren issue appears to be specific to V8/Chrome. I could not reproduce the issue with Firefox. Although I don't use Windows and can't verify this, Microsoft has stated it made Uglify specific optimizations in Chakra.

It would be fairly easy to add an Uglify option to put parens around all function expressions. But adding truly unnecessary parens for function expressions in cases such as the event handler scenario above is counter to what uglify does - reducing the size of javascript code.

If the speed gains are significant after analyzing hundreds of the most popular websites, surely V8 would have made this function expression optimization the default. I suggest you open a ticket with the V8 folks.

[kzc]

I'm not a fan of this optimization since the browsers should be in a position to better handle it, but this is how it could be implemented:

--- a/lib/output.js
+++ b/lib/output.js
@@ -67,7 +67,8 @@ function OutputStream(options) {
         screw_ie8        : true,
         preamble         : null,
         quote_style      : 0,
-        keep_quoted_props: false
+        keep_quoted_props: false,
+        wrap_funcs       : false,
     }, true);

     // Convert comment option to RegExp if neccessary and set up comments filter
@@ -552,7 +553,7 @@ function OutputStream(options) {
     // a function expression needs parens around it when it's provably
     // the first token to appear in a statement.
     PARENS(AST_Function, function(output){
-        return first_in_statement(output);
+        return output.option("wrap_funcs") || first_in_statement(output);
     });

     // same goes for an object literal, because otherwise it would be

$ cat funcs.js 

    (function iife(m) {
        function foo(x, f) {
            f(x);
        }
        foo(m, function(a) {
            console.log(a);
        });
    }("bar"));

$ bin/uglifyjs funcs.js -c
!function(m){function foo(x,f){f(x)}foo(m,function(a){console.log(a)})}("bar");

$ bin/uglifyjs funcs.js -c negate_iife=0 -b beautify=0,wrap_funcs=0
(function(m){function foo(x,f){f(x)}foo(m,function(a){console.log(a)})})("bar");

$ bin/uglifyjs funcs.js -c negate_iife=0 -b beautify=0,wrap_funcs=1
(function(m){function foo(x,f){f(x)}foo(m,(function(a){console.log(a)}))})("bar");

[vwochnik]

Considering that uglify is mostly used on the client side to make the JavaScript size smaller for it to load faster, the option should be available considering it gives some performance gains which is the ultimate goal.

[kzc]

This is a browser quality of implementation issue and that's where it should be addressed. V8 has this issue, while webkit does not. "Fixing" the issue in client code is a hack.

[arnoudb]

@kzc I totally agree with you that this optimizing should be the task of the browser.

But the fact is that at the moment we are stuck with the current browser implementations. Especially if you target mobile the parsing differences are getting really big for chrome. Browsers on mobile are not ever green. So i hope you will consider your implementation to be part of the next release. Turned off by default it would be just a nice option like negate_iife for users with specific wishes.

However If you don't think it should be part of uglify then i still thank you very much for posting the solution. We then can create our own local build of uglify.

Thnx!

[vwochnik]

@arnoudb +1

[kzc]

@arnoudb Unconditionally adding unnecessary parens around all function expressions including event handlers seems fundamentally wrong to me. The Chrome/V8 team would have analyzed thousands of the most popular websites and tuned their JS engine accordingly. If there's a demonstrable gap in their analysis then it's a problem that they'd be happy to fix it to make their browser experience better. Once you've reported the issue to the V8 project please post a link to the ticket here.

[rvanvelzen]

Adding parenthesis around all function expressions is ridiculous, and we will not implement that.

#1310 will add parens around IIFEs. All other functions are not guaranteed to be executed immediately, so adding parens there would only increase the start-up time.

[kzc]

@rvanvelzen What's the advantage of PR #1310 over just setting negate_iife=false?

The normal IIFE pattern (function(){...})() is already optimized by V8.

[rvanvelzen]

The only slight advantage is that it also wraps IIFEs that don't appear in statement position. E.g.:

foo = (function () { ... }());

That aside, setting negate_iife to false does exactly the same thing.

[kzc]

Wouldn't it be easier to have Uglify default negate_iife to false?

It would "solve" the program completion value issue #640 as well.

People who are not concerned with completion values and wanting to save the one byte per IIFE can manually enable negate_iife.

[kzc]

foo = (function () { ... }());

@rvanvelzen I see what you mean now. That is different than negate_iife=false.

I guess if the proposed wrap_iife option is disabled by default there's no harm done.

If you decide to merge #1310 could you please add #1307 (comment) as a test case?

[arnoudb]

Just curious. Why is the option negate_iife added to uglify at all? If it's for performance and we follow the reasoning that runtime optimizations are the domain of the browser than it feels a bit inconsistent to not want the option to wrap function expressions.

If however negate_iife is added for another reason then i my reasoning is not correct

[rvanvelzen]

negate_iife exists to save a byte per IIFE, not for runtime performance.

(Note: I'll merge #1310, but I keep being a bit too busy. Will do soon, along with other PRs)

[arnoudb]

ah of course, stupid me :-) But awesome that you are going to merge this. thnx a lot!