Hi Daniil, Looks good to me :)
Thanks, Jc On Mon, Mar 11, 2019 at 4:32 PM Daniil Titov <daniil.x.ti...@oracle.com> 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 > > > > > > -- Thanks, Jc
