Open wolfy1339 opened 5 years ago
Same issue on Ubuntu 18.04
https://github.com/andywer/leakage/pull/36
Does this fix it?
Still not working with new v0.5.0. At least node 10 works now 🙄
Those native V8 bindings are a horror...
got the same error here using node 12 =(
Can we refactor package to use https://nodejs.org/api/v8.html#v8_v8_getheapstatistics instead of memwatch?
@brandonros That actually makes a lot of sense!
Sorry for the late response. Yeah, that sounds like a good idea, but it will come with some effort, as memwatch also provides the heap snapshot diffing - we would have to re-implement that and make sure to garbage-collect thoroughly after diffing to not pollute the next snapshot.
I suppose that additional full GC will in turn slow down things even more 😕 Might still be worth doing it, though.
Alternative 1: We extract the C++ code for heap diffing into a package of its own and use it to heap diff without touching the heap. Even better: Let's compile it to WASM.
Alternative 2: Let's re-fork memwatch and make it use the node.js getHeapStatistics()
function in newer node versions.
Edit: I did a mistake here. I was writing about v8.getHeapStatistics()
while I actually meant v8.getHeapSnapshot()
.
as memwatch also provides the heap snapshot diffing
Can you outline what that would look like in a require('v8').getHeapStatistics()
world?
@brandonros Sorry once more for the late response – just so much to do...
I think getHeapStatistics()
won't do. What we'd need to use is getHeapSnapshot()
(v8
module in node 11.13+) as it comes with all the details we need to print an error message that gives you enough information to hunt down that memory leak.
We could publish a new package for heap snapshotting that uses memwatch is the local node version is < 12 and uses v8.getHeapSnapshot()
in node 12+. Then we'd still need the heap diffing functionality that memwatch currently provides, but in a way (webassembly?) that it does not touch the heap.
The changes in leakage
would be straight forward then:
- const diffStart = new memwatch.HeapDiff()
- // ...
- const heapdiff = diffStart.end()
+ const snapshot1 = heapSnapshotPackage.createSnapshot()
+ // ...
+ const snapshot2 = heapSnapshotPackage.createSnapshot()
+ const heapdiff = heapDiffer.diff(snapshot1, snapshot2)
I wonder if it's not significantly easier to just fork and patch memwatch once more... Any volunteers? 😅
How many times has memwatch been forked now? I would have thought that the airbnb fork would be better maintained, but I guess not. Their last release was in May 2018.
node officially releasing the importable ‘v8’ library threw things off a bit, I’d imagine
What about to update leakage for nodejs >= 12 ?
any fix for this?
Is this package dead or alive? T_T Or may be there some alternatives?
More dead than alive at the moment, tbh. We would really need a more stable solution under the hood that doesn't break with every other node.js release which is not impossible, but I hardly have the time to re-invent the low-level internals 🙄
Sorry to bump an old issue but does anyone know any alternatives to this since this hasn't been updated in years?
Thanks for bringing this back up, @OmgImAlexis. Unfortunately, I don't have the time to update it / re-implement it for today's node.js, though. So I don't really see a short-term fix here, unless someone feels like making a major contribution or a rewrite.
@andywer I might have a play around over the next few weeks and see if I can get something working. Not promising anything though as this gets down to a lower level than I'm familiar with.
This could possibly also be due to VS 2019, but I doubt it