1130397 - fun.asApply breaks SpiderMonkey lazy-arguments optimization

Status: RESOLVED FIXED

(Firefox Graveyard :: Shumway, defect)

Description

[Jan de Mooij [:jandem]]

In avm2.js, in makeTrampoline, we return a function (trampoline) that does:

    return target.asApply(this, arguments);

It'd be great if we could change this to target.apply, so that SpiderMonkey can optimize this better (it can get rid of the arguments-object allocation and probably inline the call better).

This is a serious problem on deltablue.swf for instance. On top of a WIP patch for bug 1129382 we're like 3x faster with this fixed and faster than V8.

Comment 1

[Till Schneidereit [:till]]

Great find! I fixed that and a few other sites that used `asApply` to use `apply` instead. It's not always possible to do so, because we have to support functions that're both constructors and callables, and, though I don't remember the details right now, asApply is required there. My changes should cover all the really hot usages, though.

I'm seeing my deltablue score go from ~950 to ~1750.

Comment 2

[Jan de Mooij [:jandem]]

(In reply to Till Schneidereit [:till] from comment #1)
> I fixed that and a few other sites that used `asApply` to use
> `apply` instead.

Awesome, thanks! AWFY confirms the deltablue win:

http://arewefastyet.com/#machine=28&view=breakdown&suite=shumway

However it also shows huge regressions on some tests. Especially V8 regressed but also SM and JSC got slower on some tests. Any idea if that's related to this bug or to another commit?

Comment 3

[Till Schneidereit [:till]]

(In reply to Jan de Mooij [:jandem] from comment #2)
> However it also shows huge regressions on some tests. Especially V8
> regressed but also SM and JSC got slower on some tests. Any idea if that's
> related to this bug or to another commit?

Turns out this was caused by another change: https://github.com/mozilla/shumway/commit/394e196fdfe84a9f519e6b9b6e2c5e43dc41dea0

For some reason, using `eval(funSource)` instead of the more convoluted `new Function('return ' + funSource)()` causes serious slowdowns across engines. I have no idea why that is, but I changed it back to `new Function`.