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

Considerations about bcrypt porting to N-API #287

Closed
NickNaso opened this issue Jan 3, 2018 · 4 comments
Closed

Considerations about bcrypt porting to N-API #287

NickNaso opened this issue Jan 3, 2018 · 4 comments

Comments

@NickNaso
Copy link
Member

NickNaso commented Jan 3, 2018

Hi, everyone!

as you know recently I worked at porting of bcrypt to new N-API. The module isn't published on npm yet, so I created a repo based on my work here: https://github.com/NickNaso/bcrypt-napi and used it in web application in production here: https://www.lexgenda.com
I wrote an article about the described activity here: How I ported bcrypt to new N-API . Another developer translated the article in Russian and you can find it here: Russian version of: How I ported bcrypt to new N-API

Performance estimation

The first activity that I did about the estimation of performance was to execute all the tests and calculate the average of the execution time. The N-API version of the addon was found to be about 2% faster.

To verify and reinforce this preliminary results I created a repo https://github.com/NickNaso/bcrypt-perf where I conducted some experiments about bcrypt performance.
At the end in the various situations the N-API version was faster than NAN version and the gain in percentage was between 0.8% - 4.19% and the gap is to high in the synchronous api.

@mhdawson
Copy link
Member

mhdawson commented Jan 3, 2018

Thanks for writing up that article, tweeted out here https://twitter.com/mhdawson1/status/948562156127055872. Can the rest of the team do the same or retweet if you have notalready.

Looks like you already have a good number of people who have read and 'clapped', good job :)

I'll be interested to hear a bit more about your perf experiments in the next N-API meeting.

@NickNaso
Copy link
Member Author

Hi everyone,
today I want announce that I just finished my work on bcrypt module. In this thread I want recap all that I did to arrive at use the N-API version of the addon in production on my web application.

  • Fork the original project https://github.com/kelektiv/node.bcrypt.js and refactor all the functions and methods with the API provided by the C++ Wrapper of N-API

  • Execute all tests on local machine and once all of them passed successfully I created a PR and after the module passed all tests on CI my work was merged on the napi branch https://github.com/kelektiv/node.bcrypt.js/tree/napi

  • Until I attempt that mantainers publish the module on npm I conducted some experiments about performance of N-API version against the previous implemented with classical V8 API using NAN. All my works and results about that are on this repo https://github.com/NickNaso/bcrypt-perf. Substantially the N-API version is little faster in avarage 2% than the NAN version. On my tests I considered the execution time of the most common operations accomplished by bcrypt (password hashing and comparing password)

  • Start testing bcrypt on web application before on development environment and after on production. I created a repo that contains my work and use the fit reference on package.json of my application.

  • Use published n-api tagged version of bcrypt (bcrypt@n-api) in production on web application Lexgenda
    Lexgenda is a web application where user after signup and signing a subscription can execute research over the judgments issued by italian courts, essentially it's a search engine.
    Below I report some screenshots:
    search
    details

@mhdawson
Copy link
Member

@NickNaso thanks for the good summary. Great work!

@mhdawson
Copy link
Member

This was a great FYI but I think it can now be closed. 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

2 participants