Bug 113668 - fourthTier: FTL JIT should support GetByVal on Int32 arrays
Summary: fourthTier: FTL JIT should support GetByVal on Int32 arrays
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 112840
  Show dependency treegraph
 
Reported: 2013-03-31 17:38 PDT by Filip Pizlo
Modified: 2013-03-31 21:19 PDT (History)
7 users (show)

See Also:


Attachments
the patch (3.13 KB, patch)
2013-03-31 17:48 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (3.67 KB, patch)
2013-03-31 18:01 PDT, Filip Pizlo
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2013-03-31 17:38:52 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2013-03-31 17:48:44 PDT
Created attachment 195910 [details]
the patch
Comment 2 Filip Pizlo 2013-03-31 17:50:58 PDT
This allows me to measure this program, and see what is going on:

function foo(o) {
    var result = 0;
    for (var i = 0; i < o.length; ++i)
        result += o[i];
    return result;
}

var array = [];
for (var i = 0; i < 1000; ++i)
    array.push(i);

eval("// Blah"); // prevent DFG compilation of global code.

function run(whichRun, numRuns) {
    eval("// Blah"); // prevent DFG compilation of this function.
    
    var result = 0;
    var before = preciseTime();
    for (var i = 0; i < numRuns; ++i)
        result += foo(array);
    var after = preciseTime();
    
    var expected = numRuns * (array.length - 1) * array.length / 2;
    if (result != expected)
        throw "Error: bad result: " + result + "; expected: " + expected;
    print(whichRun + ": " + numRuns +" iterations took " + (after - before) * 1000 + " ms.");
}

run("Warmup", 1000);
run("Warmup", 10000);
run("Measurement", 100000);


Here are the results without FTL:

[pizlo@dethklok OpenSource] DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ WebKitBuild/Release/jsc --useExperimentalFTL=false test3.js
Warmup: 1000 iterations took 3.092050552368164 ms.
Warmup: 10000 iterations took 20.300865173339844 ms.
Measurement: 100000 iterations took 198.03905487060547 ms.


And with FTL:

[pizlo@dethklok OpenSource] DYLD_FRAMEWORK_PATH=WebKitBuild/Release/ WebKitBuild/Release/jsc --useExperimentalFTL=true test3.js
Warmup: 1000 iterations took 6.757020950317383 ms.
Warmup: 10000 iterations took 14.333009719848633 ms.
Measurement: 100000 iterations took 141.18385314941406 ms.


It's interesting that we pay a ~3.7ms cost to compile this function, but it pays off not too long after that.  Note that 3.7ms includes doing all of the initialization that the FTL does (JIT execution engine creation, module creation, blah, blah).  I suspect we can bring this down if we wanted to just by doing smarter things on our end.
Comment 3 Filip Pizlo 2013-03-31 18:01:40 PDT
Created attachment 195912 [details]
the patch
Comment 4 Sam Weinig 2013-03-31 19:23:24 PDT
Comment on attachment 195912 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195912&action=review

> Source/JavaScriptCore/ChangeLog:10
> +        Also adds an option to enable LICM. LICM isn't doing me any good right

Since people in this project might not know what LICM is, can you please expand this abbreviation the first time you use it.
Comment 5 Filip Pizlo 2013-03-31 21:18:26 PDT
(In reply to comment #4)
> (From update of attachment 195912 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=195912&action=review
> 
> > Source/JavaScriptCore/ChangeLog:10
> > +        Also adds an option to enable LICM. LICM isn't doing me any good right
> 
> Since people in this project might not know what LICM is, can you please expand this abbreviation the first time you use it.

Yup, will do!
Comment 6 Filip Pizlo 2013-03-31 21:19:14 PDT
Landed in http://trac.webkit.org/changeset/147297