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

Brice Dutheil bdutheil at openjdk.org
Thu Nov 3 15:10:13 UTC 2022


On Thu, 6 Oct 2022 20:28: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.
>
> Suchita Chaturvedi has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Adding higher timeout as default value

I'm still not at ease there. I think contributing an actual test case for this rule (and/or the `DefaultIItemResultSet`) might beneficial.

Having a synchronized block wrapping the `add(row)`, looks suspicious as this somehow coordinate threads but this behavior still non-determinist as it depends on how threads get scheduled. 

As per this [comment](https://github.com/openjdk/jmc/pull/419#issuecomment-1273226601)
> To be more precise, by wrong results, I mean the size of data structure (I am trying printing while debugging) and the final top 5 results are not consistent.

I do believe this happen due to the reason mentioned above. Maybe the resulting list needs to be sorted then ?

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

> 63: 	private final ArrayList<Object[]> data = new ArrayList<>();
> 64: 	private int cursor = -1;
> 65: 	private ExecutorService exec;

This could be final.

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

> 143: 				}
> 144: 				if (Thread.currentThread().isInterrupted())
> 145: 					return;

This `if` statement is irrelevant now, as it's the end of this function regardless the interruption status.

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

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


More information about the jmc-dev mailing list