RFR: 8241123: Refactor vmTestbase stress framework to use j.u.c and make creation of threads more flexible

[Leonid Mesnik]

On 3/18/20 2:30 PM, Igor Ignatyev wrote:
>> I need more time to get grasp of Wicket and your changes in it; will 
>> come back to you after I understand them. 
> ok, now when I believe that I have enough understanding of Wicket, I 
> have a few comments:
> 1.
>> 68 private Lock lock = new ReentrantLock();
>> 69 private Condition condition = lock.newCondition();
> it's better to make these fields final.
>
> 2. as all writes and reads of Wicket::count are guarded by lock.lock, 
> there is no need for it to be atomic.
> 3. adding lock to getWaiters will also remove need for Wicket::waiters 
> to be atomic.

All 3 are fixed. Thanks for your suggestions.

Updated version:

http://cr.openjdk.java.net/~lmesnik/8241123/webrev.01/

Leonid

>
> the rest looks good to me.
>
> Thanks,
> -- Igor
>
>
>
>> On Mar 18, 2020, at 12:48 PM, Igor Ignatyev <igor.ignatyev at oracle.com 
>> <mailto:igor.ignatyev at oracle.com>> 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?
>>
>>> 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?
>>
>> 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 
>>> <leonid.mesnik at oracle.com <mailto:leonid.mesnik at oracle.com>> 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
>>>
>>
>