On 27/03/2020 9:16 am, Leonid Mesnik wrote:
On 3/26/20 4:06 PM, David Holmes wrote:
Hi Leonid,
On 27/03/2020 7:39 am, Leonid Mesnik wrote:
Replying with correct summary.
Leonid
On 3/23/20 8:55 PM, Leonid Mesnik wrote:
Hi
Could you please review following fix which update ThreadsRunner to
use AtomicInteger/spinOnWait instead of Wicket to synchronize
starting of stress test threads.
Failing tests allocated all memory by earlier started threads before
Lock.unlock is called in the latest threads. So thread might get an
OOME exception while trying to release lock and/or get into
inconsistent state.
You have a bug in Wicket:
+ try {
+ lock.lock();
...
+ } finally {
+ lock.unlock();
The lock() has to go outside the try block. That is why you were
getting IllegalMonitorStateExceptions when the lock() threw OOME.
Thanks for explanation. But anyway, as I understand locks use memory and
might be inconsistent if OOME happened.
They use memory and so lock() can throw OOME, but they are never
inconsistent.
But the OOME itself is still a problem as it means you can't use any
proper synchronizer. I don't like seeing the spin-loops but in this
code you may have no choice if memory may already be exhausted.
It should be really short spin-loop, test only start thread during this
loop and don't do anything more. Also, it is done only once for all
stress test. The goal is to start thread completely before heap is
exhausted.
Okay. I'm somewhat dubious about making these changes in mainline now
just to support loom. I don't see why we need to care about pinning
threads in this kind of situation.
David
Leonid
David
-----
The bug was introduced by
https://bugs.openjdk.java.net/browse/JDK-8241123
<https://bugs.openjdk.java.net/browse/JDK-8241123>
The Atomic works fine for stress test finishing sync. I just didn't
expect that tests might OOME while releasing start lock.
Verified that tests now don't fail with -Xcomp -server
-XX:-TieredCompilation -XX:-UseCompressedOops.
webrev: http://cr.openjdk.java.net/~lmesnik/8241456/webrev.00/
<http://cr.openjdk.java.net/~lmesnik/8241456/webrev.00/>
bug: https://bugs.openjdk.java.net/browse/JDK-8241456
<https://bugs.openjdk.java.net/browse/JDK-8241456>
Leonid
