defined function and function expression much slower than dynamic closure when using inner function

NEW
Unassigned

Status

()

defect
9 years ago
5 years ago

People

(Reporter: shaver, Unassigned)

Tracking

({perf})

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

()

Humph told me about this. May be a dup, but I couldn't find it in quick (200ms) search.

 var createdClosure = (function() {
  function inner(a) {
   return a + 1;
  }
  return function(a) {
   return inner(a);
  }
 })();
 
 var expressionClosure = function(a) {
  function inner(a) {
   return a + 1;
  }
  return inner(a);
 };
 
 function func(a) {
  function inner(a) {
   return a + 1;
  }
  return inner(a);
 }

I get 33M/s for createdClosure, and 9M/s for each of expressionClosure and func.  (Chrome 9 on the same hardware is 73M/s and 13M/s.)

If I remove the inner function (http://jsperf.com/closure-vs-function-no-inner) I get ~40M/s for each.  Chrome 9 gets about 151M/s on each; this bug is about our disparity between the patterns, though, not Chrome's 4x lead on the simple function-call case. (Which I thought we'd destroy, but loop overhead probably dominates function call cost, now that I think about it.)
Calling the function returned by createdClosure has to be faster since there is no inner to clone -- that function lives in the power-constructor-pattern closure that is evaluated once to make createClosure.

Anyone profile yet? Just curious but not a blocker, so I'll wait.

/be
That makes sense, though in this specific case I guess the cloning of inner isn't needed since there's nothing captured to distinguish instances?  Not sure if analyzing that is feasible.

I'll profile after 4.
(In reply to comment #2)
> That makes sense, though in this specific case I guess the cloning of inner
> isn't needed since there's nothing captured to distinguish instances?  Not sure
> if analyzing that is feasible.

It's quite feasible. Fixing it here seems like the right thing.

/be
for expressionClosure in the browser, shark says

- 27.4%, js_NewNullClosure(JSContext*, JSObject*, JSObject*, JSObject*), XUL
- 24.9%, JSCompartment::finalizeObjectArenaLists(JSContext*), XUL
- 16.2%, js::ExecuteTree(JSContext*, js::TraceMonitor*, js::TreeFragment*, unsigned int&, js::VMSideExit**, js::VMSideExit**), XUL
- 10.7%, fun_finalize(JSContext*, JSObject*), XUL
- 5.3%, js::gc::Arena<JSObject_Slots2>* AllocateArena<JSObject_Slots2>(JSContext*, unsigned int), XUL
- 1.1%, js::PropertyTree::unmarkShapes(JSContext*), XUL

system libraries, NSPR, and unknown libraries (JIT code) charged to callers.

(it also thinks that js_NewNullClosure should be inlined, since virtually all of its time is spent in the prologue; probably hitting the !closure case?)
profile for func is basically identical.

Comment 6

5 years ago
Usint the testcase from jsperf I get the same numbers on all 3 cases, and Nightly 32 is 34% faster than Chrome 35.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → WORKSFORME
Not for me, not even close.

http://jsperf.com/closure-vs-function

created closure with inner
755,726,239 ±0.41% fastest

expression closure with inner
78,618,211 ±0.38% 90% slower

function with inner
78,634,572 ±0.36% 90% slower

On linux x64, official nightly from https://hg.mozilla.org/mozilla-central/rev/e017c15325ae with a fresh profile.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
same here on mac os: 

created closure: 1,797,026,819
expression closure: 99,470,750 (94% slower)
func: 99,715,966 (94% slower)

Comment 9

5 years ago
Uhn, right, I'm stupid, sorry. I tested the no inner case only.
(In reply to Mike Shaver (:shaver -- probably not reading bugmail closely) from comment #0)
>  var createdClosure = (function() {
>   […]
>  })();

This pattern is optimized as we know that the function will only be called once, and that we do not have to create the functions multiple times.  This is done in the parser when we are creating the functions.  This optimization is made to handle the Module-like syntax of libraries.

>  var expressionClosure = function(a) {
>   […]
>  };
>  
>  function func(a) {
>   […]
>  }

At the opposite of these functions where we might call the outer functions multiple times.
(In reply to Nicolas B. Pierron [:nbp] from comment #10)
> At the opposite of these functions where we might call the outer functions
> multiple times.

Note comment 2 and comment 3: this is about adding an analysis that'd tell us if the cloning is required at all, even for non-run-once outer functions.
Status: REOPENED → NEW
Assignee: general → nobody
You need to log in before you can comment on or make changes to this bug.