RFR: 7122: Rules evaluation never complete

[Henrik Dafgård]

On Wed, 28 Jul 2021 20:04:39 GMT, Alex Macdonald <aptmac at openjdk.org> wrote:

> This PR addresses JMC-7122 [[0]](https://bugs.openjdk.java.net/browse/JMC-7122), in which certain rules (mainly GC) do not finish evaluation.
> 
> This occurs when the `result` [[1]](https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L133) of a rule is never updated, and still holds it's initialized value of `IN_PROGRESS`  despite exiting the evaluation loop. What happens in the case of the 4 GC rules listed in the bug report (**1**. GCs Caused by System.gc(), **2**. GCs Caused by Heap Inspection, **3**. GC Stall, **4**. GCs Caused by GC Locker) is that they have a dependency on `GarbageCollectionInfoRule`, which ends up being ignored [[2]](https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L164) because it doesn't match the event availability map. As a result, the 4 GC rules cause `shouldEvaluate(rule)` [[3]](https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc
 /flightrecorder/ui/RuleManager.java#L143) to return false, but because the result value was already initialized it isn't caught by the subsequent if-statements.
> 
> In this approach, the conditional is adjusted to check the event availability map and `shouldEvaluate()` at the same time, because if either of them are false we should ignore that rule.
> 
> Before:
> ![2021-07-28-160356_1489x270_scrot](https://user-images.githubusercontent.com/10425301/127388246-5d22181c-317e-44ec-a059-9ade1f212dab.png)
> 
> After:
> ![2021-07-28-160243_1087x303_scrot](https://user-images.githubusercontent.com/10425301/127388151-4ab862f5-49f9-4b40-a86f-b6f167d20771.png)
> 
> 
> [0] https://bugs.openjdk.java.net/browse/JMC-7122
> [1] https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L133
> [2] https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L164
> [3] https://github.com/aptmac/jmc/blob/master/application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/RuleManager.java#L143

This looks good to me, we should investigate whether or not that thread sleep is needed at all, but that can be a separate issue.

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

Marked as reviewed by hdafgard (Reviewer).

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