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

[Alex Macdonald]

On Mon, 9 Aug 2021 23:15:26 GMT, Suchita Chaturvedi <schaturvedi at openjdk.org> wrote:

> This PR addresses the NullPointerException issue in core module for the following rules:
> 
> 1. GCs Caused by System.gc()
> 2. GCs Caused by Heap Inspection
> 3. GC Stall
> 4. String Deduplication
> 5. GCs Caused by GC Locker

I think these changes are okay, although they address the issues with the individual rules rather than the system itself. I'd appreciate if @Gunde could lend some expertise or insight here.

>From my time looking through the application-side code, the `RuleManager` makes two checks to determine if a rule should be evaluated [[0]](https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L138), and these are to:

1. check the rule's availability, which happens in `RulesToolkit.matchesEventAvailabilityMap()` [[1]](https://github.com/openjdk/jmc/blob/master/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java#L494), and
2. check the rule's dependencies (which happens in `RuleManager.shouldEvaluate()` [[2]](https://github.com/openjdk/jmc/blob/511071d2111a9c303b3aa88219200ad81c04f835/core/tests/org.openjdk.jmc.flightrecorder.rules.jdk.test/src/test/java/org/openjdk/jmc/flightrecorder/test/rules/jdk/TestRulesWithJfr.java#L245))

However, the core implementation that leads into `JfrHtmlRulesReport` does not perform these two checks, which results in the errors for this issue.

To address problem 1 listed above, we're in luck because the `matchesEventAvailabilityMap()` is in core already in `RulesToolkit`, so this could be used when evaluating rules at `RulesToolkit.evaluateParallel()` [[3]](https://github.com/openjdk/jmc/blob/master/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java#L1213). This alleviates the issue from the `StringDeduplicationRule`, although it's still probably nice to have the null check included in this PR just incase something happens. Additionally, for the JFR file I was using to test this bug, by checking the rules with `matchesEventAvailabilityMap()` I was able to bypass several rules (including StringDeduplicationRule) that looked to be handled properly within their own rule implementation however could be omitted in my case. For context they were: Class Loading Pressure, Fatal Errors, Heap Content, DMS Incidents, Class Leak, Code Cache, Heap Dump, Competing Processe
 s, Exceptional Dump Reason, Java Blocking, Socket Write Peak Duration, Process Started, Primitive To Object Conversion. I'm thinking that adding a check for the rules before evaluating might be nice for future-proofing if new rules are potentially added in the future.

To address problem 2, it's not as easy as reusing the `shouldEvaluate()` function because that lies in application, but also because it checks currently evaluated rules to determine if a rule depends on another one that has already ran, which isn't checked in core. There are only four rules with dependencies, and those are the four GC rules that are adjusted in this PR, so this is an okay work around. However, it might be nice to have a way of checking dependent rules here just incase new ones are made down the line.

[0] https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L138
[1] https://github.com/openjdk/jmc/blob/master/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java#L494
[2] https://github.com/openjdk/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L107
[3] https://github.com/openjdk/jmc/blob/master/core/org.openjdk.jmc.flightrecorder.rules/src/main/java/org/openjdk/jmc/flightrecorder/rules/util/RulesToolkit.java#L1213

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

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