-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
My work on a next-generation proof of concept for ZMQ bindings for Node.js #189
Comments
This is super cool. I'm generally pretty happy with taking this direction. I'll give this new API a spin with our current code to see how it behaves and see how easy it is to adapt. The way I'd end up wrapping this (like we do now in nteract) is to turn these async iterators into observables which I don't think is too bad. I can see some areas where I'd prefer the error flow I have with observables (noting the timeout causing promise rejection), though I think I see how I'd handle these generally. All that being said, async iterators are stage 3 (further along), Observables stage 1 (and ready for next stage). I guess that's a yay for async iterators from me I suppose. I'm interested to hear what @kkoopa thinks as someone so heavily involved in this library and in node.js addons. |
Hi @rolftimmermans, good stuff! Please steal what you need from JustinTulloss/zeromq.node#516 which is also a full rewrite. |
Looks good. This library has been in need of an overhaul. |
An update: I have added API documentation, support for socket events and prebuilt binaries for Node.js 8.6 on Linux/macOS. It is available as |
@rolftimmermans Wow thanks for digging into this. I'm excited to take a look soon. |
I think for us at least we'll need to figure out how well N-API works with Electron, so we're going to have to link up our current setup with the new bindings. Thanks for publishing on |
Electron compatibility depends on one of the two following happening:
|
I have some initial benchmarks comparing zeromq-ng vs zeromq.js. There are three types of benchmarks, all are executed with both zeromq-ng (zmq=ng) and zeromq.js (zmq=cur) various message sizes (msgsize=...), and tcp/inproc transport (proto=...):
All benchmarks can be found here. The benchmarks were executed on my Macbook. It would be nice to get some Linux benchmarks too! queue proto=tcp msgsize=1 zmq=cur x 51.79 ops/sec ±1.43% (56 runs sampled) Some casual observations:
Overall I find the differences pretty small. It would be nice to have some "real world" benchmarks. Can anyone suggest a good use case to test? |
How much of the old API (the "typical" API that is) can we keep around, marking for deprecation, even if it wraps the new implementation? |
The only issue I have is, why does this have to be in yet another npm namespace? We had zmq, I don't know why but you guys moved it to zeromq, and now you're moving it again to zeromq-ng? Can't we just keep one namespace? It's so much easier to upgrade when the namespace doesn't change. |
We're not moving it to |
As mentioned I much prefer to keep everything in @rgbkrk I like your idea and will try to create a wrapper that behaves like the current API. It would sacrifice all the semantic improvements, but should make it easier to use both APIs side by side. |
I have added a compatibility layer that covers 95% of the existing API. Missing (and currently impossible to add with the new API) are: (un)bindSync, and read(). |
That is great. Should hopefully make the transition much smoother. |
Hi there |
As far as I'm concerned – absolutely! More real world usage means more feedback and more fixes. |
Here's the direction I'd like to go:
How's that sound @rolftimmermans? |
That sounds absolutely excellent. I will continue work on
For the last one I'm dependent on changes to node-pre-gyp (or prebuild – same issues apply there). I also wonder what we should do about Electron builds; I am not quite aware what the N-API story is there. I'll gather all details over the next week and open a new issue to discuss the practicalities of how this can be accomplished. Any feedback regarding the NG API is more than welcome in the mean time. |
This is really great stuff. @rolftimmermans, have you been able to do this without copying the buffers you send? In the old implementation, every buffer gets duplicated, which really ruins part of what makes 0MQ fast in the first place. |
@rolftimmermans It's been quite some time since you started this effort and it looks like I would be 👍for moving your efforts into this repo and start making a few beta releases to get more feedback from the community. @ronkorving @reqshark @rgbkrk @kkoopa What do you think? |
I see the tests are written in typescript now @rolftimmermans, would you consider doing the base library in typescript too? 🤤 |
I think it's sort of ready for prime time.
|
Most of it is C++ so that won't give us any TypeScript bindings. I have hand written TypeScript definitions published as part of the library and the test suite itself uses them (and therefore they should be tested, too). Potentially the getters and setters for sockets and context (currently implemented with meta-programming) can be written in TypeScript by using code generation. That is a fine approach, but it requires some more effort. I'm not sure how easy it is to combine an automatically generated |
That's awesome! 🎉 🚀
I tried it out at nteract/nteract#3388 and it worked great with Electron 3 so far. There are a few annoying issues related to the publishing process, since
I think technically one might even be able to use N-API together with Node 6, but I think it's totally reasonable drop support for Node 6. Personally I'd even be OK with only supporting Node 10+ 😇
If nobody raises objections to
|
I'm cool with that plan. |
Great work, looks very clean. I would be very worried about the segfault, though. You'll probably want to analyze a core dump. Maybe this group can help with the analysis? |
ZeroMQ-NG has been merged! The tracking issue is #347. |
Great culling of the issues @rolftimmermans! I'm finally trying to catch myself on everything for the new zeromq.js. 😄 |
TL;DR: I'm this issue to start a discussion about the future of zeromq.js and my work on a proof of concept for a next-generation version. Please let me know your opinion!
Background
We have recently taken a system into production that uses zeromq.js. This works well, but I encountered a number of issues with this library that triggered me to think about solutions. Unfortunately some problems did not appear to be fixable without a massive overhaul of the internals. I will briefly describe the main problems.
Problem 1: Sent messages are queued, but not with ZMQ
Zeromq.js uses an internal queuing mechanism for sent messages. Essentially it pushes all messages onto a JavaScript array and sends them out as soon as the socket is ready for writing. This works nicely except when it doesn't:
Related issues: #152 #185 #124
From #152 (comment) I understand that this queueing was built to improve performance. Which means that matching the performance of the current library should be a goal of any new version.
Problem 2: Messages that are received are read automatically
Readable sockets follow the event emitter pattern. This is common in Node.js APIs, so it makes sense for most people. There is important one drawback, though: messages are read automatically.
It appears that a call to
pause()/resume()
offers a suitable workaround, however:pause()
also disables writes, which may not be intentional.pause()
will always dispatch all messages that can be read from the ZMQ queue without blocking before it actually stops reading from the socket.pause()/resume()
to approximate features that are offered by libzmq by default, which is not a very friendly developer experience.Related issues: #139
Other potential improvements
There were some other things we encountered while working on the library that we believed could be improved:
My work on a proof of concept
Given that nature of problems 1 & 2 I set out to rewrite the internals and design a better API that is suitable for honouring high water marks and timeouts for both send & receive operations.
Because I was now effectively rewriting the entire library I also decided to address the additional improvements mentioned above. Especially the move to N-API is non-trivial and influences most of the native code.
To summarize, the goals of my experiment were:
What is included and working
What is still missing
The following features have not yet been added. My plan is to implement them over the next
weeksyears:A proxy classA curve keypair helper functionMonitoring sockets for eventsSupport for prebuilt binariesDraft sockets and associated (new) APIsThorough documentation of the APIWindows supportElectron supportReal world testingSo, how do I use it?
To build the proof of concept version you need a local installation of libzmq with development headers. For example, on Debian/Ubunto you canapt-get isntall libzmq3-dev
, or on macOS you canbrew install zeromq
.Then clone the repository and run the tests withyarn test
ornpm test
.Install with
npm install zeromq-ng
.Next, have a look at the the examples in the README. To give you a taste of the API, this is how you can use it:
I've you made it this far reading this post, thank you for your attention!
What next?
I'd like to invite you to have a look at https://github.com/rolftimmermans/zeromq-ng and give your feedback.
Specifically, I'd like to discuss two scenarios:
I'd be completely happy with either.
In addition to the above I am looking from feedback from users that have experience with one or more of the problems mentioned above (or even other problems!). It would be nice if you can give your opinion on the API or even test the proof of concept.
I am also in the process of writing and collecting benchmarks and will post the results soon. Based on what I have tested so far I'm expecting similar performance in general.
The text was updated successfully, but these errors were encountered: