Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
+1
Chris
On 6/12/18 3:02 PM, serguei.spit...@oracle.com wrote:
Hi Daniil,
Looks good.
Thank you for the update!
Minor: two extra empty lines at the end:
}
+
+// Filters out events with location not matching the given type.
+public static boolean filtered(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
+
+
}
No need in another review if you delete them.
Thanks,
Serguei
On 6/12/18 14:57, Daniil Titov wrote:
Thank you, Serguei and Chris,
Please review a new version of the fix.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.03/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 1:08 PM
To: Daniil Titov ,
Chris Plummer ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
On 6/12/18 12:30, Daniil Titov wrote:
Hi Serguei and Chris,
As I mentioned in the previous emails
there are only 4 other tests that calls
EventFiliters.filtered(event) and only 2 of them need to
be changed to use EventFiliters.filtered(event,
debuggeName).
So if the approach in the first
webrev http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
is preferable
I guess, it is just a typo above.
It is not the first approach but the second.
we could stick with it and I will
just update this patch to add 2 more tests and combine
EventFiliters.filtered(event, typeName) and
EventFiliters.filteredByLocation(event, typeName) in the
single method as Serguei suggested.
Yes, my preference is this approach.
It adds some ground for future improvements as well.
It is also possible to use this method for outer classes
being debugged like this:
filtered(event, "java.lang.Integer")
Thanks,
Serguei
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 12:21 PM
To: Chris Plummer ,
Daniil Titov ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests
fail with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
On 6/12/18 12:02, Chris Plummer wrote:
On 6/12/18 11:53 AM, serguei.spit...@oracle.com
wrote:
Hi Daniil,
I don't think, the approach for the tests to
filter out the JFR and Graal threads
is not good despite the fact it is relatively easy
to implement.
It does not cover any threads that may come in the
future.
So that I consider the webrev below as just a work
around of the real pr
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
Looks good.
Thank you for the update!
Minor: two extra empty lines at the end:
}
+
+// Filters out events with location not matching the given type.
+public static boolean filtered(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
+
+
}
No need in another review if you delete them.
Thanks,
Serguei
On 6/12/18 14:57, Daniil Titov wrote:
Thank you, Serguei and Chris,
Please review a new version of the fix.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.03/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 1:08 PM
To: Daniil Titov
, Chris Plummer
,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
On 6/12/18 12:30, Daniil Titov wrote:
Hi Serguei and Chris,
As I mentioned in the previous emails
there are only 4 other tests that calls
EventFiliters.filtered(event) and only 2 of them need to
be changed to use EventFiliters.filtered(event,
debuggeName).
So if the approach in the first webrev
http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
is preferable
I guess, it is just a typo above.
It is not the first approach but the second.
we could stick with it and I will just
update this patch to add 2 more tests and combine
EventFiliters.filtered(event, typeName) and
EventFiliters.filteredByLocation(event, typeName) in the
single method as Serguei suggested.
Yes, my preference is this approach.
It adds some ground for future improvements as well.
It is also possible to use this method for outer classes being
debugged like this:
filtered(event, "java.lang.Integer")
Thanks,
Serguei
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 12:21 PM
To: Chris Plummer ,
Daniil Titov ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
On 6/12/18 12:02, Chris Plummer wrote:
On 6/12/18 11:53 AM, serguei.spit...@oracle.com
wrote:
Hi Daniil,
I don't think, the approach for the tests to filter
out the JFR and Graal threads
is not good despite the fact it is relatively easy
to implement.
It does not cover any threads that may come in the
future.
So that I consider the webrev below as just a work
around of the real problem.
True, but his changes are just an
extension of what is already being done - filtering out
threads we don't care about.
I understand it.
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Thank you, Serguei and Chris, Please review a new version of the fix. Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.03/ Bug: https://bugs.openjdk.java.net/browse/JDK-8203809 Thanks! Best regards, Daniil From: "serguei.spit...@oracle.com" Date: Tuesday, June 12, 2018 at 1:08 PM To: Daniil Titov , Chris Plummer , "serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net" Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint ** Hi Daniil, On 6/12/18 12:30, Daniil Titov wrote: Hi Serguei and Chris, As I mentioned in the previous emails there are only 4 other tests that calls EventFiliters.filtered(event) and only 2 of them need to be changed to use EventFiliters.filtered(event, debuggeName). So if the approach in the first webrev http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/ is preferable I guess, it is just a typo above. It is not the first approach but the second. we could stick with it and I will just update this patch to add 2 more tests and combine EventFiliters.filtered(event, typeName) and EventFiliters.filteredByLocation(event, typeName) in the single method as Serguei suggested. Yes, my preference is this approach. It adds some ground for future improvements as well. It is also possible to use this method for outer classes being debugged like this: filtered(event, "java.lang.Integer") Thanks, Serguei Thanks! Best regards, Daniil From: "serguei.spit...@oracle.com" Date: Tuesday, June 12, 2018 at 12:21 PM To: Chris Plummer , Daniil Titov , "serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net" Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint ** On 6/12/18 12:02, Chris Plummer wrote: On 6/12/18 11:53 AM, serguei.spit...@oracle.com wrote: Hi Daniil, I don't think, the approach for the tests to filter out the JFR and Graal threads is not good despite the fact it is relatively easy to implement. It does not cover any threads that may come in the future. So that I consider the webrev below as just a work around of the real problem. True, but his changes are just an extension of what is already being done - filtering out threads we don't care about. I understand it. The tests should just filter out everything that is not a part of their testing. It can be done on a base of the Debuggee classes or threads, or any other approaches. One problem is that events don't always come in on debuggee thread, so you can't filter out all threads other than the debuggee thread. I think probably filtering out all classes not related to the debuggee is probably the way to go. However, do we want to fix all the tests to do this, or just go with Daniil's quick fix? In general, I'm Okay with this quick workaround fix. But wanted to underline that we still have a base for problems in the future. Thanks, Serguei Chris Thanks, Serguei On 6/12/18 10:57, Daniil Titov wrote: Hi Serguei, There are 4 more tests that use EventFilters.filter() method and not all of them need to be changed to call EventFilters .filtered(Event event, String typeName). Specifically, vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java and vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java tests expect to receive an ExceptionEvent for NumberFormatException exception and the location of this event happens to be not the debuggee class ( java.lang.Integer:652). The tests pass now and changing them to use EventFilters .filtered(event, debugeeName) makes them fail. There is also an alternative approach I would like to suggest. All these new Graal events are generated by the compiler threads (“JVMCI CompilerThread0, JVMCI CompilerThread1, etc.”) and could be filtered out inside EventFilters .filtered(Event event) method by checking the thread reference for the given event object. Please review the updated fix the uses this approach. Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/ Bug: https://bugs.openjdk.java.net/browse/JDK-8203809 Thanks! Best regards, Daniil From: "serguei.spit...@oracle.com" Date: Monday, June 11, 2018 at 6:18 PM To: Daniil Titov , "serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net" Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint ** Hi Daniil, It looks good in general. Some minor comments though. http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html + +// Filters out events with location not matching the given type. +public static boolean filteredByLocation(Event event, String
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Serguei, On 6/12/18 1:08 PM, serguei.spit...@oracle.com wrote: Hi Daniil, On 6/12/18 12:30, Daniil Titov wrote: Hi Serguei and Chris, As I mentioned in the previous emails there are only 4 other tests that calls EventFiliters.filtered(event) and only 2 of them need to be changed to use EventFiliters.filtered(event, debuggeName). So if the approach in the first webrev http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/ <http://cr.openjdk.java.net/%7Edtitov/8203809/webrev.02/> is preferable I guess, it is just a typo above. It is not the first approach but the second. It is a typo. The proper link is http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/ we could stick with it and I will just update this patch to add 2 more tests and combine EventFiliters.filtered(event, typeName) and EventFiliters.filteredByLocation(event, typeName) in the single method as Serguei suggested. Yes, my preference is this approach. It adds some ground for future improvements as well. It is also possible to use this method for outer classes being debugged like this: filtered(event, "java.lang.Integer") Thanks, Serguei Agree. Will prepare an updated webrev soon. Thanks! Thanks! Best regards, Daniil *From: *"serguei.spit...@oracle.com" *Date: *Tuesday, June 12, 2018 at 12:21 PM *To: *Chris Plummer , Daniil Titov , "serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net" *Subject: *Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint ** On 6/12/18 12:02, Chris Plummer wrote: On 6/12/18 11:53 AM, serguei.spit...@oracle.com <mailto:serguei.spit...@oracle.com> wrote: Hi Daniil, I don't think, the approach for the tests to filter out the JFR and Graal threads is not good despite the fact it is relatively easy to implement. It does not cover any threads that may come in the future. So that I consider the webrev below as just a work around of the real problem. True, but his changes are just an extension of what is already being done - filtering out threads we don't care about. I understand it. The tests should just filter out everything that is not a part of their testing. It can be done on a base of the Debuggee classes or threads, or any other approaches. One problem is that events don't always come in on debuggee thread, so you can't filter out all threads other than the debuggee thread. I think probably filtering out all classes not related to the debuggee is probably the way to go. However, do we want to fix all the tests to do this, or just go with Daniil's quick fix? In general, I'm Okay with this quick workaround fix. But wanted to underline that we still have a base for problems in the future. Thanks, Serguei Chris Thanks, Serguei On 6/12/18 10:57, Daniil Titov wrote: Hi Serguei, There are 4 more tests that use EventFilters.filter() method and not all of them need to be changed to call EventFilters.filtered(Event event, String typeName). Specifically, vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java and vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java tests expect to receive an ExceptionEvent for NumberFormatException exception and the location of this event happens to be not the debuggee class ( java.lang.Integer:652). The tests pass now and changing them to use EventFilters.filtered(event, debugeeName) makes them fail. There is also an alternative approach I would like to suggest. All these new Graal events are generated by the compiler threads (“JVMCI CompilerThread0, JVMCI CompilerThread1, etc.”) and could be filtered out inside EventFilters.filtered(Event event) method by checking the thread reference for the given event object. Please review the updated fix the uses this approach. Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/ <http://cr.openjdk.java.net/%7Edtitov/8203809/webrev.02/> Bug: https://bugs.openjdk.java.net/browse/JDK-8203809 Thanks! Best regards, Daniil *From: *"serguei.spit...@oracle.com" <mailto:serguei.spit...@oracle.com> <mailto:serguei.spit...@oracle.com> *Date: *Monday, June 11, 2018 at 6:18 PM *To: *Daniil Titov <mailto:daniil.x.ti...@oracle.com>, "serviceab
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
On 6/12/18 12:30, Daniil Titov wrote:
Hi Serguei and Chris,
As I mentioned in the previous emails there
are only 4 other tests that calls
EventFiliters.filtered(event) and only 2 of them need to be
changed to use EventFiliters.filtered(event, debuggeName).
So if the approach in the first webrev http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
is preferable
I guess, it is just a typo above.
It is not the first approach but the second.
we could stick with it and I will just
update this patch to add 2 more tests and combine
EventFiliters.filtered(event, typeName) and
EventFiliters.filteredByLocation(event, typeName) in the
single method as Serguei suggested.
Yes, my preference is this approach.
It adds some ground for future improvements as well.
It is also possible to use this method for outer classes being
debugged like this:
filtered(event, "java.lang.Integer")
Thanks,
Serguei
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 12:21 PM
To: Chris Plummer
, Daniil Titov
,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
On 6/12/18 12:02, Chris Plummer wrote:
On 6/12/18 11:53 AM, serguei.spit...@oracle.com
wrote:
Hi Daniil,
I don't think, the approach for the tests to filter out
the JFR and Graal threads
is not good despite the fact it is relatively easy to
implement.
It does not cover any threads that may come in the
future.
So that I consider the webrev below as just a work
around of the real problem.
True, but his changes are just an
extension of what is already being done - filtering out
threads we don't care about.
I understand it.
The tests should just filter out
everything that is not a part of their testing.
It can be done on a base of the Debuggee classes or
threads, or any other approaches.
One problem is that events don't always
come in on debuggee thread, so you can't filter out all
threads other than the debuggee thread. I think probably
filtering out all classes not related to the debuggee is
probably the way to go. However, do we want to fix all the
tests to do this, or just go with Daniil's quick fix?
In general, I'm Okay with this quick workaround fix.
But wanted to underline that we still have a base for problems
in the future.
Thanks,
Serguei
Chris
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need
to be changed to call EventFilters
.filtered(Event event, String typeName).
Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of
this event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and
changing them to use EventFilters .filtered(event, debugeeName) makes
them fail.
There is also an
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to be
changed to call EventFilters .filtered(Event
event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing them
to use EventFilters
.filtered(event, debugeeName) makes them fail.
I still like your approach with the filtered(event,
className).
Would it be a problem to use the same method to filter out all
classes that are not the java.lang.Integer with:
filtered(event, "java.lang.Integer")
Also, this can be used to accept (not to filter out) multiple classes with something like this:
if (filtered(event, "java.lang.Integer") && filtered(event, debuggeeName)) {
}
Thanks,
Serguei
There is also an
alternative approach I would like to suggest. All these new
Graal events are generated by the compiler threads (“JVMCI
CompilerThread0, JVMCI CompilerThread1, etc.”) and could be
filtered out inside EventFilters
.filtered(Event event) method by checking the thread
reference for the given event object.
Please review the updated fix the uses this
approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov
,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second
method and rename filteredByLocation()
to filtered():
+ // Filters out events with location not matching the given type.
+ public static boolean filtered(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
As I understand, it should filter out the events post on the
JFR threads.
Please, correc
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Serguei and Chris,
As I mentioned in the previous emails there are only 4 other tests that calls
EventFiliters.filtered(event) and only 2 of them need to be changed to use
EventFiliters.filtered(event, debuggeName).
So if the approach in the first webrev
http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/ is preferable we could
stick with it and I will just update this patch to add 2 more tests and combine
EventFiliters.filtered(event, typeName) and
EventFiliters.filteredByLocation(event, typeName) in the single method as
Serguei suggested.
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Tuesday, June 12, 2018 at 12:21 PM
To: Chris Plummer , Daniil Titov
, "serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException
: ** event IS NOT a breakpoint **
On 6/12/18 12:02, Chris Plummer wrote:
On 6/12/18 11:53 AM, serguei.spit...@oracle.com wrote:
Hi Daniil,
I don't think, the approach for the tests to filter out the JFR and Graal
threads
is not good despite the fact it is relatively easy to implement.
It does not cover any threads that may come in the future.
So that I consider the webrev below as just a work around of the real problem.
True, but his changes are just an extension of what is already being done -
filtering out threads we don't care about.
I understand it.
The tests should just filter out everything that is not a part of their testing.
It can be done on a base of the Debuggee classes or threads, or any other
approaches.
One problem is that events don't always come in on debuggee thread, so you
can't filter out all threads other than the debuggee thread. I think probably
filtering out all classes not related to the debuggee is probably the way to
go. However, do we want to fix all the tests to do this, or just go with
Daniil's quick fix?
In general, I'm Okay with this quick workaround fix.
But wanted to underline that we still have a base for problems in the future.
Thanks,
Serguei
Chris
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use EventFilters.filter() method and not all of
them need to be changed to call EventFilters .filtered(Event event, String
typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for NumberFormatException exception
and the location of this event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing them to use
EventFilters .filtered(event, debugeeName) makes them fail.
There is also an alternative approach I would like to suggest. All these new
Graal events are generated by the compiler threads (“JVMCI CompilerThread0,
JVMCI CompilerThread1, etc.”) and could be filtered out inside EventFilters
.filtered(Event event) method by checking the thread reference for the given
event object.
Please review the updated fix the uses this approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov ,
"serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException
: ** event IS NOT a breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+// Filters out events with location not matching the given type.
+public static boolean filteredByLocation(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null &&
typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
+
+// Filters out internal JFR events and events with location not matching
+// the given type.
+public static boolean filtered(Event event, String typeName) {
+
+return filtered(event) || filteredByLocation(event,typeName);
+
+}
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second method and rename filteredByLocation()
to filtered():
+// Filters out events with location not matching the given type.
+public static boole
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
On 6/12/18 12:02, Chris Plummer wrote:
On 6/12/18 11:53 AM, serguei.spit...@oracle.com wrote:
Hi Daniil,
I don't think, the approach for the tests to filter out the
JFR and Graal threads
is not good despite the fact it is relatively easy to
implement.
It does not cover any threads that may come in the future.
So that I consider the webrev below as just a work around of
the real problem.
True, but his changes are just an extension of what is already
being done - filtering out threads we don't care about.
I understand it.
The tests should just filter out
everything that is not a part of their testing.
It can be done on a base of the Debuggee classes or threads,
or any other approaches.
One problem is that events don't always come in on debuggee
thread, so you can't filter out all threads other than the
debuggee thread. I think probably filtering out all classes not
related to the debuggee is probably the way to go. However, do we
want to fix all the tests to do this, or just go with Daniil's
quick fix?
In general, I'm Okay with this quick workaround fix.
But wanted to underline that we still have a base for problems in
the future.
Thanks,
Serguei
Chris
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to
be changed to call EventFilters
.filtered(Event event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing
them to use EventFilters
.filtered(event, debugeeName) makes them fail.
There is also an
alternative approach I would like to suggest. All these
new Graal events are generated by the compiler threads
(“JVMCI CompilerThread0, JVMCI CompilerThread1, etc.”)
and could be filtered out inside EventFilters .filtered(Event event) method by checking
the thread reference for the given event object.
Please review the updated fix the uses
this approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests
fail with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
On 6/12/18 11:53 AM,
serguei.spit...@oracle.com wrote:
Hi Daniil,
I don't think, the approach for the tests to filter out the JFR
and Graal threads
is not good despite the fact it is relatively easy to implement.
It does not cover any threads that may come in the future.
So that I consider the webrev below as just a work around of the
real problem.
True, but his changes are just an extension of what is already being
done - filtering out threads we don't care about.
The tests should just filter out everything that is not a part
of their testing.
It can be done on a base of the Debuggee classes or threads, or
any other approaches.
One problem is that events don't always come in on debuggee thread,
so you can't filter out all threads other than the debuggee thread.
I think probably filtering out all classes not related to the
debuggee is probably the way to go. However, do we want to fix all
the tests to do this, or just go with Daniil's quick fix?
Chris
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to be
changed to call EventFilters
.filtered(Event event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing
them to use EventFilters
.filtered(event, debugeeName) makes them fail.
There is also an
alternative approach I would like to suggest. All these
new Graal events are generated by the compiler threads
(“JVMCI CompilerThread0, JVMCI CompilerThread1, etc.”) and
could be filtered out inside EventFilters .filtered(Event event) method by checking the
thread reference for the given event object.
Please review the updated fix the uses
this approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second
method and rename filteredByLocation()
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Additionally, this fix makes an
assumption about the Graal/JVMCI thread names.
Our fix should allow to rename them in the future.
Thanks,
Serguei
On 6/12/18 11:53, serguei.spit...@oracle.com wrote:
Hi Daniil,
I don't think, the approach for the tests to filter out the JFR
and Graal threads
is not good despite the fact it is relatively easy to implement.
It does not cover any threads that may come in the future.
So that I consider the webrev below as just a work around of the
real problem.
The tests should just filter out everything that is not a part
of their testing.
It can be done on a base of the Debuggee classes or threads, or
any other approaches.
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to be
changed to call EventFilters
.filtered(Event event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing
them to use EventFilters
.filtered(event, debugeeName) makes them fail.
There is also an
alternative approach I would like to suggest. All these
new Graal events are generated by the compiler threads
(“JVMCI CompilerThread0, JVMCI CompilerThread1, etc.”) and
could be filtered out inside EventFilters .filtered(Event event) method by checking the
thread reference for the given event object.
Please review the updated fix the uses
this approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov ,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second
method and rename filteredByLocation()
to filtered():
+ // Filters out events with location not matching the given type.
+ public static boolean filtered(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
I don't think, the approach for the tests to filter out the JFR
and Graal threads
is not good despite the fact it is relatively easy to implement.
It does not cover any threads that may come in the future.
So that I consider the webrev below as just a work around of the
real problem.
The tests should just filter out everything that is not a part of
their testing.
It can be done on a base of the Debuggee classes or threads, or
any other approaches.
Thanks,
Serguei
On 6/12/18 10:57, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to be
changed to call EventFilters .filtered(Event
event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing them
to use EventFilters
.filtered(event, debugeeName) makes them fail.
There is also an
alternative approach I would like to suggest. All these new
Graal events are generated by the compiler threads (“JVMCI
CompilerThread0, JVMCI CompilerThread1, etc.”) and could be
filtered out inside EventFilters
.filtered(Event event) method by checking the thread
reference for the given event object.
Please review the updated fix the uses this
approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov
,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second
method and rename filteredByLocation()
to filtered():
+ // Filters out events with location not matching the given type.
+ public static boolean filtered(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
As I understand, it should filter out the events post on the
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
I didn't see this email come in before pressing send on my
previous reply. I think this is a better solution. Just one minor
nit:
340 // Filter out events generated by the compiler
threads
This comment isn't quite correct. I would assume the "JVMCI
CompilterThread" just triggers the compilation of the methods, but
compilations are actually done on what have traditionally been
called the compiler threads, of which there can be many. Usually
you'll see multiple instances of "C1 CompilerThread" and
"C2 CompilerThread". You aren't filtering out these, but
that is probably fine and not something you want to do (otherwise
tests would probably not see compilation events).
thanks,
Chris
On 6/12/18 10:57 AM, Daniil Titov wrote:
Hi Serguei,
There are 4 more tests that use
EventFilters.filter() method and not all of them need to be
changed to call EventFilters .filtered(Event
event, String typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for
NumberFormatException exception and the location of this
event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing them
to use EventFilters
.filtered(event, debugeeName) makes them fail.
There is also an
alternative approach I would like to suggest. All these new
Graal events are generated by the compiler threads (“JVMCI
CompilerThread0, JVMCI CompilerThread1, etc.”) and could be
filtered out inside EventFilters
.filtered(Event event) method by checking the thread
reference for the given event object.
Please review the updated fix the uses this
approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov
,
"serviceability-dev@openjdk.java.net
serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail
with: JDITestRuntimeException : ** event IS NOT a
breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null && typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second
method and rename filteredByLocation()
to filtered():
+ // Filters out events with location not matching the given type.
+ public static boolean filtered(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null &
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
I partly agree. There seem to be a
couple of similar tests that did not get the change to use the new
EventFilters.filtered() method. Perhaps there is something unique
about those tests that prevented them from failing, but I'm not so
sure that's a good reason for not applying this change to them
also. However, it's not a requirement that all tests (and users of
EventFilters) confine the desired events to those in the debuggee
main class. There could be more than one class being debugged, and
the test may want to see events for these other test classes.
Chris
On 6/11/18 6:18 PM, serguei.spit...@oracle.com wrote:
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+// Filters out events with location not matching the given type.
+public static boolean filteredByLocation(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
+
+// Filters out internal JFR events and events with location not matching
+// the given type.
+public static boolean filtered(Event event, String typeName) {
+
+return filtered(event) || filteredByLocation(event,typeName);
+
+}
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second method and rename filteredByLocation() to filtered():
+// Filters out events with location not matching the given type.
+public static boolean filtered(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
As I understand, it should filter out the events post on the JFR
threads.
Please, correct me if I'm wrong.
My view is that we may want to get rid of the filtered(Event
event) method and
convert all the tests to use the filtered(Event
event, String typeName) instead.
Thank you a lot for taking care about these issues!
Thanks,
Serguei
On 6/11/18 17:52, Daniil Titov wrote:
Please review the changes that fix failure of 6 JDI tests run with Graal.
The problem here is that currently the tests filter out only 2 specific JFR events (nsk.share.jdi.EventsFilter.filtered()) before analyzing them and fail if the received event doesn't match the expected one. But with Graal turned on new events generated by the compiler thread are occasionally found in the event queue and cause the tests failure. The fix makes the tests to use an additional check to filter out events not related to the debuggee class.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.01
Thanks,
Daniil
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Serguei,
There are 4 more tests that use EventFilters.filter() method and not all of
them need to be changed to call EventFilters .filtered(Event event, String
typeName). Specifically,
vmTestbase/nsk/jdi/ExceptionEvent/exception/exception001/TestDescription.java
and
vmTestbase/nsk/jdi/ExceptionEvent/catchLocation/location001/TestDescription.java
tests expect to receive an ExceptionEvent for NumberFormatException exception
and the location of this event happens to be not the debuggee class (
java.lang.Integer:652). The tests pass now and changing them to use
EventFilters .filtered(event, debugeeName) makes them fail.
There is also an alternative approach I would like to suggest. All these new
Graal events are generated by the compiler threads (“JVMCI CompilerThread0,
JVMCI CompilerThread1, etc.”) and could be filtered out inside EventFilters
.filtered(Event event) method by checking the thread reference for the given
event object.
Please review the updated fix the uses this approach.
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.02/
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Thanks!
Best regards,
Daniil
From: "serguei.spit...@oracle.com"
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov ,
"serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net"
Subject: Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException
: ** event IS NOT a breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+ // Filters out events with location not matching the given type.
+ public static boolean filteredByLocation(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null &&
typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
+
+ // Filters out internal JFR events and events with location not matching
+ // the given type.
+ public static boolean filtered(Event event, String typeName) {
+
+ return filtered(event) || filteredByLocation(event,typeName);
+
+ }
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second method and rename filteredByLocation()
to filtered():
+ // Filters out events with location not matching the given type.
+ public static boolean filtered(Event event, String typeName) {
+ if (event instanceof Locatable) {
+ Location location = ((Locatable) event).location();
+ if (location != null) {
+ ReferenceType declaringType = location.declaringType();
+ if (declaringType != null &&
typeName.equals(declaringType.name())) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }
As I understand, it should filter out the events post on the JFR threads.
Please, correct me if I'm wrong.
My view is that we may want to get rid of the filtered(Event event) method and
convert all the tests to use the filtered(Event event, String typeName) instead.
Thank you a lot for taking care about these issues!
Thanks,
Serguei
On 6/11/18 17:52, Daniil Titov wrote:
Please review the changes that fix failure of 6 JDI tests run with Graal.
The problem here is that currently the tests filter out only 2 specific JFR
events (nsk.share.jdi.EventsFilter.filtered()) before analyzing them and fail
if the received event doesn't match the expected one. But with Graal turned on
new events generated by the compiler thread are occasionally found in the event
queue and cause the tests failure. The fix makes the tests to use an additional
check to filter out events not related to the debuggee class.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.01
Thanks,
Daniil
Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **
Hi Daniil,
It looks good in general.
Some minor comments though.
http://cr.openjdk.java.net/~dtitov/8203809/webrev.01/test/hotspot/jtreg/vmTestbase/nsk/share/jdi/EventFilters.java.udiff.html
+
+// Filters out events with location not matching the given type.
+public static boolean filteredByLocation(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
+
+// Filters out internal JFR events and events with location not matching
+// the given type.
+public static boolean filtered(Event event, String typeName) {
+
+return filtered(event) || filteredByLocation(event,typeName);
+
+}
+
+
There are a couple of extra empty lines.
I'm suggesting to get rid of the second method and rename filteredByLocation() to filtered():
+// Filters out events with location not matching the given type.
+public static boolean filtered(Event event, String typeName) {
+if (event instanceof Locatable) {
+Location location = ((Locatable) event).location();
+if (location != null) {
+ReferenceType declaringType = location.declaringType();
+if (declaringType != null && typeName.equals(declaringType.name())) {
+return false;
+}
+}
+}
+return true;
+}
As I understand, it should filter out the events post on the JFR
threads.
Please, correct me if I'm wrong.
My view is that we may want to get rid of the filtered(Event
event) method and
convert all the tests to use the filtered(Event
event, String typeName) instead.
Thank you a lot for taking care about these issues!
Thanks,
Serguei
On 6/11/18 17:52, Daniil Titov wrote:
Please review the changes that fix failure of 6 JDI tests run with Graal.
The problem here is that currently the tests filter out only 2 specific JFR events (nsk.share.jdi.EventsFilter.filtered()) before analyzing them and fail if the received event doesn't match the expected one. But with Graal turned on new events generated by the compiler thread are occasionally found in the event queue and cause the tests failure. The fix makes the tests to use an additional check to filter out events not related to the debuggee class.
Bug: https://bugs.openjdk.java.net/browse/JDK-8203809
Webrev: http://cr.openjdk.java.net/~dtitov/8203809/webrev.01
Thanks,
Daniil
