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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Thomas Schatzl
Hi all,

  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.

CR:
https://bugs.openjdk.java.net/browse/JDK-8190703
Webrev:
http://cr.openjdk.java.net/~tschatzl/8190703/webrev
Testing:
Ran the specific tests in our test lab on slow configurations (SPARCv9,
debug build), checking manually that they do not exceed their specified
time out.

Thanks,
  Thomas

Reply | Threaded
Open this post in threaded view
|

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

Aleksey Shipilev-4
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);

*) This change also does not look right -- should we instead increase delays in callers?

 101             ThreadUtils.sleep(delayMS * 10);


*) I think you can just say:

 for (int i = 0; (i < 4) && (System.currentTimeMillis() < endTime); i++)

 ...instead of rewriting the whole loop.

-Aleksey


signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Thomas Schatzl
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

Reply | Threaded
Open this post in threaded view
|

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

Stefan Johansson
Hi Thomas,

On 2017-11-07 10:16, Thomas Schatzl wrote:
>
> 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)
These changes looks good, thanks for fixing this.
StefanJ
> I rechecked that these changes do not change the execution times of the
> test.
>
> Thanks,
>    Thomas
>

Reply | Threaded
Open this post in threaded view
|

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

Aleksey Shipilev-4
In reply to this post by Thomas Schatzl
On 11/07/2017 10:16 AM, Thomas Schatzl wrote:
> http://cr.openjdk.java.net/~tschatzl/8190703/webrev.1/ (full patch)
> http://cr.openjdk.java.net/~tschatzl/8190703/webrev.0_to_1/ (diff)

Looks good.

-Aleksey



signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

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

Thomas Schatzl
Hi Aleksey, Stefan,

On Tue, 2017-11-07 at 13:22 +0100, Aleksey Shipilev wrote:
> On 11/07/2017 10:16 AM, Thomas Schatzl wrote:
> > http://cr.openjdk.java.net/~tschatzl/8190703/webrev.1/ (full patch)
> > http://cr.openjdk.java.net/~tschatzl/8190703/webrev.0_to_1/ (diff)
>
> Looks good.
>
> -Aleksey

On Tue, 2017-11-07 at 12:50 +0100, Stefan Johansson wrote:
> Hi Thomas,

[...] 
> These changes looks good, thanks for fixing this.
> StefanJ


  thanks for your reviews :) Is on its way...

Thomas