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

N-API extensions for debugging/monitoring #296

Closed
Flarna opened this issue Mar 8, 2018 · 9 comments
Closed

N-API extensions for debugging/monitoring #296

Flarna opened this issue Mar 8, 2018 · 9 comments

Comments

@Flarna
Copy link
Member

Flarna commented Mar 8, 2018

Is it planned/possible to extend N-API with debugging/monitoring APIs of the underlying VM?

I have following APIs in mind:

  • v8::CpuProfiler for in process CPU profiling
  • v8::Isolate::NumberOfHeapSpaces(), v8::Isolate::GetHeapSpaceStatistics, v8::Isolate::AddGCPrologueCallback() and v8::Isolate::AddGCEpilogueCallback() for in process GC/Heap statistics collection
  • v8::HeapProfiler for creation of Heap dumps

List above may be incomplete but I think the main question is if N-API should be limited of "real production use" or include also debugging/monitoring APIs most likely only of value for few modules.

@Qard
Copy link
Member

Qard commented Mar 8, 2018

All those things are very specific to the internals of V8, so they probably shouldn't be part of n-api. It's best to just use platform-specific code if you want to use platform-specific features.

@Flarna
Copy link
Member Author

Flarna commented Mar 8, 2018

As far as I know one target of N-API is to get rid of the requirement to have a stable V8 ABI within a major Node.JS version. This would allow updating V8 more frequently for easier bug fixing and to get new features/improvements.

Assuming that such V8 updates will be done at some time in the future. This would require to re-compile all addons where you can't be sure they use only N-API - even for Node.JS patch releases. Not sure if this is a good end user experience (in special for Windows where native addons are quite a pain...).

Maybe another option instead of extending N-API with VM specific functionality would be to map these API to JS via some built in node modules specific to the VM. This would allow the use of these features without native calls and only API changes are relevant for node core during updating VM.
Or maybe have a set of VM specific N-APIs which just take care about ABI/API compatibility within a VM?

@mhdawson
Copy link
Member

mhdawson commented Mar 8, 2018

We discussed briefly in the N-API call today and the consensus is that we should ask the Diagnostics WG to comment on what's appropriate/needed on this front. @nodejs/diagnostics

@mmarchini
Copy link

After playing with v8::CpuProfiler for a while, I agree we should have an API into core for this, and probably for taking snapshots and GC metrics as well. Not sure about v8::Isolate::NumberOfHeapSpaces() and v8::Isolate::GetHeapSpaceStatistics, since those are strictly related to how V8 organizes the heap. Maybe we should even have a VM-independent format for each of these functions.

If this should be an in N-API or a JavaScript module I'm still not sure, although I am more inclined to the latter, so that end users (i.e., JavaScript non-module developers) can access these functions easily and without having to install separate dependencies.

Maybe we should discuss this in the Diagnostics repository first (or add this issue to the diag. agenda)? It seems like a continuation of our discussions about VM independency and having diag. tools shipped with Node.js.

@bnoordhuis
Copy link
Member

Keep in mind though that all of the profiling tools changed considerably over V8's lifetime and quite possibly will again.

Papering over that is hard enough (I did that for StrongLoop, huge time sink) but doing it in a VM-neutral way is extra challenging.

@Flarna
Copy link
Member Author

Flarna commented Apr 12, 2018

At least for me the main target is not full platform neutrality.
Sure it would be nice to get this but in the end every VM will have different features in detail (e.g. TimeTravel in Chakra,...) which would be most likely not accessible if VM independent API is needed.
Besides that I assume new VM specific debugging/monitoring features would be available with significant delay.

Different VMs will offer different metrics and I it's correct and expected to report only those available (not just the common ones). Also Java/Ruby VMs differ in such details therefore this would be nothing NodeJs specific.

I'm quite sure that we will have always differences between VMs besides this topic here:

  • timeline when ESx features are implement
  • naming of anonymous functions
  • performance in various scenarios
  • ...

@mmarchini
Copy link

Keep in mind though that all of the profiling tools changed considerably over V8's lifetime and quite possibly will again.

Yes, I'm aware of that. But I believe we can provide a minimal set of metrics which could be VM-independent (with the help of VM vendors). Not sure how we could do that, but it's something that came up during the Diagnostics Summit and I believe is worth discussing. I'll open an issue in the diagnostics repo to keep the conversation going.

As far as I know one target of N-API is to get rid of the requirement to have a stable V8 ABI within a major Node.JS version

From the README: N-API aims to provide ABI compatibility guarantees across different Node versions and also across different Node VMs - allowing N-API enabled native modules to just work across different versions and flavors of Node.js without recompilations

We discussed briefly in the N-API call today and the consensus is that we should ask the Diagnostics WG to comment on what's appropriate/needed on this front. @nodejs/diagnostics

I'm -1 in introducing V8-specific functions to N-API because this goes against the goal of the API. -0 about introducing this in another part of core though.

@mhdawson
Copy link
Member

I agree we need to avoid any V8-specifics to N-API. I think best to let the discussion take place in the diagnostics repo and then this issue can be updated with any decisions/recommendations from the diagnostics team. Even in that case it will likely be people from the diagnostics team that would need to implement them.

@mhdawson
Copy link
Member

There have been no additional comments since the last repose a number of years ago, closing. Please let us know if you think that was not the right thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants