Skip to content
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

Add GC Insights #4496

Closed
bripkens opened this issue Dec 31, 2015 · 10 comments
Closed

Add GC Insights #4496

bripkens opened this issue Dec 31, 2015 · 10 comments
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.

Comments

@bripkens
Copy link
Contributor

As more and more enterprise customers are using Node.js, the demand for Node.js monitoring increased as well. Node.js already supports a bunch of ways for retrieving interesting information, but GC information is lacking. Many platforms, e.g. the JVM, support means to programmatically retrieve information about GCs. It would be useful if Node.js had these features too.

The main users for GC insights will probably be monitoring products. The way these products currently access GC information is via native addons that make use of AddGCEpilogueCallback. An example open source implementation of this is node-gcstats. Strongloop, Ruxit, Instana and others have their own, probably similar implementations.

Users of such monitoring tools, or more generally users interested in GC activity, need to compile native addons in order to access this information. It is hard to explain why users need to do this (or have a package such as build-essential installed). This gets increasingly harder in Java shops which are used to having everything in the platform. Building these addons automatically on install only reduces this problem, but doesn't remove it.

Node.js used to contain post-gc event infrastructure, but it was removed at the end of last year (Dec 2014) by @bnoordhuis with the following commit:

lib,src: remove post-gc event infrastructure
Remove the 'gc' event from the v8 module and remove the supporting
infrastructure from src/. It gets the axe because:

  1. There are currently no users. It was originally conceived as
    an upstreamed subset of StrongLoop's strong-agent GC metrics,
    but the strong-agent code base has evolved considerably since
    that time and has no use anymore for what is in core.
  2. The implementation is not quite sound. It calls into JS land
    from inside the GC epilog and that is unsafe. We could fix
    that by delaying the callback until a safe time but because
    there are no users anyway, removing it is all around easier.

PR-URL: #174
Reviewed-By: Trevor Norris trev.norris@gmail.com

I believe that we got users of such functionality (mainly through monitoring products) and that we could fix the implementation (the second argument in the commit comment). For these reasons, I propose to add the removed functionality back to the v8 module and improve on it, e.g. by scheduling work via uv_queue_work. I am willing to do this work and open a PR.

Disclaimer: I am working for Instana and we have a Node.js sensor with GC information.

@mscdex mscdex added v8 engine Issues and PRs related to the V8 dependency. feature request Issues that request new features to be added to Node.js. labels Dec 31, 2015
@trevnorris
Copy link
Contributor

are there specific data points you'd like to retrieve from the GC, or simply be able to call a callback before/after the GC? also, I believe this API only addresses the GC calls that interrupt the main thread, and not for those collections happening off the main thread.

@bripkens
Copy link
Contributor Author

bripkens commented Jan 1, 2016

@trevnorris: are there specific data points you'd like to retrieve from the GC, or simply be able to call a callback before/after the GC?

I'd opt for a minimum set of metrics in the beginning.

  • When did a GC occur (possibly via callback, as done before),
  • how long did the garbage collection take (endTime - startTime) and
  • the type of GC.

Further information, e.g. heap statistics, could be retrieved in user-land via v8.getHeapStatistics() in the callback. In combination with PR#4463 (v8.getHeapSpaceStatistics()), we can get pretty close to node --log_gc --trace_gc. On the other hand, we won't get information about freed handles, marking steps, holes… My current assumption is that most users do not need to know about handles, marking steps, fragmentation or any more of the low-level metrics. For this reason, I believe that the metrics listed above (when did what type of GC occur and how long did it take) are sufficient for most use cases.

@trevnorris: also, I believe this API only addresses the GC calls that interrupt the main thread, and not for those collections happening off the main thread.

I traced the usage of the v8 GCTracer and specifically when GCTracer#Stop(…) (source) is called. As far as I know, this is the class responsible for printing information to stdout when v8 is executed with GC tracing enabled, e.g. --trace_gc. Turns out GCTracer#Stop(…) is only used in Heap#CollectGarbage() (source, call location). In the same method we also have a call path to PerformGarbageCollection (source) and from that method to CallGCPrologueCallbacks (source) and CallGCEpilogueCallbacks (source).

What I want to say: The GC prologue and epilogue callbacks are called whenever the GC Tracer runs. As such it should be sufficient for our use case as well.

@trevnorris
Copy link
Contributor

@nodejs/ctc Thoughts?

@ofrobots
Copy link
Contributor

ofrobots commented Jan 2, 2016

One thing to note is that this use-case has some overlap with tracing and trace-event (some background). Now that blink's trace-event has landed in V8 4.9, once we implement a trace-event buffer in Node.js, we should be able to expose the trace stream to user space which could include these gc trace events.

@bripkens
Copy link
Contributor Author

bripkens commented Jan 3, 2016

One thing to note is that this use-case has some overlap with tracing and trace-event

That is some very big overlap. I like where the trace-event discussions are going. If trace-event works out and is controllable via Node.js, my proposal would be completely obsolete. Still got some questions in my head and will post them to the WG.

I'll update this issue once these questions are resolved. Thank you @ofrobots.

@Fishrock123
Copy link
Contributor

@ofrobots could you explain what that does in some more detail?

In general I think this sounds useful. (Although I don't know this area in any great detail.)

@megastef
Copy link

+1 - today monitoring GC activites needs modules like memwatch-next, gc-profiler or gc-stats. I hear sometimes complains from windows users having problems to compile those native modules ...it would be great to have this available in node!

@megastef
Copy link

Hi, any progress for this issue? It looks windows support here dainis/node-gcstats#18 stucks as well. I just want to know if it makes sense to invest more time in gc-stats windows support/providing pre-compiled modules for gc-stats.

@Fishrock123
Copy link
Contributor

Nothing of note so far.

@bnoordhuis
Copy link
Member

I think this has been superseded by nodejs/diagnostics#84. I'll go ahead and close this out but let me know if I should reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

7 participants