Closed
Bug 776022
Opened 11 years ago
Closed 11 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•11 years ago
|
||
What is the callpath that asks for the current script/pc?
Assignee | ||
Comment 2•11 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•11 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•11 years ago
|
Assignee: general → jdemooij
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•11 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•11 years ago
|
Attachment #646513 -
Flags: review?(dvander) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/ee40f69169e9
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•