Fix JVM memory leaks #1526
Fix JVM memory leaks #1526
Conversation
| for (classIndex in 0 until count) { | ||
| val classInfo = realmc.classArray_getitem(cclasses, classIndex) | ||
|
|
||
| // Clean properties | ||
| val propertyArray = realmc.propertyArrayArray_getitem(cproperties, classIndex) | ||
|
|
||
| val propertyCount = classInfo.getNum_properties() + classInfo.getNum_computed_properties() | ||
| for (propertyIndex in 0 until propertyCount) { | ||
| val property = realmc.propertyArray_getitem(propertyArray, propertyIndex.toInt()) | ||
|
|
||
| realmc.realm_property_info_t_cleanup(property) | ||
| property.delete() | ||
| } | ||
|
|
||
| realmc.delete_propertyArray(propertyArray) | ||
|
|
||
| // end clean properties | ||
|
|
||
| realmc.realm_class_info_t_cleanup(classInfo) | ||
| classInfo.delete() | ||
| } | ||
|
|
||
| realmc.delete_propertyArrayArray(cproperties) | ||
| realmc.delete_classArray(cclasses) |
There was a problem hiding this comment.
Not sure I get why we need to do cleanup this way? Also, this is quite a lot of JNI traversals to do cleanup. Couldn't we move this entire block into a helper function in realm_api_helpers?
There was a problem hiding this comment.
Also, doesn't realmc.classArray_getitem cause a new allocation rather than returning the old one?
There was a problem hiding this comment.
But now that you mention, in realm_get_class we could free the allocated struct afterwards.
There was a problem hiding this comment.
This is still doing a ton of JNI traversals. My comment was about us moving the entire for-loop into C++. I assume that is possible?
There was a problem hiding this comment.
Yes, it is possible. I wanted to use, if possible, swig APIs, and then in a separate PR move this logic to c++ to measure the actual performance gain from doing it.
| ) | ||
| ) | ||
| } finally { | ||
| realmc.delete_queryArgArray(args.head) |
There was a problem hiding this comment.
I don't think we release everything here. The query argument array holds a pointer to a value array allocated in JvmMemAllocator.allocRealmValueList. Maybe not showing up in the benchmarks if you are not testing with list query-arguments (query("x in $0", [1, 2, 3])
This PR fixes some native memory leaks caused when handling core: