786491 - Fix v8-deltablue regression from bug 778724

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

dmandelin saw this in the browser and I'm able to confirm it in the shell (not sure why it didn't show earlier).  After bug 778724 v8-deltablue is no longer inlining calls to Array.push, which hurts it significantly.  This is because of a change in bug 778724 which speculatively marks the result of calls whose return value is ignored as potentially undefined, which hurts type information and breaks the Array.push fast path (which wants to see a known int32 result for the call).

This may not be the only fallout from the change, too.  If I just fix Array.push, score goes from 7200 to 9400, but if I remove the type despecialization entirely I get 9600.  (I get 9500 before bug 778724; there seem to be significantly fewer compilations post-778724).

The point of the type despecialization is to avoid recompiling large swaths of a script when it calls a function that has never been called before, adding a void type to the call site which is promptly popped.  There's a better mechanism for this now, though, used for avoiding recompilation after new overflows in (a + b) | 0 etc. operations.  This fix reverts the type despecialization change, and switches to this other mechanism to avoid any new recompilation activity in large scripts.

Comment 1

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce700bebd73c

Comment 2

[Brian Hackett [Laid off!]]

Actually, I'm a moron, the mechanism for (a + b) | 0 won't apply to call results as they are JOF_TYPESET opcodes and use TypeMonitorResult rather than TypeDynamicResult.  There isn't a simple fix to avoid the new recompilations, but I need to make another pass over this stuff for bug 785905 anyways and will clean things up there.

Comment 3

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ce700bebd73c

Comment 4

[David Mandelin [:dmandelin]]

Luke, could you land this on the 17 branch?

Comment 5

[Luke Wagner [:luke]]

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 778724
User impact if declined: perf regression
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): low

Comment 6

[Luke Wagner [:luke]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4840dbaa967b

Comment 7

[David Mandelin [:dmandelin]]

(In reply to Luke Wagner [:luke] from comment #6)
> https://hg.mozilla.org/releases/mozilla-aurora/rev/4840dbaa967b

Thanks again for picking up the Aurora landing.