> On Mar 18, 2020, at 1:29 PM, Leonid Mesnik <[email protected]> wrote:
> 
> 
> On 3/18/20 12:48 PM, Igor Ignatyev wrote:
>> Hi Leonid,
>> 
>> I've started looking at your webrev, and so far have a couple questions:
>> 
>>> Test 
>>> vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects003/referringObjects003a.java
>>>  was updated to don't use Wicket. (The lock has a reference to thread which 
>>> affects test.)
>> can't you use just a volatile boolean field?
> I can, but I don't see any benefits to use volatile fields instead of 
> atomics. I prefer to use Atomic* anywhere because of it's clearer semantics. 
> Using of explicit get/set and other similar accessors.
you aren't using any accessors other than plain get/set, which are semantically 
equal to setting/getting a volatile field, so I'm not sure how it's clearer.as 
of benefits of a volatile field, the code is shorter (and arguable cleaner) and 
you save some heap space. anyhow, I don't insist on usage of volatile boolean 
over AtomicBoolean, 
>> 
>>> Wicket "finished" in class ThreadsRunner was changed to atomicInt/sleep to 
>>> avoid OOME in j.u.c.l.Condition::await() which might happened in stress GC 
>>> tests.
>> won't j.u.c.CountDownLatch be more appropriate and cleaner solution here?
> 
> Unfortunately no. The CountDownLatch would be a nice solution but it is 
> possible to get OOME in gc/lock (might be other) tests. I replaced Wicked by 
> the same reason. Updating the AtomicInteger doesn't allocate any memory and 
> don't cause OOME.
I see.
> 
> Leonid
> 
>> 
>> I need more time to get grasp of Wicket and your changes in it; will come 
>> back to you after I understand them.
>> 
>> -- Igor
>> 
>>> On Mar 18, 2020, at 12:37 PM, Leonid Mesnik <[email protected]> 
>>> wrote:
>>> 
>>> Hi
>>> 
>>> Could you please review following fix which slightly refactor vmTestbase 
>>> stress test harness. This refactoring helps to add virtual threads testing 
>>> support.
>>> 
>>> The Wicket uses plain sync/wait/notify mechanism which cause carrier thread 
>>> starvation and should not be used in virtual threads. The ManagedThread is 
>>> a subclass of Thread so it couldn't be virtual thread.
>>> 
>>> 
>>> Following fix changes Wicket to use locks/conditions to don't pin vthread 
>>> to carrier thread while starting testing.
>>> 
>>> ManagedThread is fixed to keep execution thread as the thread variable and 
>>> isolate it's creation.
>>> 
>>> Test 
>>> vmTestbase/nsk/jdi/ObjectReference/referringObjects/referringObjects003/referringObjects003a.java
>>>  was updated to don't use Wicket. (The lock has a reference to thread which 
>>> affects test.)
>>> 
>>> Wicket "finished" in class ThreadsRunner was changed to atomicInt/sleep to 
>>> avoid OOME in j.u.c.l.Condition::await() which might happened in stress GC 
>>> tests.
>>> 
>>> webrev: http://cr.openjdk.java.net/~lmesnik/8241123/webrev.00/
>>> 
>>> bug: https://bugs.openjdk.java.net/browse/JDK-8241123
>>> 
>>> 
>>> Leonid
>>> 

Reply via email to