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