-
Bug
-
Resolution: Fixed
-
21w37a
-
None
-
Plausible
-
(Unassigned)
The snapshot 21w37a of Minecraft adds user defined JFR events. This is really cool!
I work on OpenJDK and JDK Flight Recorder, so I'm excited to see it being used in Minecraft. I will try to use recordings from Minecraft in demos.
I thought I bring some feedback when it comes to the event metadata.
The event "minecraft.ServerTickTime" has a field called averageTickMs of type float, with the annotation @Timespan(Timespan.MILLISECONDS).
Unfortunately, this doesn't work well with the API to consume JFR recordings, in particular the jdk.jfr.RecordedObject::getDuration(String) method, which can only operate on fields of integral data types. Float values may become too large to fit into a java.time.Duration object, which uses a long for seconds internally, so we can't allow this.
This can be seen when using the JAVA_HOME/bin/jfr tool:
$ jfr print minecraft.jfr
Which will result in an IllegalArgumentException.
(This doesn't happen in JDK Mission Control, because it doesn't use the parser that comes with the JDK as it needs to be backward compatible with old versions of the Oracle JDK. JMC converts timespans to nanos since 1970 and uses long values internally, which has its own limitations, but works most of the time.)
The easiest way to fix this is to take the value and multiply it by 1_000_000 and cast it to a long and use @Timespan(Timespan.NANOSECONDS).
If you are poking around in JFR the code, you may consider the following changes to make event metadata consistent with other events and more in line with recommendations [1][2]
------------------------------------------------------------
Event: minecraft.ChunkGeneration:
Label("Chunk generation duration") -> Label("Chunk Generation")
Label("First block x world position") -> Label("First Block X World Position")
long worldPosX -> long worldPositionX;
...
Label("Chunk x position") -> Label("Chunk X Position")
int chunkPosX -> int chunkPositionX;
...
We have tried to avoid abbreviation, such as Info, Alloc, Pos in field names. Labels should use headline capitalization.
Add Label("Status")
Add Label("Level")
Add Label("Success")
JMC creates a label from the field name, if it is missing, but that is not the case with the API in the JDK as it doesn't work well in some locales.
------------------------------------------------------------
Event: minecraft.PacketRead / minecraft.PacketSend
Label("Packet name") -> Label("Packet Name)
Label("Remote address") -> Label("Remote Address")
------------------------------------------------------------
Event: minecraft.ServerTickTime:
Label("Server tick time") ->Label("Server Tick Time")
Label("Average server tick time (ms)") -> Label("Average Server Tick Time")
@Timespan("MILLISECONDS") -> @Timespan("NANOSECONDS")
float averageTickMs -> long averageTick
There is no need to include the unit in the name, if it has been annotated with a timespan.
------------------------------------------------------------
Event: minecraft.WorldLoadFinishedEvent
Name("minecraft.WorldLoadFinishedEvent") -> Name("minecraft.LoadWorld")
Label("World create/load duration") -> Label("Create/Load World")
Avoid using the word "Event" in the name.
[1] [https://docs.oracle.com/en/java/javase/16/docs/api/jdk.jfr/jdk/jfr/Label.html
[2] https://docs.oracle.com/en/java/javase/16/docs/api/jdk.jfr/jdk/jfr/Name.html