> 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.l...@oracle.com" <dean.l...@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.l...@oracle.com>
> Organization: Oracle Corporation
> Date: Thursday, March 7, 2019 at 12:19 PM
> To: Daniil Titov <daniil.x.ti...@oracle.com>, OpenJDK Serviceability
<serviceability-dev@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
>
>
>
>
>
