Repository navigation
N-API Support for node-sqlite3 #830
Description
The recently announced Node 8 has a new experimental feature called N-API which is aimed at reducing maintenance cost for node native addons. Checkout this blog for more details on its benefits.
Module node-sqlite3 is one of the top 30 native modules by dependency count, and in order to help the Node.js community make the important transition to N-API, we are hoping you will be able to work with us in order to port node-sqlite3 to support N-API. Your support and feedback is important in making this effort a success.
I am part of the N-API working group and and would like to talk to you about how you can can get started with N-API and what help we might be able to provide. I'm available to talk on the phone individually if you'd like. Alternatively, if you prefer, feel free to jump in our WG meeting which happens every Thursday at 1.30 Eastern / 10.30 Pacific US time, to discuss any issues.
Here’s a video of N-API in action
Activity
springmeyer commentedon Jun 15, 2017
Hello @sampsongao - this is great - thank you for your efforts on N-API. Yes, I would like to support N-API in node-sqlite3. I see you've created ttps://github.com/mhdawson/node-sqlite3/tree/v3.1.4-abi to test the N-API port. How did it go? Was there any performance impact?
sampsongao commentedon Jun 15, 2017
The v3.1.4-abi is the port directly using N-API. But since the N-API is quite verbose, so we also developed C++ helpers to make it simpler to port. I am currently working on porting node-sqlite3 to helpers. And there are some problems I still need to address. Feel free to take a look at the v3.1.4-napi-wrapper branch.
springmeyer commentedon Jun 15, 2017
Thanks for the details @sampsongao. Are there any examples of ports of very simple modules you could share as well? For reference our team is planning to work on porting our simple example node module: mapbox/node-cpp-skel#45
sampsongao commentedon Jun 15, 2017
You can take the node-nanomsg port as reference. If you want to take something a little more complex, you can checkout the node-canvas port. Also, take a look on the node-addon-api. That is the wrapper I was talking about.
aruneshchandra commentedon Jun 15, 2017
@springmeyer - you can tryout the N-API yeoman generator to get started with this https://github.com/digitalinfinity/generator-napi-module.
sampsongao commentedon Jul 13, 2017
@springmeyer How is your experience using N-API is like? We would like to hear your feedback and the obstacle you faced during your migration.
springmeyer commentedon Jul 14, 2017
@sampsongao - thank you for the ping. I've not had a chance yet to experiment with the N-API but I will let you know as soon as I do.
sampsongao commentedon Aug 17, 2017
Hi @springmeyer , any update on your experiment?
springmeyer commentedon Nov 27, 2017
@sampsongao @aruneshchandra - do you have a branch of latest node-sqlite3 that is working against latest
node-addon-apithat I can take a look at to learn more?sampsongao commentedon Nov 28, 2017
@springmeyer You can find it here. https://github.com/mhdawson/node-sqlite3/tree/node-addon-api
Commands for building and testing should be the same as before.
springmeyer commentedon Dec 8, 2017
@sampsongao - thanks, however I hit these errors trying to build that branch: https://gist.github.com/springmeyer/f767c07b662c200121a0296b2fe36a08. Is it working for you? What node version are you testing with? I tried with node v8.9.3 on OS X 10.12
springmeyer commentedon Dec 16, 2017
@sampsongao any idea about the above errors?
springmeyer commentedon Dec 18, 2017
@sampsongao bump
sampsongao commentedon Dec 20, 2017
I am using node master now which is node 10 pre. However, I can't reproduce the problem on my machine and it seems to be node-addon-api related. Can you make sure you are using node-addon-api@1.10.
sampsongao commentedon Dec 20, 2017
Actually, it is problem with std::vector. The assign method is a build in method for std::vector. Do you know any reason vector is not working in X code?
19 remaining items
jschlight commentedon Feb 11, 2019
I'm now actively working on a PR to address this issue. My immediate objective is to apply the commits @sampsongao and @anisha-rohra previously created for this issue to the current codebase up to the point where the libuv dependencies need to be addressed.
jschlight commentedon May 13, 2019
At this point I've got nearly all the unit tests passing. I will be working with Michael Dawson to configure my development system to debug the remains failing unit tests.
springmeyer commentedon May 15, 2019
@jschlight - great news! Please push up a PR as soon as you have something ready. Thank you.
springmeyer commentedon Jun 29, 2019
@jschlight any update on progress here?
jschlight commentedon Jul 2, 2019
Michael Dawson and I were able to resolve the issues with my development system. I am able to build and run the N-API module on my development system and run the Mocha tests in
lldb. The failing Mocha tests are mostly related tolibuvwhich I'm working to understand. My objective is to see if the dependencies onlibuvcan be replaced with N-API so that we get full benefit from N-API's stable ABI.This process is taking longer than I would like, but I remain committed to seeing it through.
jschlight commentedon Jul 12, 2019
The Mocha tests are all passing on my Mac development system, but the AppVeyor and Travis CI operations are filling in the compile process. I'm now investigating why that's the case.
mohd-akram commentedon Oct 1, 2019
I've rebased @jschlight's branch on the latest master and converted the new code added to N-API. I've also replaced the libuv async work code with N-API code. Disabling N-API C++ exceptions helped reveal why some tests were failing which I fixed. Some tests are still failing and there are some random segmentation faults on some platforms if anyone wants to take a look.
jschlight commentedon Oct 6, 2019
Thank you so much @mohd-akram for your work on this. I've merged your changes into my fork and resolved a handful of conflicts. I'm now looking into the CI errors.
Is it possible to see the errors you're getting from Travis CI and AppVeyor for your fork?
The N-API team meets for an hour each week on Zoom. You are most welcome to join us to answer any questions you may have. You can see the meeting specifics on the Node.js Foundation Calendar under the heading Node.js N-API team weekly meeting. The meeting is currently held on Mondays.
mohd-akram commentedon Oct 7, 2019
You can see the failures here. I was able to avoid the segmentation fault in Node.js 8 on macOS by modifying the
Statement::Work_AfterRunmethod and adjusting itsSTATEMENT_ENDmacro to exclude thedelete batonpart. It seems something uses it after it's freed. I'll see if I can join this meeting.jschlight commentedon Oct 7, 2019
Thank you @mohd-akram. I will look at this more closely first thing in the morning.
jschlight commentedon Oct 7, 2019
@mohd-akram We discussed your macOS issue during today's N-API Team call. It was suggested that it might help to set
visibility=hiddenfor macOS. The details are under step 4 on this page. This solution was for modules that load multiple times. But it may be applicable here also.I'd like to see if we can get to a common codebase. Do you see any issues with me deleting my fork of mapbox/node-sqlite3 and forking off of your project instead?
springmeyer commentedon Apr 11, 2020
@jschlight @mohd-akram - what are the next steps on getting node-sqlite3 upgraded to use N_API? If you have something that is close to ready I recommend creating a PR against this repo. Thanks!
mohd-akram commentedon Apr 11, 2020
I've rebased and created PR #1304.
jschlight commentedon May 4, 2020
@springmeyer @kewde At the request of the N-API team, I've run some benchmarks on the N-API implementation compared to the NAN version. The results are available here:
nodejs/abi-stable-node#288 (comment)
Overall, the
node-sqlite3N-API implementation is running about 15% longer than the NAN version which is not out-of-line with other implementations we've seen.I'm now monitoring incoming
node-sqlite3issues. I plan to triage any issues specific to the N-API implementation and make the N-API team aware of any I cannot resolve.