-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Package not working with NodeJS 12 #34
Comments
Same issue on Ubuntu 18.04 |
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 😕 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 Edit: I did a mistake here. I was writing about |
Can you outline what that would look like in a |
@brandonros Sorry once more for the late response – just so much to do... I think We could publish a new package for heap snapshotting that uses memwatch is the local node version is < 12 and uses The changes in - 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 |
It hasn’t been updated to support Node 12: andywer/leakage#34
What about to update leakage for nodejs >= 12 ? |
any fix for this? |
Is this package dead or alive? T_T |
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. |
@aide-master/node-memwatch hasn't been updated for 3 years [1], so it might be worthwhile exploring other forks like @airbnb/node-memwatch that have been more recently touched. After blowing away package-lock.json and rebuilding it with node 16.15.0, it appears that the node-gyp issue per [2] is fixed, so this seems sufficient. [1]: https://www.npmjs.com/package/@aidemaster/node-memwatch [2]: andywer#34
@aide-master/node-memwatch hasn't been updated for 3 years [1], so it might be worthwhile exploring other forks like @airbnb/node-memwatch that have been more recently touched. After blowing away package-lock.json and rebuilding it with node 16.15.0, it appears that the node-gyp issue per [2] is fixed, so this seems sufficient. [1]: https://www.npmjs.com/package/@aidemaster/node-memwatch [2]: andywer#34
This could possibly also be due to VS 2019, but I doubt it
The text was updated successfully, but these errors were encountered: