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
