> 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. the rest looks good to me. Thanks, -- Igor > On Mar 18, 2020, at 12:48 PM, Igor Ignatyev <[email protected]> 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 <[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 >> >
