8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed

[Daniel D. Daugherty]

 > Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.02

test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.cpp
     L604:     if ((strlen(threadinfo.name) > prefixLength) &&
         nit - you have an extra '(' ')' pair that's not needed.

Thumbs up! No need for another webrev if you choose to fix the nit.

Dan


On 3/12/19 7:14 PM, Daniil Titov wrote:
> Hi Daniel,
>
> Since you have reviewed the first version of the webrev, just wanted to check with you that you are OK with the new version of the fix. Please note that a new enhancement [3] was created  to cover the work related with hiding JVMCI compiler (and probably some other) threads.
>
> References
> ----------------
> [1] Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.02
> [2] Bug:  https://bugs.openjdk.java.net/browse/JDK-8218812
> [3] Enhancement: https://bugs.openjdk.java.net/browse/JDK-8220468
>
> Thanks,
> Daniil
>
> On 3/11/19, 9:22 PM, "dean.long at oracle.com" <dean.long at oracle.com> wrote:
>
>      This is consistent with how other system threads are handled, so it
>      looks good to me.
>      
>      dl
>      
>      On 3/11/19 4:32 PM, Daniil Titov wrote:
>      > Hi Dean, JC and Daniel,
>      >
>      > Thank you for reviewing this change. Based on the overall output it seems as the common consensus is that the broader discussions is required to decide what would be a proper way to optionally hide/mute JVMCI compiler threads (and probably some other “system” Java threads) from JVMTI clients.  Thus, my suggestion is to move this discussion in a new enhancement [3] and limit the current issue just to the fixing the test itself.
>      >
>      > Please review a new version of the webrev that adds  JVMCI compiler and "HotSpotGraalManagement Bean Registration" threads to the list of the threads the tests must ignore.
>      >
>      >
>      > Reference:
>      > --------------
>      > [1] Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.02
>      > [2] Bug: https://bugs.openjdk.java.net/browse/JDK-8218812
>      > [3] Enhancement: https://bugs.openjdk.java.net/browse/JDK-8220468
>      >
>      > Thanks,
>      > Daniil
>      >
>      > From: <dean.long at oracle.com>
>      > Organization: Oracle Corporation
>      > Date: Thursday, March 7, 2019 at 12:19 PM
>      > To: Daniil Titov <daniil.x.titov at oracle.com>, OpenJDK Serviceability <serviceability-dev at openjdk.java.net>
>      > Subject: Re: RFR: 8218812: vmTestbase/nsk/jvmti/GetAllThreads/allthr001/TestDescription.java failed
>      >
>      > There are other places where is_hidden_from_external_view() is used.  Should is_hidden_from_external_view() also check the new capability?  If so, then you can simplify your changes.  I'm not sure the new capability is the best choice, however.  There is still no way to control whether compiler threads can post events, hit breakpoints, single-step, etc.  And "compiler thread" might be too specific.  There might be other types of "system thread" that we want to ignore.  Since this is a JVMTI spec change, I think it deserves more discussion.  For example, an alternative would be a way to set "can debug"/"visible"/"can post events"/etc flags on individual threads.
>      >
>      > dl
>      > On 3/7/19 9:54 AM, Daniil Titov wrote:
>      > Please review a change that fixes this test.
>      >
>      > The problem here is that the test checks the number of threads and with Graal on additional threads the test doesn't expect are started and cause the test fail.
>      >
>      > The fix introduces a new capability " can_show_compiler_threads" that affects whether Java compiler threads are retuned with JVMTI GetAllThreads call. By default this capability is off. The fix also adds " HotSpotGraalManagement Bean Registration" thread to the list of the threads the tests must ignore.
>      >
>      > Webrev: http://cr.openjdk.java.net/~dtitov/8218812/webrev.01
>      > Bug: https://bugs.openjdk.java.net/browse/JDK-8218812
>      >
>      > Mach5 tier1, tier2 and tier3 tests successfully passed with this change.
>      >
>      > Thanks!
>      > -Daniil
>      >
>      >
>      >
>      >
>      >
>      
>      
>
>