Closed
Bug 776022
Opened 12 years ago
Closed 12 years ago
IonMonkey: Concatenating String objects is slow
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: jandem, Assigned: jandem)
References
(Blocks 2 open bugs)
Details
Attachments
(1 file)
15.66 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
We're slower than JM on SS date-format-xparb, because the benchmark uses String objects a lot. If I replace "new String" in String.leftPad with just "String" --no-jm is as fast as --no-ion. On the micro-benchmark below we have: JM+TI : 1010 ms Ion : 3046 ms A profile shows a lot of time under InlineFrameIterator, SnapshotReader, etc so it looks like the concatenation somewhere queries the current script/pc. It seems like there are two solutions: 1) Add a cache to get the current script/pc. 2) Specialize String objects. I think we should really try (1) first because there are other operations which need the current script/pc, and specializing all of them seems like it will be a lot of work. We can always do (2) too if needed. function f() { var s = new String("f"); var res = ""; for (var i=0; i<10000000; i++) { res += s; } return res; } var t = new Date; f(); print(new Date - t);
Comment 1•12 years ago
|
||
What is the callpath that asks for the current script/pc?
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Luke Wagner [:luke] from comment #1) > What is the callpath that asks for the current script/pc? MonitorString in AddOperation. Looking at AddOperation there seems to be an easy fix for this particular case: we only have to call MonitorString if both operands are not strings. In this case one operand is a string so we don't have to monitor the result. But this will fail if both operands are String objects, it would be good to fix that as well.
Comment 3•12 years ago
|
||
Given that TI penalizes any time in the VM, I would guess the right fix is to somehow stay in jit code (or at least stay in C++ stub calls that don't need the general VM type-monitoring path).
Assignee | ||
Updated•12 years ago
|
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•12 years ago
|
||
I tried to use MMonitorTypes to do the monitoring in JIT code, but it turns out the dynamic monitoring for JSOP_ADD etc (MonitorString, MonitorOverflow, etc) is quite different from the Monitor(..) calls for JSOP_GETPROP etc and the patch got pretty hackish and complicated. So instead I went with the simpler approach of passing the script/pc to the stub. The micro-benchmark in comment 0 is now about as fast as JM+TI and we can always win more by specializing more cases - that's going to be faster than the generic stubs anyway. On date-format-xparb Ion is now about as fast as JM+TI, Ion+JM is still a bit slower but that's most likely bug 775818.
Attachment #646513 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #646513 -
Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/ee40f69169e9
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•