-
-
Notifications
You must be signed in to change notification settings - Fork 56.2k
Java Finalize Class is a removal candidate in JDK 18 #22260
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Comments
As mentioned above, code with the In addition to the language evolution problem, I also noted that intense Mat object creation leads to problems because memory release is done by the In addition to the discussions #19450 #8683, I think that there are solutions but it is especially the transition that is more difficult to manage. @sgjava's solution does not solve the root problem. There are 2 types of solution for removing
I made some modifications to implement the 2nd solution because the 1st one doesn't seem acceptable. The idea is to generate Java 8-compatible classes and Java 9-compatible classes with the Cleaner class. The build of my branch produces 2 jars:
If you want a package exclusively compatible with Java 11, you can take the sources and copy the java9 directory to the root folder which will overwrite the classes containing the If you think this is a solution you can merge my commit. There are certainly still some modifications to make it work in all contexts. In particular, I haven't tested with Android (with Java 6 Ant build). |
@nroduit My solution was not to use finalize period five years ago since it produces unpredictable code like calling GC does. I provided a work around hack. However, you are going down the wrong path trying to simulate finalize. Just be responsible and free native memory you allocate. Keep release and expose free. Or just leave it alone and let it break in JDK 17+ releases (ignoring seems to be the solution thus far). My initial issue with this was never taken seriously, so I gave up. Any solution shouldn't be JDK dependent. This is also the wrong path. I do use try with resources (AutoCloseable) to free native memory and close native file handles, etc. in my JNI code https://github.com/sgjava/javauio/blob/main/demo/src/main/java/com/codeferm/periphery/demo/SpiLoopback.java#L46 But this is the calling application, not the API. Again it's your responsibility to free resources. Not some lazy closing/freeing mechanism. If you want to do that than use a pool (oh yeah, you need to return the object to the pool or it leaks as well). |
@sgjava I clearly disagree with your statement that it is up to the developer to free up memory in Java code. The principles of memory management in Java are not those of the C language. When a Java object is dereferenced it is expected that the associated resources are completely freed otherwise it is a memory leak. The above proposition is not only for the Mat object which consumes a lot of memory but is generalized to all native objects. For Mat objects, it is always possible to release the memory in some situations before dereferencing the object with the release() method. If you look in the OpenJDK project, you can see that particular objects that use native memory like DirectByteBuffer or network resources (see PKCS11) implement the Cleaner class method. |
You are conflating JVM and native memory management principles and swapping one bad idea for another. See https://kylec32.medium.com/effective-java-avoid-finalizers-and-cleaners-7906bbd1cbb1 and my original post https://github.com/sgjava/opencvmem (It covered other classes besides Mat) Even https://dzone.com/articles/native-memory-allocation-in-examples explicitly calls Unsafe.freeMemory and doesn't rely on some lazy cleanup. So you can disagree, but you have not proved my view is incorrect in an empirical way. It is the developers job to manage native memory (frankly all resources) and not use lame cleaners, finalizers, weak references. These are all anti-patterns for lazy developers. Let's look at it a different way. Memory is just another resource like connections, file handles, etc. So when a JDBC connection is dereferenced is it automatically closed? Of course not, it leaks. But for lazy developers there's https://dzone.com/articles/detecting-and-resolving-database-connection-leaks which is basically what you are proposing for memory management. The bottom line is nothing was done for 5 years. I can appreciate you suggesting something different albeit another bad solution. If the PR is accepted then I'll add this to my https://github.com/sgjava/opencvmem project as the latest anti pattern. Frankly my solution now is just don't create unnecessary Mats and use release (delete(nativeObj) just frees the pointer), so the leakage is minimal). Some of the classes still leak native memory though. Even your solution will need to be tested with Valgrind or other native memory analyzer which I suspect hasn't been done. |
Sorry, but I can't come to a conclusion between lazy developers who should call I would prefer to keep the discussion about concrete changes in OpenCV in order to solve the problem that has been clearly expressed. You could provide your expertise to analyze the native memory behavior with the proposed changes using the different JVM 8, 11 and 17. |
I'll meet you half way. I made it so I could call free, but still use lazy finalize if memory was already deallocated. Just set the memory pointer to nil $00 and test for that condition. Treating native memory like a Java object comes with consequences. My point is very simple. Programmers are required to deallocate, close. release etal resources all the time. This is no different. |
As the goal is to find a solution that meets all usage contexts, here are the benefits of using the Cleaner method:
Another element that leads to performance problems is the use of put() and get() on the Mat object because the JNI layer creates intermediate arrays. In order to have a more direct native memory management, it would be necessary to evolve towards another wrapping, see #22776. |
@nroduit I'm totally cool with allowing lazy cleanup if I have a way to take care of it myself real time. Lazy code should handle that automatically like I did. I've used https://github.com/fusesource/hawtjni to generate JNI code, but Panama may be a better alternative once it matures. HawtJNI already handles C++ http://fusesource.github.io/hawtjni/documentation/developer-guide.html#Binding_to_C___Classes, but I understand the desire to move forward. You should probably have a wrapper abstraction, so you could generate different implementations using templates. Then there could be an official one in Panama, but I could generate HawtJNI bindings if I wanted. Just keep the OpenCV Java API the same. If you are going to totally redo it, it might as well be improved architecturally. |
Disappointing to see that this is still an open issue. If OpenCV wants to provide Java bindings then they should conform to how things are done in Java, independently of the opinion of the OpenCV contributors about its memory model, or the laziness of Java developers... There are mechanisms in Java to deal with this type of thing, we should use it. As the saying goes, when in Rome... |
That wasn't my opinion, it was a fact based on https://github.com/sgjava/opencvmem. You should see the irony in my laziness comment as the whole idea of finalize is lazy clean up and using it has been considered an antipattern for a long time. I came up with a workaround years ago since there was resistance to changing this. I just work around difficult developers. I don't have time for such a foolish debate. |
@davidreis97 you can test this PR. We've been using it for a while in production without any problems. If you're really using memory intensively, you can always release it manually by calling the release() function. |
@nroduit this doesn't fix all the other classes that leak (from my article I posted above): ./org/opencv/video/BackgroundSubtractorMOG2.java
./org/opencv/video/BackgroundSubtractor.java
./org/opencv/video/DenseOpticalFlow.java
./org/opencv/video/BackgroundSubtractorKNN.java
./org/opencv/video/DualTVL1OpticalFlow.java
./org/opencv/core/Algorithm.java
./org/opencv/calib3d/StereoMatcher.java
./org/opencv/calib3d/StereoBM.java
./org/opencv/calib3d/StereoSGBM.java
./org/opencv/features2d/FeatureDetector.java
./org/opencv/features2d/DescriptorMatcher.java
./org/opencv/features2d/DescriptorExtractor.java
./org/opencv/videoio/VideoCapture.java
./org/opencv/videoio/VideoWriter.java
./org/opencv/ml/StatModel.java
./org/opencv/ml/LogisticRegression.java
./org/opencv/ml/Boost.java
./org/opencv/ml/ANN_MLP.java
./org/opencv/ml/RTrees.java
./org/opencv/ml/DTrees.java
./org/opencv/ml/TrainData.java
./org/opencv/ml/SVM.java
./org/opencv/ml/KNearest.java
./org/opencv/ml/EM.java
./org/opencv/ml/NormalBayesClassifier.java
./org/opencv/imgproc/Subdiv2D.java
./org/opencv/imgproc/CLAHE.java
./org/opencv/imgproc/LineSegmentDetector.java
./org/opencv/objdetect/CascadeClassifier.java
./org/opencv/objdetect/HOGDescriptor.java
./org/opencv/objdetect/BaseCascadeClassifier.java
./org/opencv/photo/TonemapDrago.java
./org/opencv/photo/CalibrateRobertson.java
./org/opencv/photo/CalibrateCRF.java
./org/opencv/photo/CalibrateDebevec.java
./org/opencv/photo/Tonemap.java
./org/opencv/photo/AlignMTB.java
./org/opencv/photo/MergeDebevec.java
./org/opencv/photo/MergeExposures.java
./org/opencv/photo/MergeMertens.java
./org/opencv/photo/MergeRobertson.java
./org/opencv/photo/TonemapReinhard.java
./org/opencv/photo/TonemapDurand.java
./org/opencv/photo/TonemapMantiuk.java
./org/opencv/photo/AlignExposures.java |
@sgjava Look carefully, all these classes register a Cleaner. |
PhantomReference Why It Can Be Considered an Antipattern:
Better Alternatives Exist:
Conclusion: Using PhantomReference isn’t necessarily an antipattern, but if you're using it for lazy cleanup instead of explicit resource management, you're likely misusing it. If possible, prefer deterministic cleanup methods (AutoCloseable, explicit close() calls) over relying on the garbage collector. Anyways, basically a better Finalize at best? |
PhantomReference is not meant to replace AutoCloseable and try-with-resources, it’s designed to handle non-deterministic cleanup tasks when the exact timing of resource release is less critical or not feasible through explicit methods. However, when used appropriately, PhantomReference can still be a powerful tool for scenarios requiring post-GC actions that don’t interfere with normal object lifecycles.
|
@nroduit I believe you are missing the point. I'm simply pointing out that lazy cleanup especially of potentially large native objects is an antipattern. It doesn't really matter how you implement it. I already came up with a solution a long time ago. https://github.com/sgjava/opencvmem?tab=readme-ov-file#fix-mat The design of the Java wrapper was ill conceived from the get go. Everything else has just been "cleaner" hacks and do not address the root cause of poor design. I can understand what you are doing, but it's just another hack. |
@sgjava I understand that you have a different point of view. I have already replied to it with #22260 (comment). If you believe your approach is more effective, I encourage you to submit a pull request reflecting your method or suggest constructive improvements to the current approach. Collaborative progress depends on solution-oriented dialogue. |
@nroduit I've done multiple PRs and they were rejected because the author thought Finalizers were the way to go. I've already pointed that out multiple times that lazy cleanup is an antipattern and proven it with profilers. I've been constructive for years and frankly it goes nowhere. Talk is cheap. If you think the cleaner is the way to go more power to you. The easiest way is to just work with the antipatterns https://github.com/sgjava/opencvmem?tab=readme-ov-file#tricks-of-the-trade. Frank and forthright talk is constructive. It may not be what you want to hear, but reality sometimes isn't pleasant. Anyways, at this point I find this very unproductive. Basically the same thought pattern from 7 years ago. Nothing has changed. |
Uh oh!
There was an error while loading. Please reload this page.
Detailed description
Java Finalize Class is a removal candidate in JDK 18 (link) and when I compiling OpenCV there are a lot of warnings about it.
Issue submission checklist
forum.opencv.org, Stack Overflow, etc and have not found any solution
The text was updated successfully, but these errors were encountered: