Excellent ! It seems to me that my bug sprint on Marlin is done except the new one created as a follow-up.
Please push them all ! I will now work on other improvements: - Png compression (in progress) - handling NaN / huge coords in Marlin Thanks for all reviews, Cheers, Laurent Le 10 déc. 2015 23:26, "Jim Graham" <james.gra...@oracle.com> a écrit : > Looks good... > > I ran it both ways and got similar run times... > > ...jim > > On 12/10/15 1:14 PM, Laurent Bourgès wrote: > >> Hi Jim, >> >> Here is the updated webrev: >> http://cr.openjdk.java.net/~lbourges/marlin/marlin-8144446.3/ >> >> 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? >> >> >> Fixed: I agree it is better to clear completely bucket arrays. >> >> 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. >> >> >> Fixed: I adopted your approach and it works well: >> >> ----------messages:(3/129)---------- >> command: main -mx512m CrashTest >> reason: User specified action: run main/othervm -mx512m CrashTest >> elapsed time (seconds): 8.318 >> >> ----------messages:(3/150)---------- >> command: main -ms4g -mx4g CrashTest -slow >> reason: User specified action: run main/othervm -ms4g -mx4g CrashTest >> -slow >> elapsed time (seconds): 49.777 >> >> >> 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... >> >> >> I enabled also the huge image test that consumes ~5Gb and it passes on >> my laptop (16Gb). >> >> Cheers, >> Laurent >> >
