On Wed, 5 Feb 2020 22:50:59 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> The finalize() method is deprecated in JDK9. See [Java 9 deprecated >> features](https://www.oracle.com/technetwork/java/javase/9-deprecated-features-3745636.html). >> And so the >> [PiscesRenderer.finalize()](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/pisces/PiscesRenderer.java#L439) >> and >> [AbstractSurface.finalize()](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/pisces/AbstractSurface.java#L100) >> method should be removed. >> >> The change is, >> 1. Remove finalize method from PiscesRenderer and AbstractSurface classes. >> 2. Use >> [Disposer](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/prism/impl/Disposer.java#L48) >> class for cleanup instead of finalize(). For SW pipeline the cleanup is >> initiated in >> [here](https://github.com/openjdk/jfx/blob/71ca899fbfc74d825c29b20a778d6d9eb243f90f/modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/UploadingPainter.java#L195) >> in UploadingPainer.java. >> 3. Added new JNI methods to JPiscesRenderer.c and JAbstractSurface.c and >> removed the ones which were used previously with finalize() method. >> >> Verification: >> Verified that the newly added dispose() methods get invoked as required and >> no OOM occurs. >> 1. Create a sample program which adds and removes non 3D shapes and/or >> controls to stage in a loop. >> 2. Run the program using -Dprism.order=sw, -Dprism.verbose=true. (optional >> -Xmx50m) >> Expected Behavior: >> a. Maximum memory consumption after this change should reduce or remain same >> as that of before this change. >> b. No OOM should occur with this change or without this change. > > The pull request has been updated with a new target base due to a merge or a > rebase. The code changes look good to me, with a couple minor points. I tested it and it does what I would expect. modules/javafx.graphics/src/main/java/com/sun/pisces/JavaSurface.java line 46: > 45: initialize(dataType, width, height); > 46: addDisposerRecord(); > 47: } Should this be called from the superclass instead? It works as-is, but if there were ever another subclass added, it would have to be replicated there as well. modules/javafx.graphics/src/main/java/com/sun/pisces/AbstractSurface.java line 2: > 1: /* > 2: * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights > reserved. > 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. Best to revert this or make it 2020. modules/javafx.graphics/src/main/native-prism-sw/JAbstractSurface.c line 189: > 188: static void > 189: disposeNativeImpl(JNIEnv* env, jobject objectHandle) { > 190: AbstractSurface* surface; Since you removed `disposeNativeImpl`, you can also remove the static declaration on [line 40](https://github.com/openjdk/jfx/blob/0591e41f9106e93ae957cb9d80cf2e9e2c6fd805/modules/javafx.graphics/src/main/native-prism-sw/JAbstractSurface.c#L40). ------------- PR: https://git.openjdk.java.net/jfx/pull/66
