[Rev 01] RFR: 8244297: memory leak test utility

[Florian Kirmaier]

On Mon, 4 May 2020 13:47:58 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   JDK-8244297
>>   Updated JMemoeryBuddy
>>   Added Copyright header
>>   disabled default heapdump
>
> modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 1:
> 
>> 1: package de.sandec.jmemorybuddy;
>> 2:
> 
> This file needs a standard Oracle copyright header, which you can copy from any of the other test files.
> 
> Also, this utility class should be moved into a package matching the pattern we already use for our tests. Something
> like `test.util.memory`?

Done!

> modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 15:
> 
>> 14:
>> 15: public class JMemoryBuddy {
>> 16:
> 
> Since this is a utility intended to be used by various tests, it would be very helpful to add some documentation about
> how to use it.

Where/How should I add the documentation? Ideally, I would reference the documentation of the project itself.

> modules/javafx.base/src/test/java/de/sandec/jmemorybuddy/JMemoryBuddy.java line 145:
> 
>> 144:
>> 145:     public static void createHeapDump() {
>> 146:         try {
> 
> I don't think we want automatic creation of heap dumps by our unit test suite. That would need a larger discussion. I
> recommend to either comment this out, or else to qualify it with a system property that could be passed in via a
> `gradle` property (e.g., as is done for `UNSTABLE_TEST`), with a default of `false`.

I've made it configurable in the Library with system-parameters and I've changed the default not create a heap dump.
Is that ok?

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

PR: https://git.openjdk.java.net/jfx/pull/204