RFR (S): 8190703: TestSystemGCWith* infrequently times out on SPARC

Thomas Schatzl thomas.schatzl at oracle.com
Tue Nov 7 09:16:50 UTC 2017


Hi Alexey,

  thanks for your review, and sorry for the bad quality of the patch :(

On Mon, 2017-11-06 at 20:46 +0100, Aleksey Shipilev wrote:
> On 11/06/2017 12:27 PM, Thomas Schatzl wrote:
> >   can I have reviews for this change the fixes intermittent
> > timeouts in
> > a stress test when run on slow machines and debug builds.
> > 
> > The fix is to simply let the test bail out early if time is up -
> > this
> > allows the test run through as intended in the many configurations
> > where it does not take excessive amount of time.
> 
> Makes sense. Real timeouts would still surface.
> 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8190703
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8190703/webrev
> 
> *) This change seems like a leftover:
> 
>   93 System.err.println("SystemGCTask with delay " + delayMS);

Yes.

> 
> *) This change also does not look right -- should we instead increase
> delays in callers?
> 
>  101             ThreadUtils.sleep(delayMS * 10);
> 

Another debugging leftover when I tried to come up with alternative
solutions.

> 
> *) I think you can just say:
> 
>  for (int i = 0; (i < 4) && (System.currentTimeMillis() < endTime);
> i++)
> 
>  ...instead of rewriting the whole loop.

Done.

Also added some very basic check that a timeout is passed to the main
program.

http://cr.openjdk.java.net/~tschatzl/8190703/webrev.1/ (full patch)
http://cr.openjdk.java.net/~tschatzl/8190703/webrev.0_to_1/ (diff)

I rechecked that these changes do not change the execution times of the
test.

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list