Closed Bug 576934 Opened 11 years ago Closed 11 years ago
Int(double) is slow when not tracing
From bug 454037, parseInt(double) is needlessly slow because it converts to a string and back. The patch for that bug only fixed the case when we're on trace, but we should change num_parseInt to behave that way, too. This helps in SunSpider's string-unpack-code, which does parseInt(c / a) about 10,000 times as a way of doing integer division. On moo, this change saves us about 10% to 20% of string-unpack-code, which is 1% to 2% of sunspider as a whole.
Everything except for unpack-code can probably be ignored.
Assignee: general → apierce
Status: NEW → ASSIGNED
This is for the moo branch.
Attachment #456024 - Flags: review?(dvander)
Very minor (and pre-existing) nit: +static inline jsdouble +DoubleToInt(jsdouble d) returns an integral double, so DoubleToInteger would be better. We (and IIRC the ECMA-262 spec) spell out "Integer" to mean integral double, while "Int" usually means jsint/int32. /be
Comment on attachment 456024 [details] [diff] [review] Special-case parseInt(double) in num_parseInt Nice find. r=me (w/ brendan's nits :)
Attachment #456024 - Flags: review?(dvander) → review+
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Looks like this change got accidentally removed when the fix for bug 577536 was merged from tracemonkey to moo. Here's a redo of the patch, updated for the surrounding code. This is for tracemonkey, since it looks like otherwise it would need another manual merge at the next merge from TM to moo, since the code has been updated again since the last merge.
Thanks for catching this. Relanded to JM: http://hg.mozilla.org/projects/jaegermonkey/rev/842dce90c28e Getting it landed on TM is still a good idea.
Based on the SS #s in this bug, it appears to be a clear win. If you look at AWFY, rev 89964b93d40d (before) and rev a2f760bfd2bd (after) are virtually identical in perf. Any thoughts as to why?
(In reply to comment #8) > Based on the SS #s in this bug, it appears to be a clear win. If you look at > AWFY, rev 89964b93d40d (before) and rev a2f760bfd2bd (after) are virtually > identical in perf. Any thoughts as to why? This was originally landed on July 6, which is off the graph on AWFY. It was a clear perf win on the graph at the time. The change was accidentally clobbered after 89964b93d40d, and then relanded before a2f760bfd2bd, so the big perf regression from the clobber doesn't show up on AWFY.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.