rainyl / opencv_dart

OpenCV bindings for Dart language and Flutter.
https://pub.dev/packages/opencv_dart
Apache License 2.0
78 stars 10 forks source link

isolates memory not being auto freed #97

Closed abdelaziz-mahdy closed 1 week ago

abdelaziz-mahdy commented 2 weeks ago

Describe the bug when running opencv tasks inside isolates example https://github.com/abdelaziz-mahdy/face_grouping/blob/ac18fed722f7ec773632acc7ca613c91ff4afb08/lib/image_service.dart the memory will not be auto freed by finalizer i think this issue is related https://github.com/dart-lang/sdk/issues/55062

https://github.com/rainyl/opencv_dart/issues/64#issuecomment-2163960631 is the same problem

i think for my case i will need to dispose before finishing the tasks inside isolate , so i will keep this for tracking

edit: even force dispose doesnt free memory https://github.com/abdelaziz-mahdy/face_grouping/commit/0e757770a773f31bd76f25e452d8fc5aa882e8be i dont think there is any missing dispose

actions to redo the memory leak, proccesss dir multiple times

rainyl commented 2 weeks ago

Well, it seems the free of smart pointer is wrong, I will fix it soon.

rainyl commented 2 weeks ago

@abdelaziz-mahdy Please update to v1.0.9 and test whether it is better, leave more messages if the problem still exists.

abdelaziz-mahdy commented 2 weeks ago

thank you very much, i can no longer see a memory leak, even without forcing the dispose, so auto dispose is working

i think having async support built in will be awesome but i dont know if native threads or isolates is better

rainyl commented 2 weeks ago

i think having async support built in will be awesome but i dont know if native threads or isolates is better

Yes, but it's not easy, there are much works todo, for dart isolates, the most difficult one is that native resources can't be passed, BUT as you mentioned, they are trying to implement pragma('vm:shared') and I do see that jni has implemented the sharable JObject, but I have no time to explore more.

As for native ways, we need to refactor all APIs to add callbacks, it's crazy and I do not want to refactor them again...

So maybe with "sendable native resources" supported it will be easier, but I do not think it's easy to implement, because native resources created in main isolate will needs to be copied to helper isolate.

If you have better solutions please tell me or try to implement it, I do think we should support async so I have added it to TODO in readme, but it just not easy...

abdelaziz-mahdy commented 2 weeks ago

I think we can take https://pub.dev/packages/image as a reference since he used a different approach, if that doesn't work for our case then I guess people will need to implement their own background handlers same as I did then

Also if you want to add my app to your readme as examples I don't mind, I am going to test tflite but will try to keep code as modular as I can Incase people want to use it

rainyl commented 2 weeks ago

You mean the commands? Yes it's another way to implement async, but adding every API to command is laborious and not efficient because many memory copies will be needed, maybe with GAPI implemented this way will be the best option for now.

Also if you want to add my app to your readme as examples I don't mind,

Thanks~ It will be helpful for other developers. I will update the readme to link to your repo.

I am going to test tflite but will try to keep code as modular as I can Incase people want to use it

I remember that opencv 4.9.0 just support a few TFLite operators, I tried many tflite models but opencv can't even load them, that's why I removed TFLite, but in opencv 4.10.0 it seems the support of TFLite is much better, so you can try it now, thanks for your efforts in advance~

abdelaziz-mahdy commented 2 weeks ago

You mean the commands? Yes it's another way to implement async, but adding every API to command is laborious and not efficient because many memory copies will be needed, maybe with GAPI implemented this way will be the best option for now.

Also if you want to add my app to your readme as examples I don't mind,

Thanks~ It will be helpful for other developers. I will update the readme to link to your repo.

I am going to test tflite but will try to keep code as modular as I can Incase people want to use it

I remember that opencv 4.9.0 just support a few TFLite operators, I tried many tflite models but opencv can't even load them, that's why I removed TFLite, but in opencv 4.10.0 it seems the support of TFLite is much better, so you can try it now, thanks for your efforts in advance~

For async tasks, if I found an easy workaround will let you know

My point of trying tflite to make use of apple chip GPU, since from what I am seeing the detection of faces is slow so it may not being fully utilizing GPU power, detection of 60 images using 5 isolates finishes in 10 sec so I don't think it's using gpu

rainyl commented 2 weeks ago

Maybe you can try to use OPENCL, although it's also slow on my AMD chip and shows some warnings, but it does use GPU, maybe some extra configurations needed.

final faceDetector = cv.FaceDetectorYN.fromBuffer(
      "onnx",
      buf,
      Uint8List(0),
      (320, 320),
      backendId: cv.DNN_BACKEND_OPENCV,
      targetId: cv.DNN_TARGET_OPENCL,
    );
rainyl commented 1 week ago

@abdelaziz-mahdy maybe we should close this issue now?