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

Status: NEW

(Core :: JavaScript Engine, defect)

Description

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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.)

Comment 1

[Brendan Eich [:brendan]]

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

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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.

Comment 3

[Brendan Eich [:brendan]]

(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

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

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?)

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

profile for func is basically identical.

Comment 6

[Guilherme Lima]

Usint the testcase from jsperf I get the same numbers on all 3 cases, and Nightly 32 is 34% faster than Chrome 35.

Comment 7

[Stefan Fleiter (:sfleiter)]

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.

Comment 8

[Zibi Braniecki [:zbraniecki][:gandalf]]

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

[Guilherme Lima]

Uhn, right, I'm stupid, sorry. I tested the no inner case only.

Comment 10

[Nicolas B. Pierron [:nbp]]

(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.

Comment 11

[Till Schneidereit [:till]]

(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.