Optimize closures that only run once

RESOLVED FIXED in mozilla21

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

Other Branch
mozilla21
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Currently, placing code inside a closure will give considerably worse performance than if that code was at the top level of a page.  This is because the engine treats all functions as if they can run multiple times, and optimizations that are done to scripts and data at the top level cannot be applied (top level <script> scripts will only run once).

In many cases, however, these closures will only run once, e.g. they are of the form '(function() { ....})()'.  Code should run at the same speed whether it is at the top level or within a closure such as this.

It's not easy to prove that the closure will only run once, as the lambda can still escape through fun.caller accesses while it is still on the stack.  So, this I think requires the compiler to optimistically treat these closures as running once, but be able to fall back and deoptimize if that assumption turns out incorrect.

Comment 1

5 years ago
+1 !! This would make firefox a lot faster for read world websites.

Comment 2

5 years ago
+1 !! This would make firefox a lot faster for real world websites.

Comment 3

5 years ago
By the way, this happens on windows also, not only MacOSx

Updated

5 years ago
OS: Mac OS X → All
Hardware: x86 → All

Comment 4

5 years ago
I recently read that V8 looks for the standard closure pattern "(function(" and handles it differently (optimizes). I try to dig up the source …
(Assignee)

Comment 5

5 years ago
Created attachment 692010 [details] [diff] [review]
patch (946bc83d50df)

This patch watches for the (function() {...})() pattern at the top level and marks the lambda so that it should be treated as running once (though it may run more than once).  This will cause singleton types to be generated for initializers and functions within the lambda, and if the lambda ends up being invoked multiple times its inner functions are deeply cloned.  On this benchmark:

x = (function foo() {
    var n = 0;
    function g() {
      this.a = 0;
      this.b = 1;
    }
    function f() {
      for (var i = 0; i < n; i++) {
        new g();
      }
    }
    function setn(n_) {
      n = n_;
    }
    return {
      f: f,
      setn: setn
    }
})();

x.setn(1000000);
x.f();

This takes time with --ion from 160ms to 40ms.  The equivalent script in global code takes 40ms.  This hasn't wiped out all overhead from using the closure, as closure variable lookups are still more expensive than global lookups.  LICM/GVN in the compiler should mitigate these costs so that closure speed is close to what is seen in global code.
Assignee: general → bhackett1024
Attachment #692010 - Flags: review?(luke)

Comment 6

5 years ago
Please note there are several different "styles" for this kind closures / IIFE [1]. This optimization should also (and at least) work with the Crockford style: 

(function() {
  // ...
}());


[1] http://jsperf.com/immediate-anonymous-function-invocation
(Assignee)

Updated

5 years ago
Attachment #692010 - Flags: review?(luke) → review?(jdemooij)
(Assignee)

Updated

5 years ago
Attachment #692010 - Flags: review?(jdemooij) → review?(luke)

Comment 7

5 years ago
Comment on attachment 692010 [details] [diff] [review]
patch (946bc83d50df)

Looks good.  Could you add a test-case that calls a treatAsRunOnce lambda twice and that asserts/faults if you take out the "script->hasBeenCloned = true" statement in CloneFunctionObjectIfNotSingleton?  r+ with this.
Attachment #692010 - Flags: review?(luke) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/88daef90f2ab

Re: comment 6, this works with all the closure styles on that webpage except those which explicitly invoke Function() (which involves a function call, so is harder to handle during parsing).
https://hg.mozilla.org/mozilla-central/rev/88daef90f2ab
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21

Updated

5 years ago
Depends on: 835140
You need to log in before you can comment on or make changes to this bug.