Track external memory usage for matrices #602
Conversation
Codecov Report
@@ Coverage Diff @@
## master #602 +/- ##
=========================================
Coverage ? 57.23%
=========================================
Files ? 32
Lines ? 4043
Branches ? 30
=========================================
Hits ? 2314
Misses ? 1729
Partials ? 0
Continue to review full report at Codecov.
|
|
After some extensive discussion with @btsimonh (which you can view in #601), I've made some changes to how the memory tracking works. The original version would double count memory when it was referenced from more than one JS object. The latest version now correctly takes into account how many JS objects are referencing an OpenCV object, so that memory is tracked more accurately, particularly when matrices are cloned or cropped. The latest changes also fix some tracking issues that were missed earlier. Note that for the tracking to work correctly, internal code must not independently retain references to Mat memory also referenced by a JS object (our Matrix wrapper class) past the end of an operation. In order to comply with this requirement, I've slightly changed the pattern for implementing asynchronous functions so that they retain our Matrix wrapper classes instead of the raw OpenCV Mat objects. At this point, we are only tracking memory for Matrix objects, so the tracking code has been consolidated into the Matrix implementation in Matrix.cc. |
|
Amazing work, thanks! |
Adds tracking of the memory used by matrices to Node's external memory count, allowing the Javascript garbage collector to make better decisions about when to run.
Without these changes, Node only knows about the memory usage of the Javascript objects, and not the associated C and C++ objects. In the case of Matrix, those associated objects are quite large. As a result, it was possible to quickly allocate a large amount of memory without triggering garbage collection, for example: by reading multiple frames from a camera. This is especially problematic on low-memory systems, such as a Raspberry Pi, where memory usage could quickly grow past the available memory, triggering out-of-memory crashes.
To resolve this, this PR adds external memory adjustment calls to the Matrix wrapper class, and every location where the size of the referenced OpenCV matrix is altered, so that Node has a reasonable estimate of the memory usage of the Matrix objects. This causes garbage collection to run more often when needed, and prevents memory growth far outside of the bounds that Node is configured to run in.
It is still possible to use
Matrix.release()to manually free memory, as this call has also been instrumented to adjust the external memory usage correctly.Note that it is now important that every place that can change the size of the OpenCV Mat object referenced from a Matrix correctly adjusts the external memory usage (using
Nan::AdjustExternalMemory()). If this is not done correctly, Node's external memory count could get out of sync with the library's actual memory usage, which could cause the garbage collector to run too often or not often enough.I have attempted to find all the locations where these memory sizes could change, and have updated them in the least invasive way that I could. There might be better ways to restructure the code (for example, by strengthening the encapsulation of the referenced
Matwithin theMatrixwrapper class) to make some of this cleaner and safer, but that could be significantly more work and would probably change some of the usage patterns for theMatrixclass.In most cases, I was able to replace the code that creates a Javascript wrapped Matrix with a call to a helper method that does the same thing and correctly tracks the external memory size of the new object. A new destructor was also added to correctly reduce the tracked external memory size when the Matrix object is released.
I believe that this PR fixes #601, #411, #209, #167, and #101.