Closed Bug 821361 Opened 12 years ago Closed 12 years ago

Optimize closures that only run once

Categories

(Core :: JavaScript Engine, defect)

Other Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(1 file)

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.
+1 !! This would make firefox a lot faster for read world websites.
+1 !! This would make firefox a lot faster for real world websites.
By the way, this happens on windows also, not only MacOSx
OS: Mac OS X → All
Hardware: x86 → All
I recently read that V8 looks for the standard closure pattern "(function(" and handles it differently (optimizes). I try to dig up the source …
Attached patch patch (946bc83d50df) (deleted) — Splinter Review
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)
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
Attachment #692010 - Flags: review?(luke) → review?(jdemooij)
Attachment #692010 - Flags: review?(jdemooij) → review?(luke)
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+
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).
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Depends on: 835140
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: