Skip to content

Memory tracking in node.js core with hints to VMs #155

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

Closed
joyeecheung opened this issue Feb 13, 2018 · 9 comments
Closed

Memory tracking in node.js core with hints to VMs #155

joyeecheung opened this issue Feb 13, 2018 · 9 comments
Labels
never stale stale triaging-requested Issues that need extra attention before deciding if they should be closed or merged together

Comments

@joyeecheung
Copy link
Member

We have talked about adding a memory tracking machenism to Node.js core in order to make it easier to debug native-layer leaks.

A typical senario

A developer notices that the RSS usage of their application is high, but the heap usage is normal. The process may or may not goes out of memory eventually. When dumping a heap snapshot (which contains proper size information about Buffers and wraps), there is no outstanding type of objects that indicate what the high RSS usage is coming from, which means that the leak comes from C++ (either Node.js core or addons) but it's not directly caused by buffers or wraps, potentially caused by other types of heap allocations from the C++ side managed inproperly or not interacting well with the GC.

We get these kind of reports in the Node.js core issue tracker from time to time, people come with different types of applications and usually all they can provide are some memory usage graphs, so it is not exactly clear if those issues are actually fixed for all of them even when the issue gets closed.

Example: nodejs/node#12805, nodejs/node#7109, nodejs/help#947

I also get this kind of bugs reported from time to time by teams inside Alibaba but oftentimes before we even pinpoint where the bug is, upgrading Node.js fixes those problems even though we don't actually know what patch fixes that due to difficulties to git bisect a production app.

Proposal

The VM vendors provide a (maybe common) API that Node.js core can use to provide hints about the memory allocation in core, so that they can be reported in memory-related profiles, traces or reports (similar to what node-report generates) to facilitate debugging this type of issues, making it easier to track down the cause. Node.js core might also use a custom allocator that somehow "tags" the C++ heap allocations and find a way to report them to diagnostic tools.

We might also want to provide hints about Node.js's heap allocations to the VMs so they could tune the GC to work better with Node.js. @hashseed talked about some a separate GC implemented by Chromium that talked to the V8 GC to coordinate the memory management (maybe this?)

Some related work

  • RetainedAsyncInfo: what we use to track the size of wraps so they are reported in the heap snapshot
  • ArrayBufferAllocator: what we use to interact with V8 so it knows about the size of the buffers (also reported properly in heap snapshots)
  • AliasedBuffer: not exactly related to memory management, but is how node-chakracore observes the state change that's not obvious without knowledge in Node.js core
@joyeecheung
Copy link
Member Author

Belated update: RetainedObjectInfo is superseded by EmbedderGraph which is capable of adding nodes and edges to the heap snapshot. There is an issue in chromium's issue tracker about hooking the new API into Blink's Olipan GC: https://bugs.chromium.org/p/chromium/issues/detail?id=854094

@addaleax
Copy link
Member

addaleax commented Jul 3, 2018

@joyeecheung Have you seen nodejs/node#21633? I think the work linked there might be interesting. My WIP at https://github.com/addaleax/node/tree/heap is only more accurate on the reporting side, though, not on the GC side.

@joyeecheung
Copy link
Member Author

@addaleax Not yet, thanks for the pointer!

@gireeshpunathil
Copy link
Member

should this remain open? [ I am trying to chase dormant issues to closure ]

@github-actions
Copy link

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Jul 18, 2020
@mmarchini mmarchini added never stale triaging-requested Issues that need extra attention before deciding if they should be closed or merged together and removed stale labels Jul 21, 2020
@mmarchini
Copy link
Contributor

Not entirely sure, but I think this is similar to tracepoints: we would need to list which subsystems would benefit from memory tracking and which ones have this today.

@joyeecheung @addaleax does that make sense (creating a list of subsystems/classes which would benefit from MemoryRetainer but are not using it today)?

@joyeecheung
Copy link
Member Author

@mmarchini That makes sense to me, though I think the list is quite dynamic and probably applies to..all subsystems?

@mmarchini
Copy link
Contributor

You're right, there's a good chance the list is dynamic and it applies to all subsystems. Might be worth doing some deep dives into it to understand if any subsystem is missing it and maybe come up with a strategy to ensure it is properly used in the future for new subsystems or features.

@github-actions
Copy link

github-actions bot commented Aug 5, 2022

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
never stale stale triaging-requested Issues that need extra attention before deciding if they should be closed or merged together
Projects
None yet
Development

No branches or pull requests

4 participants