604031 - JM: fast path for JSOP_CALLELEM

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

The shell testcase for the gameboy emulator in bug 514765 uses CALLELEM in its dispatch loop, which is currently stubbed.  I think this is responsible for the slowdown remaining vs. v8 after accounting for bug 604029.

Comment 1

[David Anderson [:dvander] - inactive, e-mail if emergency]

Do you know here if the array if dense or not? It definitely sounds like a use case to keep in mind for bug 602641.

Comment 2

[Brian Hackett [Laid off!]]

It is dense.  I don't see where CALLELEM is more complex than GETELEM; do a getelem, then push the lval.  Will write a patch today.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

Okay, so no IC needed, but just keep in mind that "push the lval" comes with the catch that you can't just copy it, since the slow path could overwrite it.

By "use case", bug 602641 and 603779 are refactoring array use in JM. If this patch can wait, it'll be a lot cleaner.

Comment 4

[Brian Hackett [Laid off!]]

Yeah, this would be done by passing a flag to jsop_getelem, as the stub call would be pushing both values.  Also, date-format-xparb looks like it does 8000 callelems ("this[func]()", where func is one of two different strings) so using the getelem PIC on those would be nice.  No hurry here.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: patch

Stealing :) Sleep-deprived and this seemed like a good test of whether the new ICs were flexible (seems: they are).

before tm: 33 ms. - 212.9 MHZ
after  tm: 22 ms. - 319.3 MHZ
       v8: 25 ms. - 281.0 MHZ

Comment 6

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/tracemonkey/rev/4a6259d13882

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

(This was about a 1-2ms win on date-format-xparb, as Brian hinted.)

Comment 8

[Brendan Eich [:brendan]]

Blamed for bug 608810, a b7 blocker. This needs to come out of m-c and probably tm, where a reduced test goes in to guard that gate.

/be

Comment 9

[Brendan Eich [:brendan]]

No citation of m-c landing via hg.mozilla.org link here, with resolved fixed change to match -- so nothing to undo now that this patch has been backed out of m-c :-P.

Bug bookkeeping matters, be safe out there.

/be

Comment 11

[LegNeato]

Did this get backed out of TM? If not this (and the regressions) will show up on m-c again when we merge...

Comment 12

[David Anderson [:dvander] - inactive, e-mail if emergency]

Attachment: follow-up fix

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

re-landed with fix as http://hg.mozilla.org/tracemonkey/rev/894e42d25be9

Comment 14

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/894e42d25be9