Bug 138483 – Refactor the get-by-pname.js test

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

[Mark Lam]

Refactored the test code to:
1. make it easier to add more test cases.
2. test each case against different tier JITs.
3. print only one set of results for each tier being tested.

[Mark Lam]

Created attachment 241140 [details]
the patch.

[Geoffrey Garen]

Comment on attachment 241140 [details]
the patch.

r=me

You should verify that this test runs in less than 200ms.

[Mark Lam]

Thanks for the review.  Landed in r175722: <http://trac.webkit.org/r175722>.

[Michael Saboff]

Comment on attachment 241140 [details]
the patch.

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

> LayoutTests/js/script-tests/get-by-pname.js:31
> +var testCases = [
> +    [ "foo(o)", "11" ],
> +    [ "foo(p)", "10" ],
> +    [ "foo(q)", "3467" ],
> +    [ "foo(r)", "113" ],
> +    [ "foo(s)", "182" ],
> +];
> +
> +function testExpr(index) {
> +    return testCases[index][0];
> +}
> +function testExpectedResult(index) {
> +    return testCases[index][1];
> +}

Instead of using indices, use named properties.
e.g.
var testCases = [
    { expr : "foo(o)", expected : "11" },
...

> LayoutTests/js/script-tests/get-by-pname.js:41
> +var tiers = [
> +    // name, iterations
> +    [ "cold", 0 ],
> +    [ "llint", 10 ],
> +    [ "baseline", 500 ],
> +    [ "dfg", 1000 ],
> +    // [ "ftl", 100000 ],
> +];

Same thing with named properties.

[Michael Saboff]

Comment on attachment 241140 [details]
the patch.

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

> LayoutTests/js/script-tests/get-by-pname.js:24
> +    [ "foo(o)", "11" ],
> +    [ "foo(p)", "10" ],
> +    [ "foo(q)", "3467" ],
> +    [ "foo(r)", "113" ],
> +    [ "foo(s)", "182" ],
> +];

Won't having a function name listed in multiple test cases cause it to tier up that much faster?  foo() is in the list 5 times, so this should be factored into the tiering.

[Mark Lam]

Created attachment 241143 [details]
follow up patch.

[Michael Saboff]

Comment on attachment 241143 [details]
follow up patch.

This looks fine.  There is still the issue that the tiering for each of the test functions will happen at different loop iterations due to functions appearing in the test list multiple times.

[Mark Lam]

Thanks.  Follow up 1 landed in r175729: <http://trac.webkit.org/r175729>.

[Mark Lam]

Created attachment 241149 [details]
follow up 2: give each test case its own unique test function

[Michael Saboff]

Comment on attachment 241149 [details]
follow up 2: give each test case its own unique test function

r+

[Mark Lam]

Thanks.  Follow up 2 landed in r175731: <http://trac.webkit.org/r175731>.