Closed Bug 602262 Opened 10 years ago Closed 9 years ago

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

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: luke, Assigned: luke)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Great find!
Nice! Also because this pattern is quite common in real-world code.
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
Depends on: 602129
Summary: Speculatively transform f.apply(x, [args]) into f.call(x, args) → JM: make f.apply(x, [args]) fast
Attached patch WIP 1 (obsolete) — Splinter Review
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.
Assignee: general → lw
Status: NEW → ASSIGNED
Attached patch patchSplinter Review
Green on try.
Attachment #485940 - Attachment is obsolete: true
Attachment #486282 - Flags: review?(dvander)
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.
Attachment #486282 - Flags: review?(dvander)
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.