RFR: 5560: Support new nio events in JDK 10.

Alex Macdonald aptmac at openjdk.org
Wed Nov 22 14:59:27 UTC 2023


On Mon, 13 Nov 2023 21:17:23 GMT, Suchita Chaturvedi <schaturvedi at openjdk.org> wrote:

> This PR addresses the enhancement request of missing event File Force.
> 1. A new rule File Force has been added to the "Automated Analysis Page", Under Java Application -> File I/O.
> <img width="368" alt="image" src="https://github.com/openjdk/jmc/assets/11155712/833747bf-39b8-455f-9081-f7a23a6de3da">
> 
> 2. "File I/O" screen is updated to accomodate values for the new event. There are two new columns added - Force Count, Update Metadata. Both these columns are hidden by default and user need to make them visible by right clicking the header of the table. Also the corresponding chart for File Force Event has been introduced.
> <img width="960" alt="image" src="https://github.com/openjdk/jmc/assets/11155712/027400c5-ae83-499e-b4b5-531fe3b74e6a">
> 
> 3. There is a new preference/configuration introduced for the File Force Rule.
>  
> <img width="313" alt="image" src="https://github.com/openjdk/jmc/assets/11155712/968b341e-e0cb-4d21-9288-71d56a0b5883">

Looks good, I left a few comments inline. The license headers on the modified files will need to be updated as well.

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/io/FileForceRule.java line 83:

> 81: 			.addEventType(JdkTypeIDs.FILE_FORCE, EventAvailability.ENABLED).build();
> 82: 
> 83: 	//public static final TypedResult<IQuantity> LONGEST_FORCE_AMOUNT = new TypedResult<>("longestForceAmount", //$NON-NLS-1$

Commented out, could be removed?

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/io/FileForceRule.java line 111:

> 109: 
> 110: 		// Aggregate of all file forced events - if null, then we had no events
> 111: 		if (longestEvent == null) {

I think if you used `EventAvailability.AVAILABLE` instead of `EventAvailability.ENABLED` for the REQUIRED_EVENTS at line 81 then there'd be no need to check if there's any events here.

core/org.openjdk.jmc.flightrecorder/src/main/java/org/openjdk/jmc/flightrecorder/jdk/messages/internal/Messages.java line 99:

> 97: 	public static final String AGGR_FILE_FORCE_COUNT = "AGGR_FILE_FORCE_COUNT"; //$NON-NLS-1$
> 98: 	public static final String AGGR_FILE_FORCE_COUNT_DESC = "AGGR_FILE_FORCE_COUNT_DESC"; //$NON-NLS-1$
> 99: 

Doesn't need the new line?

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

PR Review: https://git.openjdk.org/jmc/pull/533#pullrequestreview-1732991487
PR Review Comment: https://git.openjdk.org/jmc/pull/533#discussion_r1394802748
PR Review Comment: https://git.openjdk.org/jmc/pull/533#discussion_r1394806117
PR Review Comment: https://git.openjdk.org/jmc/pull/533#discussion_r1402182224


More information about the jmc-dev mailing list