Hi Alex,
Thanks for this suggestion. You are right, we actually don't need this
waitForAllThreads() method.
I will include this change in the new version of the webrev.
> 207 int diff = numNewThreads - numTerminatedThreads;
> 208 long threadCount = mbean.getThreadCount();
> 209 long expectedThreadCount = prevLiveTestThreadCount + diff;
> 210 if (threadCount < expectedThreadCount) {
> if some internal thread terminates, we'll get failure here
The failure will not happen. Please note that prevLiveTestThreadCount counts
only *test* threads. Thus even if some Internal threads terminated the value
mbean.getThreadCount() returns should still be no less than the expected
number of live test threads.
310 prevLiveTestThreadCount = getTestThreadCount();
Best regards,
Daniil
On 6/3/20, 3:08 PM, "Alex Menkov" <alexey.men...@oracle.com> wrote:
Hi Daniil,
couple notes:
198 waitForThreads(numNewThreads, numTerminatedThreads);
You don't actually need any wait here.
Test cases wait until all threads are in desired state
(checkAllThreadsAlive uses startupCheck, checkDaemonThreadsDead and
checkAllThreadsDead use join())
205 private static void checkLiveThreads(int numNewThreads,
206 int numTerminatedThreads) {
207 int diff = numNewThreads - numTerminatedThreads;
208 long threadCount = mbean.getThreadCount();
209 long expectedThreadCount = prevLiveTestThreadCount + diff;
210 if (threadCount < expectedThreadCount) {
if some internal thread terminates, we'll get failure here
--alex
On 06/02/2020 21:00, Daniil Titov wrote:
> Hi Alex, Serguei, and Martin,
>
> Thank you for your comments. Please review a new version of the fix that
addresses them, specifically:
> 1) Replaces a double loop in checkAllThreadsAlive() with a code that
uses collections and containsAll() method.
> 2) Restores the checks for other ThreadMXBean methods (getThreadCount(),
getTotalStartedThreadCount(), getPeakThreadCount()) but with more relaxed
conditions.
> 3) Relaxes the check inside checkThreadIds() method
>
>
> [1] http://cr.openjdk.java.net/~dtitov/8131745/webrev.03/
> [2] https://bugs.openjdk.java.net/browse/JDK-8131745
>
> Thank you,
> Daniil
>
> On 6/1/20, 5:06 PM, "Alex Menkov" <alexey.men...@oracle.com> wrote:
>
> Hi Daniil,
>
> 1. before the fix checkLiveThreads() tested
> ThreadMXBean.getThreadCount(), but now as far as I see it tests
> Thread.getAllStackTraces();
>
> 2.
> 237 private static void checkThreadIds() throws
InterruptedException {
> 238 long[] list = mbean.getAllThreadIds();
> 239
> 240 waitTillEquals(
> 241 list.length,
> 242 ()->(long)mbean.getThreadCount(),
> 243 "Array length returned by " +
> 244 "getAllThreadIds() = %1$d not matched count =
> ${provided}",
> 245 ()->list.length
> 246 );
> 247 }
>
> I suppose purpose of waitTillEquals() is to handle
creation/termination
> of VM internal threads.
> But if some internal thread terminates after mbean.getAllThreadIds()
and
> before 1st mbean.getThreadCount() call and then VM does not need to
> restart it, waitTillEquals will wait forever.
>
> --alex
>
>
> On 05/29/2020 16:28, Daniil Titov wrote:
> > Hi Alex and Serguei,
> >
> > Please review a new version of the change [1] that makes sure that
the test counts
> > only the threads it creates and ignores Internal threads VM might
create or destroy.
> >
> > Testing: Running this test in Mach5 with Graal on several hundred
times ,
> > tier1-tier3 tests are in progress.
> >
> > [1] http://cr.openjdk.java.net/~dtitov/8131745/webrev.02/
> > [2] https://bugs.openjdk.java.net/browse/JDK-8131745
> >
> > Thank you,
> > Daniil
> >
> > On 5/22/20, 10:26 AM, "Alex Menkov" <alexey.men...@oracle.com>
wrote:
> >
> > Hi Daniil,
> >
> > I'm not sure all this retry logic is a good way.
> > As mentioned in jira the most important part of the testing
is ensuring
> > that you find all the created threads when they are alive,
and you don't
> > find them when they are dead. The actual thread count
checking is not
> > that important.
> > I agree with this and I'd just simplify the test by removing
checks for
> > thread count. VM may create and destroy internal threads when
it needs it.
> >
> > --alex
> >
> > On 05/18/2020 10:31, Daniil Titov wrote:
> > > Please review the change [1] that fixes an intermittent
failure of the test.
> > >
> > > This test creates and destroys a given number of
daemon/user threads and validates the count of those started/stopped threads
against values returned from ThreadMXBean thread counts. The problem here is
that if some internal threads is started ( e.g. " HotSpotGraalManagement Bean
Registration"), or destroyed (e.g. "JVMCI CompilerThread ") the test hangs
waiting for expected number of live threads.
> > >
> > > The fix limits the time the test is waiting for desired
number of live threads and in case if this limit is exceeded the test repeats
itself.
> > >
> > > Testing. Test with Graal on and Mach5 tier1-tier7 test
passed.
> > >
> > > [1] http://cr.openjdk.java.net/~dtitov/8131745/webrev.01
> > > [2] https://bugs.openjdk.java.net/browse/JDK-8131745
> > >
> > > Thank you,
> > > Daniil
> > >
> > >
> > >
> >
> >
>
>
