RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

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

RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Coleen Phillimore-3
Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
Thanks!
Coleen

-------------

Commit messages:
 - 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Changes: https://git.openjdk.java.net/jdk/pull/3375/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3375&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264672
  Stats: 7 lines in 1 file changed: 7 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3375.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3375/head:pull/3375

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Harold Seigel-2
On Wed, 7 Apr 2021 12:53:30 GMT, Coleen Phillimore <[hidden email]> wrote:

> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
> Thanks!
> Coleen

Changes look good.  One question: does 'ready' need to be volatile ?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Lois Foltan-3
In reply to this post by Coleen Phillimore-3
On Wed, 7 Apr 2021 12:53:30 GMT, Coleen Phillimore <[hidden email]> wrote:

> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
> Thanks!
> Coleen

LGTM.
Lois

-------------

Marked as reviewed by lfoltan (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

Coleen Phillimore-3
In reply to this post by Harold Seigel-2
On Wed, 7 Apr 2021 13:55:40 GMT, Harold Seigel <[hidden email]> wrote:

> Changes look good. One question: does 'ready' need to be volatile ?

I added volatile to ready.

Thanks Lois and Harold!

-------------

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

Coleen Phillimore-3
In reply to this post by Coleen Phillimore-3
> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
> Thanks!
> Coleen

Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:

  Add volatile

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3375/files
  - new: https://git.openjdk.java.net/jdk/pull/3375/files/d5991d9e..5958bdcb

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3375&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3375&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3375.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3375/head:pull/3375

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

Harold Seigel-2
On Wed, 7 Apr 2021 15:34:31 GMT, Coleen Phillimore <[hidden email]> wrote:

>> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
>> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
>> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
>> Thanks!
>> Coleen
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add volatile

LGTM!

-------------

Marked as reviewed by hseigel (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

David Holmes-2
In reply to this post by Coleen Phillimore-3
On Wed, 7 Apr 2021 15:34:31 GMT, Coleen Phillimore <[hidden email]> wrote:

>> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
>> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
>> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
>> Thanks!
>> Coleen
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>
>   Add volatile

Hi Coleen,

The synchronization in this test is highly suspect, but your change certainly helps address one of the race conditions.

Thanks,
David

test/hotspot/jtreg/runtime/ParallelLoad/MyLoader.java line 64:

> 62:     private Object sync = new Object();
> 63:     private Object thread_sync = new Object();
> 64:     private static volatile boolean ready = false;

The name "waiting" would be more appropriate.

-------------

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

Coleen Phillimore-3
On Thu, 8 Apr 2021 10:01:37 GMT, David Holmes <[hidden email]> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Add volatile
>
> test/hotspot/jtreg/runtime/ParallelLoad/MyLoader.java line 64:
>
>> 62:     private Object sync = new Object();
>> 63:     private Object thread_sync = new Object();
>> 64:     private static volatile boolean ready = false;
>
> The name "waiting" would be more appropriate.

Ok.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v3]

Coleen Phillimore-3
In reply to this post by Coleen Phillimore-3
> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
> Thanks!
> Coleen

Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:

  rename ready to waiting

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3375/files
  - new: https://git.openjdk.java.net/jdk/pull/3375/files/5958bdcb..0048f355

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3375&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3375&range=01-02

  Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3375.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3375/head:pull/3375

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out [v2]

Coleen Phillimore-3
In reply to this post by David Holmes-2
On Thu, 8 Apr 2021 10:03:23 GMT, David Holmes <[hidden email]> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Add volatile
>
> Hi Coleen,
>
> The synchronization in this test is highly suspect, but your change certainly helps address one of the race conditions.
>
> Thanks,
> David

Hopefully the synchronization will be more reliable with this change.  I'll watch for problems with this test.
Thanks for the reviews, Lois, Harold and David.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3375
Reply | Threaded
Open this post in threaded view
|

Integrated: 8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Coleen Phillimore-3
In reply to this post by Coleen Phillimore-3
On Wed, 7 Apr 2021 12:53:30 GMT, Coleen Phillimore <[hidden email]> wrote:

> Added a flag so that thread 2 will notify thread 1 after it's waiting, and not before.
> Ran 100x on macosx-x64-debug without timeout (actually ran original 100x without timeout also).
> The test relies on one thread waiting at a place where another can find the class loading in progress.  Alternate suggestions welcome.
> Thanks!
> Coleen

This pull request has now been integrated.

Changeset: 255afbea
Author:    Coleen Phillimore <[hidden email]>
URL:       https://git.openjdk.java.net/jdk/commit/255afbea
Stats:     7 lines in 1 file changed: 7 ins; 0 del; 0 mod

8264672: runtime/ParallelLoad/ParallelSuperTest.java timed out

Reviewed-by: hseigel, lfoltan, dholmes

-------------

PR: https://git.openjdk.java.net/jdk/pull/3375