Hi Leonid, The call isJFRActive() need to be executed on HeapwalkingDebuggee side. This is what my understanding is.
Thanks, Fairoz > -----Original Message----- > From: Leonid Mesnik > Sent: Wednesday, June 10, 2020 1:16 AM > To: Serguei Spitsyn <serguei.spit...@oracle.com>; Fairoz Matte > <fairoz.ma...@oracle.com>; Erik Gahlin <erik.gah...@oracle.com> > Cc: serviceability-dev@openjdk.java.net > Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > incorrect > and corresponsing logic seems to be broken > > http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/test/hotspot/jtreg/vmT > estbase/nsk/share/jdi/TestDebuggerType2.java.udiff.html > > I see that isJFRActive() depends on "nsk.share.jdi.HeapwalkingDebuggee". > It is not going to work of debugee is not "nsk.share.jdi.HeapwalkingDebuggee". > > Shouldn't it be placed in HeapWalkingDebugger? > > Leonid > > On 6/8/20 9:26 PM, serguei.spit...@oracle.com wrote: > > Hi Fairoz, > > > > LGTM. > > > > Thanks, > > Serguei > > > > > > On 6/8/20 21:20, Fairoz Matte wrote: > >> Hi Serguei, > >> > >> Thanks for the clarifications, > >> I have incorporated the 2nd suggestion, below is the webrev, > >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.09/ > >> > >> Thanks, > >> Fairoz > >> > >> From: Serguei Spitsyn > >> Sent: Monday, June 8, 2020 10:34 PM > >> To: Fairoz Matte <fairoz.ma...@oracle.com>; Erik Gahlin > >> <erik.gah...@oracle.com> > >> Cc: serviceability-dev@openjdk.java.net > >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > >> incorrect and corresponsing logic seems to be broken > >> > >> Hi Fairoz, > >> > >> > >> On 6/8/20 02:08, mailto:serguei.spit...@oracle.com wrote: > >> Hi Fairoz, > >> > >> There are two different isJFRActive() methods, one is on debuggee > >> side and another on the debugger side. > >> The one on debuggee side is better to keep in Debuggee.java (where it > >> was before) instead of moving it to HeapwalkingDebuggee.java. > >> It is okay to keep the call to it in the HeapwalkingDebuggee.java. > >> > >> Please, skip this suggestion as Debugger.java is not one of supers of > >> HeapwalkingDebuggee.java as I've assumed. > >> > >> Thanks, > >> Serguei > >> > >> > >> + protected boolean isJFRActive() { > >> + boolean isJFRActive = false; > >> + ReferenceType referenceType = > >> debuggee.classByName("nsk.share.jdi.HeapwalkingDebuggee"); > >> + if (referenceType == null) > >> + throw new RuntimeException("Debugeee is not initialized > >> yet"); > >> + > >> + Field isJFRActiveFld = > >> referenceType.fieldByName("isJFRActive"); > >> + isJFRActive = > >> ((BooleanValue)referenceType.getValue(isJFRActiveFld)).value(); > >> + return isJFRActive; > >> } > >> It is better to remove the line: > >> + boolean isJFRActive = false; > >> and just change this one: > >> + boolean isJFRActive = > >> ((BooleanValue)referenceType.getValue(isJFRActiveFld)).value(); > >> > >> Otherwise, it looks good to me. > >> I hope, it really works now. > >> > >> Thanks, > >> Serguei > >> > >> On 6/8/20 00:26, Fairoz Matte wrote: > >> Hi Serguei, Erik, > >> Thanks for the reviews, > >> Below webrev contains the suggested changes, > >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.08/ > >> The only thing I couldn’t do is to keep the local copy of > >> isJFRActive() in HeapwalkingDebugger, The method is called in debugee > >> code. > >> In debugger, we have access to debugee before test started or after > >> test completes. > >> isJFRActive() method need to be executed during the test execution. > >> Hence I didn’t find place to initialize and cannot make local copy. > >> Thanks, > >> Fairoz > >> From: Serguei Spitsyn > >> Sent: Tuesday, June 2, 2020 7:57 AM > >> To: Fairoz Matte mailto:fairoz.ma...@oracle.com; Erik Gahlin > >> mailto:erik.gah...@oracle.com > >> Cc: mailto:serviceability-dev@openjdk.java.net > >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > >> incorrect and corresponsing logic seems to be broken > >> On 6/1/20 12:30, mailto:serguei.spit...@oracle.com wrote: > >> Hi Fairoz, > >> > >> It looks okay in general. > >> But I'm not sure this check is going to work. > >> The problem is the HeapwalkingDebuggee.useStrictCheck method is > >> invoked in the context of the HeapwalkingDebugger process, not the > >> HeapwalkingDebuggee process. > >> > >> Probably, you wanted to get this bit of information from the Debuggee > >> process. > >> The debuggee has to evaluate it itself and store in some field. > >> The debugger should use the JDI to get this value from the debuggee. > >> > >> Thanks, > >> Serguei > >> > >> I'm not sure, what exactly you wanted to do here. > >> It can occasionally work for you as long as both processes are run > >> with the same options. > >> > >> Thanks, > >> Serguei > >> > >> > >> On 6/1/20 08:52, Fairoz Matte wrote: > >> Hi Erik, > >> Thanks for the review, below is the updated webrev. > >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.02/ > >> Thanks, > >> Fairoz > >> -----Original Message----- > >> From: Erik Gahlin > >> Sent: Monday, June 1, 2020 4:26 PM > >> To: Fairoz Matte mailto:fairoz.ma...@oracle.com > >> Cc: mailto:serviceability-dev@openjdk.java.net > >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > >> incorrect and corresponsing logic seems to be broken > >> Hi Fairoz, > >> What I think you need to do is something like this: > >> if (className.equals("java.lang.Thread")) { > >> return !isJfrInitialized(); > >> } > >> ... > >> private static boolean isJfrInitialized() { > >> try { > >> Class<?> clazz = > >> Class.forName("jdk.jfr.FlightRecorder"); > >> Method method = > >> clazz.getDeclaredMethod("isInitialized", > >> new Class[0]); > >> return (boolean) method.invoke(null, new Object[0]); > >> } catch (Exception e) { > >> return false; > >> } > >> } > >> Erik > >> On 2020-06-01 12:30, Fairoz Matte wrote: > >> Hi Erik, > >> Thanks for your quick response, > >> Below is the updated webrev to handle if jfr module is not present > >> http://cr.openjdk.java.net/~fmatte/8243451/webrev.01/ > >> Thanks, > >> Fairoz > >> -----Original Message----- > >> From: Erik Gahlin > >> Sent: Monday, June 1, 2020 2:31 PM > >> To: Fairoz Matte mailto:fairoz.ma...@oracle.com > >> Cc: mailto:serviceability-dev@openjdk.java.net > >> Subject: Re: RFR(s): 8243451: nsk.share.jdi.Debugee.isJFR_active() is > >> incorrect and corresponsing logic seems to be broken > >> Hi Fairoz, > >> If the test needs to run with builds where the JFR module is not > >> present(?), you need to do the check using reflection. > >> If not, looks good. > >> Erik > >> On 1 Jun 2020, at 10:27, Fairoz Matte > >> mailto:fairoz.ma...@oracle.com wrote: > >> Hi, > >> Please review this small test infra change to identify at runtime > >> the JFR is active or not. > >> JBS - https://bugs.openjdk.java.net/browse/JDK-8243451 > >> Webrev - http://cr.openjdk.java.net/~fmatte/8243451/webrev.00/ > >> Thanks, > >> Fairoz > >> > >
