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

[Igor Ignatev]

Reviewed. 

— Igor

> On Mar 18, 2020, at 3:18 PM, Leonid Mesnik <Leonid.Mesnik at oracle.com> wrote:
> 
> 
> 
> 
> 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> 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> 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
>>>> 
>>> 
>>