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.gra...@oracle.com
<mailto:james.gra...@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.gra...@oracle.com
        <mailto:james.gra...@oracle.com>
        <mailto:james.gra...@oracle.com <mailto:james.gra...@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

Reply via email to