Skip to content

Add an option to preserve program completion value #640

Closed

Description

@rvanvelzen

See #543, #625, #628.

Activity

  1. avdg commented on Jul 10, 2016

    @avdg
    Contributor

    rollup/rollup#774 seems to have a nice summary what all the fuzz is about.

  2. avdg commented on Jul 10, 2016

    @avdg
    Contributor

    Test cases would be nice.

  3. kzc commented on Jul 10, 2016

    @kzc
    Contributor

    If I interpret this thread correctly it suggests putting parens around IIFEs and any inline function because the Chrome browser - and only the Chrome browser - is slower to parse otherwise?

  4. nolanlawson commented on Sep 15, 2016

    @nolanlawson

    This issue affects Edge/IE as well. I did some benchmarks (rollup/rollup#774 (comment)) and spoke with Chakra engineers to confirm.

  5. kzc commented on Sep 15, 2016

    @kzc
    Contributor

    Firefox and Safari do not have this problem. It's odd that two other major browsers don't optimize this common IIFE idiom. Chrome's parser is reportedly being rewritten to increase its speed which would make this less of an issue.

    In any case, people can set the compress option negate_iife=false if they choose.

  6. kzc commented on Sep 15, 2016

    @kzc
    Contributor

    Based on this remark I think Microsoft would want to optimize the common !function(){...}(); idiom in Chakra:

    The experiment confirmed that usage of minified code is extremely popular on the web as it exists and amongst others, UglifyJS is very commonly used in today’s web. So in Windows 10 and Microsoft Edge, we’ve added new fast paths, improved inlining and optimized some heuristics in Chakra’s JIT compiler to ensure that minified code runs as fast, if not faster than the non-minified versions. With these changes, the performance of individual code patterns minified using UglifyJS that we tested, improved between 20-50%.

    https://blogs.windows.com/msedgedev/2015/05/20/delivering-fast-javascript-performance-in-microsoft-edge

  7. rvanvelzen commented on Sep 15, 2016

    @rvanvelzen
    CollaboratorAuthor

    This specific issue is regarding completion value. Inner IIFEs would still receive the !.

    I can't immediately find a representative benchmark (e.g. profiles of a full application running for a few seconds) in the referenced task. If it turns out to really matter in real world applications it is worth considering removing negate_iife (or defaulting to off).

    On the other hand, the performance issue is more easily solved by browser vendors - as it seems it should be a trivial change.

  8. kzc commented on Sep 15, 2016

    @kzc
    Contributor

    How does one access the "completion value" of the program through an IIFE anyway?

    $ node -e '(function(){console.log("completed");return 3})();'
    completed
    
    $ echo $?
    0
    

    I can't get it to work.

    (Edited to mention IIFE)

  9. nolanlawson commented on Sep 16, 2016

    @nolanlawson

    @kzc Just ran a benchmark to test (see more comments in #886 (comment)), and both Chakra and V8 optimize for the !function(){}() style. In fact both engines optimize for !function(){}(), (function(){})(), and (function(){}()). Made a mistake, see #886 (comment), only Chakra optimizes for this

    What's unoptimized is the UMD-style (or Browserify-style/Webpack-style/etc.) case where the function is passed in to another function. If you wrap parens around the function, though, you can get the optimization.

    So in short this has nothing to do with IIFEs and everything to do with parens around immediately-invoked functions (however they may get invoked).

  10. rvanvelzen commented on Sep 16, 2016

    @rvanvelzen
    CollaboratorAuthor

    @kzc an example would be inlined event handlers, which can be minified:

    <a onclick="(function () { return false; })()">

    After minification with negate_iife, the return value here would be true instead of false.

    Granted, this is easily mitigated by disabled negate_iife, but I believe that our default configuration should work for these cases.

  11. added a commit that references this issue on Sep 16, 2016
    29e1ff0
  12. kzc commented on Sep 16, 2016

    @kzc
    Contributor

    @rvanvelzen Thanks for explanation.

    @alexlamsl does html-minifier uglify inlined JS code within HTML as per comment above?

  13. alexlamsl commented on Sep 16, 2016

    @alexlamsl
    Collaborator

    @kzc yes it does, by wrapping the code around !function(){...} before passing to uglify-js, then unwrap the minifier result afterwards.

    So whilst the example above would produce the wrong result as stated, the more common form of <a onclick="return false"> works as expected, i.e. <a onclick=return!1>

  14. kzc commented on Sep 16, 2016

    @kzc
    Contributor

    @alexlamsl Thanks for confirming that uglify is used in the wild in that scenario.

  15. kzc commented on Sep 16, 2016

    @kzc
    Contributor

    Using the example cited above:

    <a onclick="(function () { return false; })()">

    It may be better if the negate_iife optimization were to use + rather than ! as in:

    +function(){return!1}()
    

    because although the return value is converted to a number, at least the "truthiness" of the return value remains the same:

    +false == 0
    
    +true == 1
    
  16. kzc commented on Sep 16, 2016

    @kzc
    Contributor

    Ignore my last post above. A lesser wrong is still wrong. :-)

    And Microsoft Edge browser reportedly optimizes the ! IIFEs.

  17. nolanlawson commented on Sep 17, 2016

    @nolanlawson

    Yes, based on my updated benchmark (#886 (comment)), only Chakra optimizes for ! IIFEs. The blog post you linked to about Chakra specifically optimizing for Uglify patterns is correct.

    Also, according to the new benchmark it seems ! IIFEs hurt performance in SpiderMonkey and V8 (not sure about JSC). So you could see this either as a bug in those engines or a bug in Uglify.

    In my opinion bundlers/minifiers ought to try harder to give the engine hints, but in this case, doing a good job would be hard. You'd basically have to parse the whole JS file and try to determine which functions are immediately-invoked and which ones aren't, and then wrap the immediately-invoked ones in parens to "trick" the engines into not preparsing. Wrapping non-immediately-invoked functions in parens would be a performance regression because it would increase startup time by fully parsing functions that are only executed later (or never executed).

  18. kzc commented on Sep 17, 2016

    @kzc
    Contributor

    Saying it's a bug is a bit harsh, as the generated code still produces the correct result in all but one pathological case above: #640 (comment) and the option can be disabled by the user should they choose.

    But for the few bytes negate_iife saves in a typical bundle it's probably not worth the trouble:

    $ uglifyjs -m -c negate_iife=1,warnings=0 --self  | wc -c
      131282
    
    $ uglifyjs -m -c negate_iife=0,warnings=0 --self  | wc -c
      131285
    

    Uglify is a fairly representative javascript program and only 3 bytes are saved.

    Disabling negate_iife by default is reasonable.

  19. alexlamsl commented on Sep 18, 2016

    @alexlamsl
    Collaborator

    @kzc just read #640 (comment) and thought I might be able to answer that...

    $ node
    > (function(){console.log("completed");return 3})();
    completed
    3
    >
    
  20. added 3 commits that reference this issue on Mar 1, 2017
    dfb2d43
    91f29e8
    f01c0c4
  21. added a commit that references this issue on Mar 3, 2017
    07accd2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions