RFR: 8329138: Convert JFR FileForceEvent to static mirror event [v2]
Tim Prinzing
tprinzing at openjdk.org
Mon Apr 15 20:44:17 UTC 2024
On Tue, 2 Apr 2024 04:48:37 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>> Tim Prinzing has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Add support for AsynchronousFileChannel.force().
>
> src/java.base/share/classes/jdk/internal/event/FileForceEvent.java line 35:
>
>> 33: * {@link #commit(long, long, String, boolean)} method
>> 34: * must be the same as the order of the fields.
>> 35: */
>
> You may have to re-word this comment to avoid confusion with the metaData parameter. That is, there is event meta data and there is file metaData, if you see what I mean.
I see what you mean. I reworded the javadoc.
> test/jdk/jdk/jfr/event/io/TestAsynchronousFileChannelEvents.java line 50:
>
>> 48:
>> 49: public static void main(String[] args) throws Throwable {
>> 50: File blah = File.createTempFile("blah", null);
>
> Can you change this to use the tests's scratch rather that /tmp, meaning `Files.createTempFile(Path.of("."), "blah", "jfr")`. That way the recording is available in the event that the test fails.
I'm not sure what you mean about the recording. The file is the AsynchronousFileChannel under test and does not contain the event recording.
> test/jdk/jdk/jfr/event/io/TestAsynchronousFileChannelEvents.java line 64:
>
>> 62:
>> 63: data.flip();
>> 64: ch.write(data, 0);
>
> This just initiates the write operation, it doesn't wait until it completes. It returns a Future so adding .get() will ensure that it waits and that there is potentially data to write back to the file system.
I do realize the write doesn't wait. I was under the impression that flush() does wait until everything has been flushed to disk. I went ahead and added .get() as requested.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18542#discussion_r1566405842
PR Review Comment: https://git.openjdk.org/jdk/pull/18542#discussion_r1566413683
PR Review Comment: https://git.openjdk.org/jdk/pull/18542#discussion_r1566409791
More information about the nio-dev
mailing list