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 |
- RFR 8203809: [Graal] JDI tests fail with: JDITe... Daniil Titov
- Re: RFR 8203809: [Graal] JDI tests fail wi... serguei.spit...@oracle.com
- Re: RFR 8203809: [Graal] JDI tests fai... Daniil Titov
- Re: RFR 8203809: [Graal] JDI tests... Chris Plummer
- Re: RFR 8203809: [Graal] JDI tests... serguei.spit...@oracle.com
- Re: RFR 8203809: [Graal] JDI t... serguei.spit...@oracle.com
- Re: RFR 8203809: [Graal] JDI t... Chris Plummer
- Re: RFR 8203809: [Graal] ... serguei.spit...@oracle.com
- Re: RFR 8203809: [Gra... Daniil Titov
- Re: RFR 8203809: ... serguei.spit...@oracle.com
- Re: RFR 8203809: ... daniil . x . titov
- Re: RFR 8203809: ... Daniil Titov