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

[Brice Dutheil]

On Thu, 6 Oct 2022 04:54:23 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.
>
> Suchita Chaturvedi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Added check for default configured timeout

> @bric3 We need to show the names of top 5 classes which were loaded maximum times without being unloaded. So the ordering matters here. That's why I have skipped that comment.

I'm a bit confused, as in `DefaultIItemResultSet` the _insertion_ in `data` order is dependent on the iteration order of the aggregate collection, since it's a set, there's no ordering guarantees. Although I lack knowledge of the actual type and the implementation is a `TreeSet` or something alike.

https://github.com/openjdk/jmc/blob/1696e03a1fddb9cc68d32bf21a281ef24366e552/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/util/DefaultIItemResultSet.java#L95-L99

Moreover the usage of the thread pool here will not guarantee the `add` operation will happen in the same order as the tasks have been submitted. Which leads me to think that the code should be reworked here to ensure that the results will be inserted in order.

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

> 136: 							exec.shutdownNow();
> 137: 							if (configuredTimeout != 0)
> 138: 								exec.awaitTermination(configuredTimeout, TimeUnit.MINUTES);

Sorry looking at this now I think the proposition (1) in [comment](https://github.com/openjdk/jmc/pull/419#discussion_r988313757) is incorrect

* ~~a guard value like `-1` is better and in `DefaultIItemResultSet` just skip the termination.~~ 

If the value is `0` (or the `-1` which conveys to human this is an invalid value and a such disable the timeout) the code should just wait that all results have been delivered. Also maybe make this guard value a constant that can be reused in `ItemResultSetFactory`.

Instead the other proposition, to have a very large termination timeout, might be more appropriate in the present case, JDK 19's `ExecutorService.close()` for example use a timeout of _1 day_.

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

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