|
Hi Daniil,
LGTM++
Thanks,
Serguei
On 3/11/19 17:43, Jean Christophe Beyler wrote:
Hi Daniil,
Looks good to me :)
Thanks,
Jc
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
--
|
