JFR-1751674: Add new JFR Events: FileReadIOStatisticsEvent, FileWriteIOStatisticsEvent by kthatipally · Pull Request #24 · microsoft/openjdk-jdk11u

kthatipally · 2023-08-30T18:47:33Z

Summary:

AzDO Work item: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1751674

This pull request introduces code and test cases for incorporating new JDK.JFR (Java Flight Recorder) File I/O events. These events are designed to periodically monitor Read Rate and Write Rate within specified intervals. The goal is to gain valuable insights into JVM performance patterns over time. The implementation is achieved through Instrumentation.

These events are designed to capture byte data statistics (Read Rate (Bytes/Sec), Write Rate((Bytes/Sec))) associated with JDK APIs, specifically those from classes like FileOutputStream, FileInputStream, RandomAccessFile, and sun.nio.ch.FileChannelImpl.

It's important to note that these events do not include byte data related to native file operations. (Native library loads).
By default, the events are disabled in default.jfc and profile.jfc

The events attributes:

FileReadIO Statistics	FileWriteIO Statistics	Description
Start Time (default)	Start Time (default)	The start time of the event record.
Duration (default)	Duration (default)	startTime - endTime
End Time (default)	End Time (default)	The end time of the event record
Event Thread (default)	Event Thread (default)	JFR Periodic Thread
Read Rate (Bytes/sec)	Write Rate (Bytes/sec)	Rate at which the bytes are written/Read from the file operations.
Total Accumulated Read Bytes	Total Accumulated Write Bytes	Total bytes accumulated by the end of the JVM Process

Events Screenshots from JMC:

…r Read and Write

…eriodicFileIOInstrumentationEvents

…ntationEvents' of https://github.com/microsoft/openjdk-jdk11u into kthatipally/JFR-Internal-1751674-PeriodicFileIOInstrumentationEvents

…nnel

src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/FileChannelImplInstrumentor.java

JohnTortugo · 2023-09-01T00:47:46Z

test/hotspot/jtreg/runtime/appcds/CDSandJFR.java

        "Hello",
        "GetFlightRecorder",
        "GetFlightRecorder$TestEvent",
+        "jdk/jfr/events/FileReadIOStatisticsEvent",


NIT: can you move these "jdk/jfr/..." tests closer to the ones above?

I moved the tests closer to the other fileEvent tests. Thanks for the suggestion.

JohnTortugo · 2023-09-01T00:50:16Z

test/jdk/jdk/jfr/event/io/TestFileIOStatisticsEvents.java

+import static jdk.test.lib.Asserts.assertGreaterThanOrEqual;
+
+/**
+ * @test


You may need to add the JBS work item ID here.

JohnTortugo · 2023-09-01T00:51:55Z

src/jdk.jfr/share/conf/jfr/default.jfc

    </control>

-</configuration>
+</configuration>


NIT: missing end of line.

JohnTortugo · 2023-09-01T00:53:57Z

src/jdk.jfr/share/classes/jdk/jfr/events/FileReadIOStatisticsEvent.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2012, 2018, Oracle and/or its affiliates. All rights reserved.


People may ask you to adjust the year in the files' header.

Updated year to 2023. Thanks for catching that.

src/jdk.jfr/share/classes/jdk/jfr/events/FileReadIOStatisticsEvent.java

JohnTortugo · 2023-09-01T00:55:26Z

src/jdk.jfr/share/classes/jdk/jfr/events/FileReadIOStatisticsEvent.java

+        totalReadBytesForPeriod.addAndGet(bytesRead);
+    }
+
+    // returning rate


NIT: improve comment?

Thank you for the suggestion. I've added a clear comment as per your feedback.

JohnTortugo · 2023-09-01T00:56:46Z

src/jdk.jfr/share/classes/jdk/jfr/events/FileReadIOStatisticsEvent.java

+        if (interval > 0) {
+            totalDuration.addAndGet(-interval);
+            double intervalInSec = (interval * 1.0 / 1_000_000_000);
+            long rRate = (long) (result / intervalInSec);


I think the cast may not be needed here.

@JohnTortugo , The "(result / intervalInSec)" was giving a double and erroring out with "error: incompatible types: possible lossy conversion from double to long". So, I had to explicitly add long to prevent data loss.

…ng the writebytes and read bytes if the events are not enabled, updated year in the description, added code comments

macarte · 2023-09-07T13:57:09Z

src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/FileInputStreamInstrumentor.java

+        }
+        if (readPeriodicEvent.isEnabled()) {
+            long duration = System.nanoTime() - startTime;
+            FileReadIOStatisticsEvent.setTotalReadBytesForPeriod(1, duration);


I've just realized that in the "read" cases, there's potential to report more than was actually read, in this case we report 1 , but should report using a test '(result < 0) ? 0: 1)'

Thanks @macarte for catching that. This needs to be fixed at multiple places. I'll make the necessary changes.

… to decrease the error in getting the accumulated bytes number

Keerthi Thatipally and others added 15 commits May 2, 2023 11:41

Initial Draft: FileIOStatistics JFR Periodic Events Implementtaion fo…

777359f

…r Read and Write

1751674 - Added chnages to decouple existing File Events and new events

8a90d57

Merge branch 'openjdk:master' into kthatipally/JFR-Internal-1751674-P…

c818af0

…eriodicFileIOInstrumentationEvents

Add updates to the rate calculation functionality

7f5794d

updates to the writeIO functionality and add JTreg tests

9b70373

Clean up the code and add formatting

f5cab6f

Merge branch 'kthatipally/JFR-Internal-1751674-PeriodicFileIOInstrume…

2dbc772

…ntationEvents' of https://github.com/microsoft/openjdk-jdk11u into kthatipally/JFR-Internal-1751674-PeriodicFileIOInstrumentationEvents

Fixed an issue with the FileReadI/O statistics and updated JTreg tests

cbe7f66

Formatting updates

5f97ea6

Refactor: Removed comments, refactored methods

1035583

Added new JTreg tests

8b8a2b8

Fix formatting in profile.jfc

e26509c

Revert FileRead and FileWrite threshold values

717a0eb

Formatted, removed print statements and added a JTreg test on FileCha…

952831d

…nnel

changed the default.jfc configuration and added formatting

fdc7c12

kthatipally requested a review from macarte August 30, 2023 19:17

macarte reviewed Aug 31, 2023

View reviewed changes