RFR: 7879: Automated Analysis taking very long time to produce results for Class Leak Rule and showing wrong results.

[Henrik Dafgård]

On Sat, 20 Aug 2022 18:59:14 GMT, Suchita Chaturvedi <schaturvedi at openjdk.org> wrote:

> This PR takes care of optimizing and correcting the Class leak rule results on Automated Analysis Page.

I have a few nits and one necessary change before we can merge this.

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/ClassLeakingRule.java line 108:

> 106: 			Messages.getString(Messages.ClassLeakingRule_CONFIG_CALCULATION_TIMEOUT),
> 107: 			Messages.getString(Messages.ClassLeakingRule_CONFIG_CALCULATION_TIMEOUT_LONG), NUMBER,
> 108: 			NUMBER_UNITY.quantity(5));

Nit: The timeout value should be a timestamp instead.

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/ClassLeakingRule.java line 131:

> 129: 			.addEventType(JdkTypeIDs.CLASS_LOAD, EventAvailability.ENABLED)
> 130: 			.addEventType(JdkTypeIDs.CLASS_UNLOAD, EventAvailability.ENABLED).build();
> 131: 	public static int CONFIGURED_TIMEOUT = 0;

This should not be public or static, it should be passed along as a parameter where needed instead of being a field.

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/general/ClassLeakingRule.java line 156:

> 154: 		IItemCollection items, IPreferenceValueProvider valueProvider, IResultValueProvider dependencyResults) {
> 155: 		int warningLimit = (int) valueProvider.getPreferenceValue(WARNING_LIMIT).longValue();
> 156: 		CONFIGURED_TIMEOUT = (int) valueProvider.getPreferenceValue(MAX_TIMEOUT).longValue();

Nit: This preference should use .clampedLongValueIn and passing in the appropriate unit, so since the timeout use of this specifies TimeUnit.MINUTES we should use valueProvider.getPreferenceValue(MAX_TIMEOUT).clampedLongValueIn(UnitLookup.MINUTE).longValue().

core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/util/DefaultIItemResultSet.java line 135:

> 133: 					exec.shutdown();
> 134: 					exec.awaitTermination(ClassLeakingRule.CONFIGURED_TIMEOUT, TimeUnit.MINUTES);
> 135: 				}

This timeout should be passed as a parameter instead as this could differ if rules are run concurrently with different configurations for this. Also if we would want to use this method from another rule it would be impossible to set the timeout properly.

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

Changes requested by hdafgard (Reviewer).

PR: https://git.openjdk.org/jmc/pull/419