[OpenJDK 2D-Dev] RFR 8144446: Automate the Marlin crash test

[Jim Graham]

Ignore that first paragraph - a thought train that didn't go anywhere 
and I forgot to delete it...

			...jim

On 12/9/15 4:27 PM, Jim Graham wrote:
> Hi Laurent,
>
> I did some more reading about jtreg and discovered that an @run is
> supposed to be assumed if none are present, but
>
> The fix looks correct, but one thing I would tend to do for robustness
> is that in an error case, rather than duplicate the logic that was
> skipped (which can get out of date if we later change how the bounds*Y
> variables are calculated), I would just hardcode the bounds*Y variables
> to the worst case min/max so that we do a complete fill on the
> variables.  For error cases it is less interesting to optimize out every
> memory store and more interesting to make sure that we robustly restore
> the state.  Another option would be to move the bounds logic to a
> separate function that is called in both the error and the success cases?
>
> For the test, you can have multiple test tags and include an @ignore so
> that the primary tests are run every time and the ones after the ignore
> are only run if someone runs with "-ignore:run".  That makes them
> runnable from the command line without having to edit the test:
>
> @run main/othervm -mx512m CrashTest
> @ignore tests that take a long time
> @run main/othervm -mx512m CrashTest -slow
>
> The first line would be run in all cases, the second line would only be
> run if they specify "-ignore:run" on the command line.
>
> The only down side is that the tests after the @ignore are shown on the
> final statistics as "errors" which seems kind of melodramatic, but
> that's why the "-ignore:quiet" option exists.  There are quite a few
> tests in the java hierarchy with an @ignore tag, though, often talking
> about extreme memory requirements so this is nothing new.  This would be
> the first in the sun/java2d hierarchy, though...
>
>          ...jim
>
> On 12/9/15 3:10 PM, Laurent Bourgès wrote:
>> Jim,
>>
>> My last chance for tonight !
>>
>> Here is another webrev that disables two long tests (dasher) in
>> CrashTest but fixes a state cleanup bug in Renderer (doChecks=true):
>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144446.2/
>>
>> Note: the modified CrashTest detected this bug in Renderer (happening
>> only with 2Gb off-heap overflow) so I keep both classes together in the
>> same patch as the fix in Renderer is very small.
>>
>> The CrashTest seems faster now.
>>
>> Laurent
>>
>> 2015-12-09 23:31 GMT+01:00 Jim Graham <james.graham at oracle.com
>> <mailto:james.graham at oracle.com>>:
>>
>>     Hi Laurent,
>>
>>     That sounds good.  I'm all for fast tests!  ;)
>>
>>     We might want to fix them in separate bugs, though.  If the new mods
>>     to the test case lead to failures, then we should integrate them
>>     after we fix the underlying problems, though, to prevent testing
>>     failures that might block an integration...
>>
>>                              ...jim
>>
>>     On 12/9/15 2:25 PM, Laurent Bourgès wrote:
>>
>>         Jim,
>>
>>         Thanks for explaining me the different jtreg modes (newbie) !
>>
>>         I tried disabling few long tests (using dashes) that are less
>>         critical
>>         (no failure expected, just insane rendering work):
>>                       test(0.1f, false, 0);
>>                       test(0.1f, true, 7f);
>>
>>         Doing so, I detected a new issue in the Renderer.dispose()
>> when the
>>         addLine() fails due to the AIOB (2GB off-heap overflow):
>>         the range buckets_minY/maxY are not properly set, normally by
>>         endRendering(), and the edgeBucket arrays are not properly
>>         zero-filled !
>>
>>         I will work on this issue ASAP and propose a fix within the same
>>         bug.
>>
>>         Maybe we should defer this fix as I agree it can be made faster
>>         ~ 12s
>>         now vs 52s before on my laptop.
>>
>>         Laurent
>>
>>         2015-12-09 22:31 GMT+01:00 Jim Graham <james.graham at oracle.com
>>         <mailto:james.graham at oracle.com>
>>         <mailto:james.graham at oracle.com
>> <mailto:james.graham at oracle.com>>>:
>>
>>
>>              Hi Laurent,
>>
>>              One clarification - there are levels of automation.
>>
>>              On 12/9/15 6:35 AM, Laurent Bourgès wrote:
>>
>>                  Agreed it is possible but then the bug JDK-8144446
>>                  <https://bugs.openjdk.java.net/browse/JDK-8144446>
>>         becomes invalid.
>>
>>
>>              Prior to this fix, jtreg wouldn't even see the test since
>>         it did not
>>              have any tags in it at all.  Adding tags of "some sort"
>>         makes it
>>              able to be run by the test mechanism, which I call
>> "automated".
>>              Right now, nobody who runs jtreg will run this test no
>>         matter what
>>              command line arguments they use with the tool.
>>
>>              Once jtreg recognizes a test there are variations that let
>>         it decide
>>              when/if to run it.  It has 3 main modes (related to the
>>         /manual tag):
>>
>>              no options - all tests are run, both manual and automatic
>>              -a - ignore all /manual tests
>>              -m - run only /manual tests
>>
>>              -a primarily means "there is no human here to provide
>>         interaction",
>>              but a few non-manual tests take a long time.
>>
>>              On the other hand, I just did a test run of all tests (with
>>         -a) in
>>              sun/java2d and the total time was so long that the 30
>>         seconds wasn't
>>              that noticeable.  On the other hand, there were a lot of
>>         tests run
>>              so the long time was less because a lot of tests take a
>>         long time
>>              than it was about the fact that a lot of conditions were
>>         tested in
>>              that time. For the record, the next longest test in that
>>         part of the
>>              repo takes 8 seconds, so this new test is almost 5 times
>>         longer than
>>              any existing java2d test.  Only the longest 4 tests took
>>         more than 5
>>              seconds.
>>
>>              I did find a test with "@ignore slow test" in another part
>>         of the
>>              repo and I ran it and it took 8 seconds as well, so
>>         somebody out
>>              there considers 8 seconds to be too long to run under
>> ordinary
>>              circumstances.
>>
>>              I tend to want to push hard on making tests be faster and
>>         leaner.  I
>>              see so many bug fixes come in with automated tests that
>>         only have to
>>              run a single method and see if it returns the right answer
>>         and yet
>>              somehow the test needs to launch a Frame and wait for it to
>>         open and
>>              then do a screen readback - when a simple render to a
>>         BufferedImage
>>              would take 1/100th the time.
>>
>>              I'll withdraw my suggestion to make this one /manual, but
>>         it would
>>              be nice if it could do its work in just a few seconds
>>         instead...
>>
>>
>>
>>
>> --
>> --
>> Laurent Bourgès