RFR: 8256156: JFR: Allow 'jfr' tool to show metadata without a recording

Erik Gahlin egahlin at openjdk.java.net
Fri Jan 8 04:28:59 UTC 2021


On Tue, 29 Dec 2020 07:44:36 GMT, Yang Yi <github.com+5010047+kelthuzadx at openjdk.org> wrote:

> Hi all,
> 
> May I please have a review for this minor patch?
> 
> It simplifies the use of subcommand `metadata` of jfr tool. Recording 
> file is no longer mandatory, users can directly use `jfr metadata` to 
> output JDK builtin meta information. As JDK-8256156 mentioned, it also 
> supports events and categories filters:
> $ jfr metadata 
> $ jfr metadata recording.jfr # compatible
> $ jfr metadata --events jdk.ThreadStart,jdk.ThreadEnd
> $ jfr metadata --events jdk.ThreadStart,jdk.ThreadEnd recording.jfr
> $ jfr metadata --categories GC,Detailed
> $ jfr metadata --categories GC,Detailed recording.jfr

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/Metadata.java line 200:

> 198:                 prettyWriter.flush(true);
> 199:             }
> 200:         } catch (Exception e) {

This error handling seems inadequate. If there is IOException, it still makes sense to get proper message, and not a stack trace

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/Metadata.java line 183:

> 181:                 }
> 182:                 if (foundEventFilter) {
> 183:                     if (acceptedEvents.contains(type.getName())) {

I would expect filtering for the metadata command to work like the 'print' command (accepting acronyms, wildcards etc.)

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/Metadata.java line 211:

> 209:             String[] categories = categoryAnno.value();
> 210:             for (String category : categories) {
> 211:                 if (categoryNames.contains(category)) {

I would expect filtering for the metadata command to work like the 'print' command (wildcards etc.)

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/Metadata.java line 231:

> 229:                     return tmp;
> 230:                 } catch (Exception e) {
> 231:                     // ignored since recording file for jfr metadata is optional

Inadequate error handling. If there is something wrong with the file (permission etc) I like to know it, and not get the event types for the current JVM.

test/jdk/jdk/jfr/tool/TestMetadata.java line 94:

> 92:             }
> 93:         }
> 94:         List<Type> eventTypes = TypeLibrary.getInstance().getTypes();

I think it would be better to use public API (FlightRecorder.getEventTypes()) instead of calling internals.

src/jdk.jfr/share/classes/jdk/jfr/internal/tool/Metadata.java line 107:

> 105:     }
> 106: 
> 107:     @Override

I think ine would like to get similar help as for the ''print' command when it comes to filtering.

-------------

PR: https://git.openjdk.java.net/jdk/pull/1904


More information about the hotspot-jfr-dev mailing list