(Moved over from bug 933313, see comment 18 on that bug.) We need to preserve the benchmarks and test cases from the PJS GC development.

Test cases and benchmarks, sometimes with expected output. This unpacks into js/src/parjs-{test,benchmarks} and contains some benchmarks and stress tests, along with some largeish files for input and expected output. The convolve programs are all mostly the same, with slight changes to the kernel (use diff). These are too useful to sit around in my sandbox github repo, so I'd like to land them, but I've not spent any time to try to automate them yet. Since the benchmarks in parjs-benchmarks are obsolete (they use the old ParallelArray) this patch moves them into an obsolete/ subdirectory so that we can port them when we have time.

Why the decision to put stress tests in their own directory and not in jit-tests to be automated? Do they take too long to run?

(In reply to Shu-yu Guo [:shu] from comment #2) > Why the decision to put stress tests in their own directory and not in > jit-tests to be automated? Do they take too long to run? Yes, especially in debug builds. These are not too bad but I have others, that I haven't included yet because I need to add an introspection interface to discover the maximum nursery size, that can run for a while. Part of the reason for that is the bailout-and-start-over approach that we currently take when we have to collect the general heap.

(In reply to Lars T Hansen [:lth] from comment #3) > (In reply to Shu-yu Guo [:shu] from comment #2) > > Why the decision to put stress tests in their own directory and not in > > jit-tests to be automated? Do they take too long to run? > > Yes, especially in debug builds. > > These are not too bad but I have others, that I haven't included yet because > I need to add > an introspection interface to discover the maximum nursery size, that can > run for a while. Part of > the reason for that is the bailout-and-start-over approach that we currently > take when we have > to collect the general heap. An alternative is to add the cookie // |jit-test| slow; to the top of the slow tests. Tests marked as slow aren't run on TBPL, you can still explicitly run them from the jit_test.py harness. What do you think about adding the non-benchmarks as slow tests inside jit-tests?

(In reply to Shu-yu Guo [:shu] from comment #4) > > An alternative is to add the cookie > > // |jit-test| slow; > > to the top of the slow tests. Tests marked as slow aren't run on TBPL, you > can still explicitly run them from the jit_test.py harness. What do you > think about adding the non-benchmarks as slow tests inside jit-tests? In principle I'm fine with it. The only other concern that I had about that was that most of these tests must run in a directory that contains an input file, and I had not yet investigated how the test runner does that (either stand-alone or on the buildbot). But if the harness cd's into the directory containing the test before running it, it should be OK.

Yet one more concern is that these tests can be run in one of several modes, depending on command line parameters among other things, but there's no harm in selecting sensible defaults for that. (Just recording this so that I'll remember to do it.)

'scriptdir' is predefined and contains the directory of the script. So that's fine. I'll put these tests in jit-test/tests/pjsgc and add the cookie, as suggested.

Grumble. The harness does not have a way of running slow tests; you can disable slow tests with --no-slow, but this is already the default value. Seems like a bug.

The default for run_slow is False, so --no-slow has no effect and there's no way of running slow tests. This sets the default to true. From what I understand from the mozharness, the test runners already pass --no-slow as they should, so this change should not be observable except when manually running jit_test.

(In reply to Lars T Hansen [:lth] from comment #9) > From what I > understand from the mozharness, the test runners already pass --no-slow as > they should, so this change should not be observable except when manually > running jit_test. Does this mean we'll now run the slow jit-tests locally and didn't before? There are some really slow ones and it'd be good to avoid running these by default, also because they don't run on TBPL anyway. Maybe we should change it to a --slow flag that defaults to false?

(In reply to Jan de Mooij [:jandem] from comment #10) > (In reply to Lars T Hansen [:lth] from comment #9) > > From what I > > understand from the mozharness, the test runners already pass --no-slow as > > they should, so this change should not be observable except when manually > > running jit_test. > > Does this mean we'll now run the slow jit-tests locally and didn't before? Yes. > There are some really slow ones and it'd be good to avoid running these by > default, also because they don't run on TBPL anyway. Maybe we should change > it to a --slow flag that defaults to false? I can do that.

Annoyingly, once we start running slow tests we uncover a failure in an existing test case (Array-scanPar-sorted.js).

Comment on attachment 8434008 [details] [diff] [review] Add a --slow switch, leave --no-slow the default Review of attachment 8434008 [details] [diff] [review]: ----------------------------------------------------------------- Great.

This moves the test cases into jit-test/tests/parallel with a pjsgc- prefix for easy use. All are tagged as slow and have useful defaults (run once, produce limited output). (You need the other patch on this bug to actually run slow tests, and also see bug 1020110 for other bugs in the harness interacting with the ability to run these tests.)

Comment on attachment 8434030 [details] [diff] [review] Test cases with expected output The test cases probably need a guard on getBuildConfiguration().parallelJS.

TBPL run for the patch for the harness (also see bug 1020110): https://tbpl.mozilla.org/?tree=Try&rev=728bbe3c2d95

Comment on attachment 8434030 [details] [diff] [review] Test cases with expected output Review of attachment 8434030 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the comment below fixed. The files also have trailing whitespace, probably from copying and pasting. These are just test cases so I don't really care if those are fixed or not. Overall looks like some pretty solid benchmarks and tests. Great stuff. ::: js/src/jit-test/tests/parallel/pjsgc-cat-convolve-mapPar-boxed-shared.js @@ +23,5 @@ > + > +// NOTE: This test must use Array and Object, not TypedObject array > +// and struct, because there are currently primitives on TypedObject > +// array involving references that cause the PJS engine to bail out. > + This, and the other pjsgc- tests, should be protected by if (!getBuildConfiguration().parallelJS) quit(0); or if (!getBuildConfiguration().parallelJS || !this.TypedObject) quit(0);

Review comments fixed in this version. Try is green: https://tbpl.mozilla.org/?tree=Try&rev=983d1a95a574. (Note though that this patch is all --slow tests and benchmarks to be run manually, so there should be no impact.)