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

[Leonid Mesnik]

Thank you for review and  feedback.

Leonid

On 3/18/20 3:22 PM, Igor Ignatev wrote:
> 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 <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
>>>>>
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20200318/28083951/attachment-0001.htm>