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