On Mon, 26 Oct 2020 22:49:41 GMT, Kevin Rushforth <k...@openjdk.org> wrote:

>> Florian Kirmaier has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   JDK-8244297
>>   Added some more whitespaces based on review
>
> modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 150:
> 
>> 148:     public static boolean checkNotCollectable(WeakReference 
>> weakReference) {
>> 149:         createGarbage();
>> 150:         System.gc();
> 
> I recommend calling `System.runFinalization();` here.
> 
> Also, do you think there is value in checking in a loop? Or is that overkill? 
> One drawback of checking in a loop for the negative case is that you end up 
> going through the loop all the way to the end in the expected case where a 
> reference is not collectable, so it might be best to leave it as a single 
> check as you have done.

I've added runFinalization followed with another System.gc (otherwise it 
wouldn't make any difference)
I'm quite sure it's avoiding an error in the library, but I don't have a 
unit-test for it yet.

It's a tradeoff between speed and reliability. The library tries to avoid 
false-negative test results at all costs.
false-positive results are tolerated for speed if they are "sufficiently 
reliable".
The Library has tests for assertNotCollectable, which are executed with every 
commit for different JVM's, and I don't remember any false-positive results 
(which would be visible as a negative test result, because the unit-test checks 
for failing tests). For that reason, we can assume they are "sufficiently 
reliable" despite the theoretical problem.

> modules/javafx.base/src/test/java/test/util/memory/JMemoryBuddy.java line 216:
> 
>> 214:         }
>> 215: 
>> 216: 
> 
> Minor: there are extra blank lines here.

done

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

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

Reply via email to