RFR: 8331876: JFR: Move file read and write events to java.base [v3]
Daniel Fuchs
dfuchs at openjdk.org
Thu May 9 14:43:56 UTC 2024
On Thu, 9 May 2024 11:19:14 GMT, Erik Gahlin <egahlin at openjdk.org> wrote:
>> Hi,
>>
>> Could I have a review of a change that moves the jdk.FileRead and jdk.FileWrite events to java.base to remove the use of the ASM instrumentation.
>>
>> Testing: jdk/jdk/jfr
>>
>> Thanks
>> Erik
>
> Erik Gahlin has updated the pull request incrementally with one additional commit since the last revision:
>
> Move methods
src/java.base/share/classes/java/io/FileInputStream.java line 63:
> 61: private static final int DEFAULT_BUFFER_SIZE = 8192;
> 62:
> 63: // Flag that determines if file reads should be traced by JFR
It could be good to also note what will modify this flag - here and in other classes.
I'm going to guess that the purpose of this flag is purely performance, to avoid even loading the event class, `FileReadEvent` here, during startup/bootstrap and when JFR is not enabled, as read and write methods are highly performance sensitive? Otherwise the flag could live in the event class itself? Is it substantially faster to check this flag compared to `FileReadEvent.enabled()`?
src/java.base/share/classes/jdk/internal/event/JFRTracing.java line 51:
> 49: field.setAccessible(true);
> 50: field.setBoolean(null, true);
> 51: }
Using reflection with `Field` seems expedient - a more modern way could be to use `VarHandle` but I guess it would require more setup to obtain a `Lookup` with the proper capabilities?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19129#discussion_r1595525764
PR Review Comment: https://git.openjdk.org/jdk/pull/19129#discussion_r1595530833
More information about the core-libs-dev
mailing list