Closed
Bug 594233
Opened 14 years ago
Closed 14 years ago
We seem to be hitting a stubs::Mod slow path on this testcase
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Unassigned)
References
()
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
5.74 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
Takes up about 11% of the time. More than half of that is js::ValueToNumberSlow (good name!). The testcase is also attached in bug 594230 if it ever goes away.
Comment 1•14 years ago
|
||
This testcase has this code: Number.prototype.m = function (a) { return ((this % a) + a) % a }; Which makes everything really slow, because of the the number object accessed via 'this'. I implemented a fast path for number objects in DefaultValue exactly like these for string objects. for (i = 0; i <= 1000000; i++) { i.m(a); } This micro benchmark improved from 486 to 343 with js -m.
Comment 2•14 years ago
|
||
Updated•14 years ago
|
Attachment #483931 -
Flags: review?(lw)
Comment 3•14 years ago
|
||
Comment on attachment 483931 [details] [diff] [review] fast path for numbers object in DefaultValue Great job! r+ with nits: >+ ClassMethodIsNative(cx, obj, >+ &js_StringClass, > ATOM_TO_JSID(cx->runtime->atomState.toStringAtom), > js_str_toString)) { Nit: need to fixup the indentation here and below. >+ /* Optimize (new Number(...)).valueOf(). */ >+ if (obj->getClass() == &js_NumberClass && >+ ClassMethodIsNative(cx, obj, >+ &js_NumberClass, >+ ATOM_TO_JSID(cx->runtime->atomState.valueOfAtom), >+ js_num_valueOf)) { Nit: could you factor out the obj->getClass() between the js_StringClass/js_NumberClass cases into a local var and put both the string/number cases in a single compound if?
Attachment #483931 -
Flags: review?(lw) → review+
Comment 4•14 years ago
|
||
bz, does this patch take us to where we want to be? To go further we could do something with PICs so that we never leave jit code.
Reporter | ||
Comment 5•14 years ago
|
||
Well, I dunno where we want to be. ;) Without this patch, time under stubs::Mod is 14% of total on m-c tip. With this patch, it's 7% of total. So better, for sure. The fmod time is the same for both (2.4%), the self time is the same (1.2%), js_ValueToNumberSlow went from 10.5% to 2.9% (mostly searching property tables and self time in DefaultValue). I suspect short of not having a stub call here at all we're pretty good... I don't know whether avoiding the stub call in this situation is reasonable or desirable.
Comment 6•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/759533cf3c38 To wit, I get a 3x speedup (-m) with this patch on var n = new Number(3.4); for (var i = 0; i < 1000000; ++i) { n + n; }
Whiteboard: fixed-in-tracemonkey
Comment 7•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/759533cf3c38
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•