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

3x overhead on Node 6 #102

Closed
mcollina opened this issue Aug 4, 2017 · 7 comments
Closed

3x overhead on Node 6 #102

mcollina opened this issue Aug 4, 2017 · 7 comments
Assignees

Comments

@mcollina
Copy link
Member

mcollina commented Aug 4, 2017

I've benchmarked https://github.com/mcollina/native-hdr-histogram/tree/napi on Node 6.11 against the NAN implementation with https://github.com/mcollina/native-hdr-histogram/blob/napi/bench.js.

The result is that the NAPI version takes 167ms to complete, vs 67ms to the NAN version.
This might be related to this specific implementation, as I have not tested it yet on Node 8.3.0 (I'm blocked on #101).

flamegraph.html.zip <-- if a flamegraph might be of help.

@mcollina
Copy link
Member Author

mcollina commented Aug 4, 2017

This is the flamegraph with NAN. There are some notable differences, so check them out.

flamegraph.html.zip

@jasongin
Copy link
Member

jasongin commented Aug 4, 2017

That's surprising, as I have not seen more than about 20% overhead even in the most extreme cases.

I'd like to look into it, but it might be a few days before I have time.

@mhdawson
Copy link
Member

@jasongin have you had a chance to take a look yet ?

@jasongin
Copy link
Member

Sorry I have not had time to look at this, and will not anytime soon. I will be on vacation 8/17 - 9/4.

@mhdawson
Copy link
Member

@digitalinfinity volunteered to look at this as a priority in the N-API weekly meetin glast week.

@mhdawson
Copy link
Member

@digitalinfinity can you add the info you reported in a meeting a while back so that we can close this issue.

@mhdawson
Copy link
Member

mhdawson commented Dec 7, 2020

Going to close out as Node.js 6.x is long out of LTS. Please let us know if this is not the right thing to do.

@mhdawson mhdawson closed this as completed Dec 7, 2020
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

4 participants