602262 - JM: make f.apply(x, [args]) fast

Status: RESOLVED WONTFIX

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

Although there is always the chance that Function.prototype.apply has been overwritten, we can speculatively transform expressions of the form
  f.apply(x, [arg1, ..., argn])
into
  f.call(x, arg1, ..., argn)
and insert a guard that f.apply yields js_fun_apply (which can fall back to a slow path to do the right thing).  I don't know if there are other subtle semantic differences between the two, but JSC does a similar transform, so it should be doable.

This pattern shows up in SS in string-tagcloud, which does 10K such calls.  Just manually doing the rewrite (without the guard) gives a 4ms speedup (28 to 24ms).  Furthermore, bug 602129 should make f.call Really Fast which, according to Shark, would win an additional ~1.5ms on string-tagcloud.

JM wanted SS win: 5ms.

Comment 1

[David Mandelin [:dmandelin]]

Great find!

Comment 2

[Jan de Mooij [:jandem]]

Nice! Also because this pattern is quite common in real-world code.

Comment 3

[Brendan Eich [:brendan]]

Yeah, this is like Scheme compiler 101 stuff, we should do it.

Apart from guarding that apply is still apply and using the original value of call, there's no semantic mismatch. An array initialiser makes a real Array using the original Array.prototype for the scope chain's global object, and does not call the Array constructor.

/be

Comment 4

[Luke Wagner [:luke]]

Attachment: WIP 1

Patch is lightly tested and applies on top of bug 595884.  Testing:

  function f(x) { return x+1 }
  f.apply(null, [3])

in a tight loop shows a 15x speedup.  string-tagcloud shows a total speedup of 2-4ms.  This is less than the 5ms predicted in comment 0.  I believe the reason is that bug 584917 (which sped up array literal construction) landed since the initial measurement.

Comment 5

[Luke Wagner [:luke]]

Attachment: patch

Green on try.

Comment 6

[Luke Wagner [:luke]]

Comment on attachment 486282 [details] [diff] [review]
patch

I'm going to put this on hold for now: bug 606477 wants to remove JSOP_NEWARRAY (and emit NEWINIT/INITELEM*/ENDINIT instead) and this patch isn't as big of a win as it initially was (it'll be even less still after 606477 speeds up array initialization).  I guess I'd want to see how often real-world code actually does f.apply(x, [y])... it seems like a pretty dim thing to write.

Comment 7

[Luke Wagner [:luke]]

I instrumented a browser to count the number of optimizaable JSOP_NEWARRAY/JSOP_FUNAPPLY pairs executed, and it shows up very infrequently -- less than 1K times for a 10-minute browsing session.  Fun fact: Myspace has a FarmVille knock-off called BarnBuddy.  WONTFIXing.