Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bhackett, Assigned: dvander)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(2 attachments)

Reporter

Description

9 years ago
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.
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.
Reporter

Comment 2

9 years ago
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.
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.
Reporter

Comment 4

9 years ago
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.
Posted patch patchSplinter Review
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
Assignee: general → dvander
Status: NEW → ASSIGNED
Attachment #486052 - Flags: review?(dmandelin)
Attachment #486052 - Flags: review?(dmandelin) → review+
(This was about a 1-2ms win on date-format-xparb, as Brian hinted.)

Updated

9 years ago
Depends on: 608810
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
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
Duplicate of this bug: 609082

Updated

9 years ago
Depends on: 609200

Comment 11

9 years ago
Did this get backed out of TM? If not this (and the regressions) will show up on m-c again when we merge...
Attachment #488601 - Flags: review?(dmandelin) → review+

Comment 14

9 years ago
http://hg.mozilla.org/mozilla-central/rev/894e42d25be9
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.