search

mikeseven / node-opencl

Low-level OpenCL 1.x and 2.x bindgings for node.js
156 stars 33 forks source link

node-cl leaks memory #31

Closed andreasgal closed 8 years ago

andreasgal commented 8 years ago

NOCL_REALEASE_DRIVER_ISSUES is set by default, bypassing all deallocations

mikeseven commented 8 years ago

yes, automatic deallocation is off by default because on some machines this was creating crashes when we did node-webcl, mainly due to some order of executions of CL commands and incorrect implementations of reference counting in drivers. In rare case, there were also issues with v8 garbage collector.

Instead, we decided to be as close as possible to OpenCL and thus "to force" the developer to cleanup what he created. So it's only leaking if developer makes bad code.

Currently process.exit() calls releaseAll() but it does nothing. To avoid sync and ref counting issues we could implement releaseAll() at JS level in lib/opencl.js by tracking all CL Objects allocations and calling clobject.release() of appropriate clobjects.

Would this be useful?

andreasgal commented 8 years ago

Right now its not possible at all to do any memory management at runtime, so I think my suggestion would be to map release/acquire directly 1:1 to the CL primitives and do any hacking on top of that in JS.

There is also another bug around reference counting. init() acquires another reference count, which leaks every object since a ref count of 1 is implied in allocations.

I will try to fix the two issues above and see if that works with my test setup. If so we could "disable" memory management as with NOCL_REALEASE_DRIVER_ISSUES by simply patching release() and putting all objects in a list and de-allocate them in shutdown.

Wdyt?

andreasgal commented 8 years ago

https://github.com/mikeseven/node-opencl/pull/32

andreasgal commented 8 years ago

NOCL_WRAP_AND_RELEASE is used for all allocation sites and should ensure that release() actually frees the object. I will do some testing in the morning. Let me know whether you think this approach makes sense! Thanks.

mikeseven commented 8 years ago

I think we might be close to closing down on these CL objects leaks. I added and tested various ways CL objects could deleted, incl. GC conflicts, and exit cleanup (the worst case by far for creeping GPU memory). With your fixes and mine, it seems to be clean and stable ;-)

Wdyt?

andreasgal commented 8 years ago

I can't reproduce any more leaks atm. I'll reopen if needed.