Re: RFR 8203809: [Graal] JDI tests fail with: JDITestRuntimeException : ** event IS NOT a breakpoint **

Tue, 12 Jun 2018 11:55:28 -0700

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" <serguei.spit...@oracle.com>
Date: Monday, June 11, 2018 at 6:18 PM
To: Daniil Titov <daniil.x.ti...@oracle.com>, "serviceability-dev@openjdk.java.net 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
 
 

Reply via email to