776022 - IonMonkey: Concatenating String objects is slow

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

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

[Luke Wagner [:luke]]

What is the callpath that asks for the current script/pc?

Comment 2

[Jan de Mooij [:jandem]]

(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

[Luke Wagner [:luke]]

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).

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

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.

Comment 5

[David Anderson [:dvander] - inactive, e-mail if emergency]

http://hg.mozilla.org/projects/ionmonkey/rev/ee40f69169e9