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-APIs to get information for JS function objects #295

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

N-APIs to get information for JS function objects #295

Flarna opened this issue Mar 8, 2018 · 6 comments

Comments

@Flarna
Copy link
Member

Flarna commented Mar 8, 2018

In our module we use following v8 Apis to get more details of function objects:

v8::Function::GetName()
v8::Function::GetInferredName()
v8::Function::GetBoundFunction()
v8::Function::GetScriptLineNumber()
v8::Function::GetScriptOrigin()
v8::ScriptOrigin::ResourceName()

Not 100% sure but maybe v8::Function::GetName() is available via Function#name in JS but the others are not available to my knowledge.

We use/need this info as this is actually what v8 uses internally during creation of callstacks and we need a mechanism to actually map callstacks created at one place to real functions.

Is is planned/possible to extend NAPI to offer above APIs?

@Flarna
Copy link
Member Author

Flarna commented Mar 21, 2018

No opinion from anyone?
Does it make sense to simply create a PR for NodeJS in this?
I'm not sure about how NAPI is extended usually.

@MSLaguana
Copy link

MSLaguana commented Mar 21, 2018

It sounds like this is inherently v8 functionality (as opposed to general JS functionality) that you want here, which I don't think is a good fit for N-API. What is it that you want to do with this information? It sounds like you are trying to map from text obtained from a stack trace back to the actual function object?

@Flarna
Copy link
Member Author

Flarna commented Mar 21, 2018

Agree that this is not general JS but the functionality itself is most likely in any JS-VM. I did a fast check with Node Chakra and found that call stacks there use also inferred names for anonymous functions - but different ones.

I try to provide some more info on what we do now via V8 APIs; hope it becomes more clear.
We use this in an APM tool. One part of the functionality is to record transactions (e.g. a mongoDb.find). Besides capturing start/end and some DB related data we also capture name/file of triggering function and corresponding callback. This is done by monkey patching/wrapping these functions and use of above v8 APIs.
Additionally we capture call stacks every now and then on such transaction to get more info on which code parts are involved. CallStacks provide inferred function name, file directly.
And on top of it CpuProfiler is used to give even more insight what is going on in the application without correlation to single transaction (see #296).
Again function/file name are already provided here.

Maybe I have a misunderstanding on what NAPI should provide:

  1. API to call from JS to native for not VM related native functionality (e.g. existing parsers/libs/...)
  2. Abstraction/Wrapper of VM functionality not available from JS land to get rid of VM ABI stability requirements

(1) is clear but not sure about (2).

As long as it is ensured that VM ABIs are stable across at least one major NodeJS version I'm fine with using these APIs directly. But once ABI may change within a major node version it would imply to either recompile always or provide binaries for every patch version which would be a pain.

Is it planned to keep VM ABI stability even N-API gets out of experimental?

@mhdawson
Copy link
Member

I believe the VM ABI should be stable within a major release (ie there are no changes planned on this front due to N-API).

I do think that we want commonly used functions to be part of N-API, but those need to be generic enough so that they can be implemented across platforms.

@aruneshchandra, @hashseed do you have input on what a generic api might be to match functions to entries in a stack trace?

@mhdawson
Copy link
Member

I'm going to close this as there has been no discussion for quite some time. @Flarna if this is still an issue I'd suggest opening an issue in the nodejs/node repo. Closing. Please let us know if you think that was not the right thing to do.

@Flarna
Copy link
Member Author

Flarna commented Nov 15, 2021

As of now we stick on these v8 APIs and a few more others. It's not a problem as of now as we still have the ABI guarantee with a major nodejs version.
I haven't spent a lot thought on this since I created the issue. Maybe I will create a PR to add them at some time in future.

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

3 participants