RFR: 7231: JMC createReport for JMC8 Automated Analysis fails to evaluate several rules [v3]

Marcus Hirt hirt at openjdk.java.net
Fri Oct 1 16:58:45 UTC 2021


On Wed, 29 Sep 2021 17:14:18 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

>> This PR addresses JMC-7231 [[0]](https://bugs.openjdk.java.net/browse/JMC-7231), in which using `JfrHtmlRulesReport` throws a number of exceptions and fails to evaluate a handful of rules.
>> 
>> I used some of the review comments [[1]](https://github.com/openjdk/jmc/pull/302#pullrequestreview-726780649) that I made in the other open PR to address the same issue.
>> 
>> As far as I can tell, there are two main issues here.
>> 
>> The first is some rules now (after the Rules 2.0 update) have dependencies, these are (4) `GcLockerRule`, `GcStallRule`, `HeapInspectionRule`, and `SystemGcRule` which depend on `GarbageCollectionInfoRule`. The problem here is that if `GarbageCollectionInfoRule` isn't evaluated then the other four rules end up throwing an NPE while trying to evaluate.
>> 
>> The second issue is that as of Rules 2.0, on the jmc.ui Automated Analysis page we use `RulesToolkit.matchesEventAvailabilityMap()` to verify is an event is is available and if the rule should be evaluated. This check wasn't replicated on the `core` implementation, so I've added it in. What's interesting here is that in the test jfr I was using, `GarbageCollectionInfoRule` was filtered out by `matchesEventAvailabilityMap()`, which was causing the above 4 Gc rules to explode.
>> 
>> The solution I've put together uses two lists to track 1). rules that are unavailable due to being filtered out by `matchesEventAvailabilityMap()`, and 2). rules that have dependencies. It later loops through the list of rules with dependencies, and checks to see if they exist in the list with unavailable rules. If a rule is not able to be evaluated by either 1). not being available or 2). by having a dependency that is not available, it returns a completed future `IResult` containing "not available", similar to how on this is handled on the jmc.ui side. Then the rest of the rules report proceeds as usual.
>> 
>> I think the easiest way to verify/try this fix is by creating a run configuration for JfrHtmlRulesReport in Eclipse, and feeding it a jfr file.
>> 
>> [0] https://bugs.openjdk.java.net/browse/JMC-7231
>> [1] https://github.com/openjdk/jmc/pull/302#pullrequestreview-726780649
>
> Alex Macdonald has updated the pull request incrementally with one additional commit since the last revision:
> 
>   run mvn spotless:apply

Changes requested by hirt (Lead).

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/messages/internal/Messages.java line 49:

> 47: 	public static final String RulesToolkit_ATTRIBUTE_NOT_FOUND = "RulesToolkit_ATTRIBUTE_NOT_FOUND"; //$NON-NLS-1$
> 48: 	public static final String RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG = "RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG"; //$NON-NLS-1$
> 49: 	public static final String RulesToolkit_EVALUATION_ERROR_DESCRIPTION = "RulesToolkit_EVALUATION_ERROR_DESCRIPTION"; //$NON-NLS-1$

Don't forget to update the copyright year!

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java line 50:

> 48: import java.util.Queue;
> 49: import java.util.Set;
> 50: import java.util.concurrent.CompletableFuture;

Copyright year!

core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java line 1281:

> 1279: 	 * report.
> 1280: 	 * 
> 1281: 	 * @param rule

Missing Javadoc for rule, preferences and return.

core/org.openjdk.jmc.flightrecorder.rules/src/main/resources/org/openjdk/jmc/flightrecorder/rules/messages/internal/messages.properties line 37:

> 35: # {0} is an event type id, {1} is an attribute id
> 36: RulesToolkit_ATTRIBUTE_NOT_FOUND_LONG=Could not get the attribute with id ''{0}'' from the event type with id ''{1}''. This recording is likely from an unsupported version.<p>If you are using an older Java version, then you can consider upgrading.
> 37: RulesToolkit_EVALUATION_ERROR_DESCRIPTION=Could not evaluate this rule

Copyright year.

core/tests/org.openjdk.jmc.flightrecorder.rules.test/pom.xml line 70:

> 68:                    <version>${project.version}</version>
> 69:            </dependency>
> 70:            <dependency>

Copyright year.

core/tests/org.openjdk.jmc.flightrecorder.rules.test/src/test/java/org/openjdk/jmc/flightrecorder/rules/RulesToolkitTest.java line 38:

> 36: import static org.junit.Assert.assertFalse;
> 37: import static org.junit.Assert.assertTrue;
> 38: import static org.junit.Assert.fail;

Copyright year.

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

PR: https://git.openjdk.java.net/jmc/pull/311


More information about the jmc-dev mailing list