Bug 20209 – Atomize constant strings

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

Example of code that currently performs poorly:

function get(self, keyword) {
    // do stuff
    return self[keyword];
}

function f1() {
    var localGet = get;
    var object = {};
    var start = new Date;
    for (var i =0; i < 10000000; i++)
        get(object, 'foo');
    print("f1 time taken: " + (new Date - start)/1000 + "s");
}

function f2() {
    var object = {};
    var startTime = new Date;
    for (var i =0; i < 10000000; i++)
        object['foo'];
    print("f2 time taken: " + (new Date - startTime)/1000 + "s");
}

f1();
f2();

The problem in the above script is that the string 'foo' is used twice as an identifier.  Only 'foo' from f1() will be added to the identifier map, degrading the performance of f2().  Fix this by turning constant strings into identifiers during code generation.

(patch following).

[Gavin Barraclough]

Created attachment 22537 [details]
fix

[Alexey Proskuryakov]

I'm getting some weird results on the above test with a r35249 nightly. f1 takes ~2.3s, while f2 alternates between 1.6s and 0.9s (odd runs are slow, even runs are fast, no values in between).

On a Shark profile of a slow run, 36% of time is spent in Identifier::addSlowCase, and for fast runs, this function is not on the profile.

[Geoffrey Garen]

> I'm getting some weird results on the above test with a r35249 nightly. f1
> takes ~2.3s, while f2 alternates between 1.6s and 0.9s (odd runs are slow, even
> runs are fast, no values in between).

Perhaps the garbage collector's effect on the lifetime of an identifier is in play here.

[Geoffrey Garen]

Comment on attachment 22537 [details]
fix

Note that the patch I suggested to convert a['s'] to a.s during codegen would also fix this particular test case.

+    // String literals are atomized (internalized) in the identifier map.

Probably better to state the why here, rather than the what. For example, "We atomize constant strings, in case they're later used in property lookup."

+        a script contains multiple identical strings that are used as keys keys to identify

Typo: "keys keys" should be "keys."

Please add a bugzilla link to the ChangeLog.

r=me

[Gavin Barraclough]

(In reply to comment #4)
> (From update of attachment 22537 [details] [edit])
> Note that the patch I suggested to convert a['s'] to a.s during codegen would
> also fix this particular test case.

Ah yes, that's quite true, thanks – I had writing a test case for this on my todo list, since we don't hit it in sunspider – that's one problem solved! – will raise a bug & submit a patch.
G.

[Gavin Barraclough]

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/kjs/nodes.cpp
Transmitting file data ..
Committed revision 35446.

[Gavin Barraclough]

Optimize [ "constant string" ] into an identifier access.

https://bugs.webkit.org/show_bug.cgi?id=20227