Bug 120996

Summary: run-jsc-stress-tests should run tests in parallel if possible
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: Tools / TestsAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 120994    
Bug Blocks: 120696    
Attachments:
Description Flags
the patch
none
the patch
none
the patch
none
the patch oliver: review+

Description Filip Pizlo 2013-09-07 22:09:48 PDT
And it should do so with horrible dirty makefile hacks.
Comment 1 Filip Pizlo 2013-09-07 22:11:24 PDT
Created attachment 210962 [details]
the patch

This needs a rebase after https://bugs.webkit.org/show_bug.cgi?id=120994 lands.
Comment 2 Filip Pizlo 2013-09-07 22:33:07 PDT
Created attachment 210963 [details]
the patch
Comment 3 Filip Pizlo 2013-09-07 22:35:18 PDT
(In reply to comment #2)
> Created an attachment (id=210963) [details]
> the patch

This patch needs a rebase after https://bugs.webkit.org/show_bug.cgi?id=120994 lands.
Comment 4 Filip Pizlo 2013-09-08 11:03:18 PDT
Created attachment 210982 [details]
the patch
Comment 5 Filip Pizlo 2013-09-08 11:04:03 PDT
Created attachment 210983 [details]
the patch
Comment 6 Oliver Hunt 2013-09-08 11:14:48 PDT
Comment on attachment 210983 [details]
the patch

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

> Tools/Scripts/run-jsc-stress-tests:39
> +numProcessors = `sysctl -n hw.availcpu`.to_i

Does this work everywhere?  Should we have a /proc fallback? (I really can't remember what's available in linux anymore :( )
Comment 7 Filip Pizlo 2013-09-08 11:19:58 PDT
(In reply to comment #6)
> (From update of attachment 210983 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210983&action=review
> 
> > Tools/Scripts/run-jsc-stress-tests:39
> > +numProcessors = `sysctl -n hw.availcpu`.to_i
> 
> Does this work everywhere?  Should we have a /proc fallback? (I really can't remember what's available in linux anymore :( )

No, it doesn't. ;-)  Which is why I created this bug: https://bugs.webkit.org/show_bug.cgi?id=120809

Also, here's what happens on Linux if you do that:

irb(main):002:0> `sysctl -n hw.availcpu`.to_i
error: "hw.availcpu" is an unknown key
=> 0

I.e. that expression yields 0, which then disables parallel runs.  You also get that helpful warning on the console.
Comment 8 Filip Pizlo 2013-09-08 11:23:19 PDT
Landed in http://trac.webkit.org/changeset/155307
Comment 9 Geoffrey Garen 2013-09-08 12:15:24 PDT
Comment on attachment 210983 [details]
the patch

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

> Tools/Scripts/run-jsc-stress-tests:245
> +    # The goals of our parallel test runner are scalability and simplicity. The
> +    # simplicity part is particularly important. We don't want to have to have
> +    # a full-time contributor just philosophising about parallel testing.

+1

> Tools/Scripts/run-jsc-stress-tests:248
> +    # As such, we just pass off all of the hard work to 'make'. This creates a
> +    # dummy directory ("$outputDir/.parallel") in which we create a dummy

Unrelated to this patch, this got me thinking about how I would examine passing and failing results.

> $outputDir = Pathname.new("results") 

I think this would be slightly better named as jsc-stress-test-results, or some other meaningful prefix. I end up doing lots of testing with lots of results directories, so the prefix can help me know, at a glance, what's in the directory.
Comment 10 Filip Pizlo 2013-09-08 12:24:39 PDT
(In reply to comment #9)
> (From update of attachment 210983 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210983&action=review
> 
> > Tools/Scripts/run-jsc-stress-tests:245
> > +    # The goals of our parallel test runner are scalability and simplicity. The
> > +    # simplicity part is particularly important. We don't want to have to have
> > +    # a full-time contributor just philosophising about parallel testing.
> 
> +1
> 
> > Tools/Scripts/run-jsc-stress-tests:248
> > +    # As such, we just pass off all of the hard work to 'make'. This creates a
> > +    # dummy directory ("$outputDir/.parallel") in which we create a dummy
> 
> Unrelated to this patch, this got me thinking about how I would examine passing and failing results.

The stress tests are meant to be silent.  This tool doesn't intercept or pipe output.  Hence any failure output will go to the console.

For every test failure, this tool will create a WebKitBuild/<blah>/jsc-stress-results/<testname> script (for example <testname> might be regress/script-tests/adapt-to-double-divide.default) that can be used to reexecute the failing test exactly the way the tool ran it, *except for* the environment.  So you usually run it like:

DYLD_FRAMEWORK_PATH=$PWD/WebKitBuild/Debug sh WebKitBuild/Debug/jsc-stress-results/regress/script-tests/adapto-to-double-divide.default

Note that "$PWD/" because the script will chdir, so the path needs to be absolute.

> 
> > $outputDir = Pathname.new("results") 
> 
> I think this would be slightly better named as jsc-stress-test-results, or some other meaningful prefix. I end up doing lots of testing with lots of results directories, so the prefix can help me know, at a glance, what's in the directory.

That's just the default if you invoke this script directly.  When you run it via run-javascriptcore-tests (what you're supposed to do usually) then you get a results directory in $productDir/jsc-stress-results.  See run-javascriptcore-tests for how that's done - it's just passing the -o option to run-jsc-stress-tests.

My thinking was that if you *do* run the script directly, then it's great for it to create a results directory in the most tersely named way possible.  It's kind of convenient: "where are the results of the magical thing I just ran?" "oh, they're in a directory called 'results'".
Comment 11 Geoffrey Garen 2013-09-08 12:30:45 PDT
> For every test failure, this tool will create a WebKitBuild/<blah>/jsc-stress-results/<testname> script (for example <testname> might be regress/script-tests/adapt-to-double-divide.default) that can be used to reexecute the failing test exactly the way the tool ran it

That's pretty fantastic. I wonder if run-webkit-tests can take a page from this.