search

uNetworking / uWebSockets.js

μWebSockets for Node.js back-ends :metal:
Apache License 2.0
8.08k stars 573 forks source link

Prebuilt binaries for Electron ABIs? #454

Closed mikeseese closed 3 years ago

mikeseese commented 3 years ago

Hello! This is a long shot because of your prior sentiment about not supporting Electron, which is totally fair and understandable. However, I figured I'd ask just in case.

We're packaging our NodeJS project into an Electron app, and we use uWebSeckets because of the awesome performance. However, since Electron has separate ABI versions the prebuilt binaries you've provided here do not work. Is there any chance you'd reconsider prebuilding the binaries for Electron ABIs?

Thanks!

ghost commented 3 years ago

Never say never but I don't feel like increasing the scope of this project unless I have to. Node.js is the target and that is what I support. Supporting Electron is more work, changes to how modules are loaded, more testing, and becuase Electron typically is used client side while this project is a server, it makes little sense. You can always compile the project for Electron if you absolutely need it in the client.

Or if you have a very good pitch for why it is needed and can back the need up. For instance, say Google want to build their Drive app using it (somehow) then I could rethink my stance but otherwise I don't really feel like Electron is an important platform.

mikeseese commented 3 years ago

Thanks for the quick response! I've got a couple of responses.

Never say never but I don't feel like increasing the scope of this project unless I have to

A fair stance stance, and one that is well appreciated by projects that use uWS to ensure performance and security.

Electron typically is used client side while this project is a server, it makes little sense

It should be noted that Electron has both a main and a renderer process. The renderer process is what faces the user and usually would contain client code rather than server code. However, the main process serves as a backend for more advanced Electron applications. While IPC is generally used to communicate between the two, I don't think it's unreasonable that someone may write a server in the main process which the client process acts as a client to that server. This is our particular use case; I'm not sure how common it would be, but we make a server NodeJS library that is provided with both command line and graphical user interfaces which act as clients to that server. It's just definitely not out of my thinking for Electron apps.

Or if you have a very good pitch for why it is needed and can back the need up. For instance, say Google want to build their Drive app using it (somehow) then I could rethink my stance but otherwise I don't really feel like Electron is an important platform.

I don't think I would be able to bring evidence that "someone large or many small people would love to use uWS in Electron". However, I will say that Electron is definitely an important platform. Some examples of "big time" Electron (which is built and maintained by GitHub by the way) users are:

So while I 100% understand that perhaps uWS itself might not be a prime candidate for many Electron apps, I would say that Electron is far from being an obsolete platform. It's the modern day Qt; if you want to build a cross platform GUI application, Electron is going to be a primary option. And users will likely try to bundle everything as part of that application, including any servers you need to host to support that application (like us).

You can always compile the project for Electron if you absolutely need it in the client.

With that all said, I'm looking into what is necessary to do this. https://github.com/nodejs/node-gyp supports easy rebuilding towards Electron targets, but this would require uWS.js to change the build process to use node-gyp's toolset rather than what you have now.

Even further, https://github.com/electron/electron-rebuild provides an easy scripting step for devs to rebuild any node-gyp addons for their Electron target so you wouldn't have to necessarily build/host the associated .node files

Supporting Electron is more work, changes to how modules are loaded, more testing

Even more further, as someone who has contributed their precious time to a free project, I totally understand this

ghost commented 3 years ago

If I'm going to do something there needs to be measurable gain of some sort that's demonstrable.

Can you show reduced complexity? Can you show reduced latency? Can you make some visual demo of how uWS improves Electron apps? Like lower latency or something like mouse cursor tracking or whatever.

I need measurable gain otherwise it's not worth doing anything just for the sake of doing

ghost commented 3 years ago

This is measurable gain: https://levelup.gitconnected.com/serving-100k-requests-second-from-a-fanless-raspberry-pi-4-over-ethernet-fdd2c2e05a1e

ghost commented 3 years ago

So an electron app is basically Chrome and Nodejs? In that case you should be able to beat the IPC performance using uWS. I'm thinking of a demo where you move the mouse in Chrome and every mouse move results in a WebSocket message being sent to Nodejs and back. When it comes back you update the position of a sprite on screen. That way you can visually display lag and latency

mikeseese commented 3 years ago

While our use case is not a main<=>renderer communication mechanism but rather hosting another websocket server for other non-electron clients to connect to, I went ahead and did your suggestion. You can find the repo that has a real simple time measurement at https://github.com/seesemichaelj/electron-uws-test. On an WSL 2 environment with Ubuntu, uWebSockets is consistently slower than IPC. I haven't had the time to build you a full analysis, but you can go see it for yourself if you would like to. It's fairly simple to run, just yarn && yarn start; time differences between time sent and time received are shown in the main process's console output. Since uWebSockets is only a server, I had to use ws as a client. I'm sure this test isn't fully verified and accurate, but I don't have time to continue down this road.

Here are some sample delta timestamps for convenience:

IPC 0.2272464856505394
uWS 0.28674647957086563
IPC 0.2882464826107025
uWS 0.3228464871644974
IPC 0.2639464810490608
uWS 0.31764648109674454
IPC 0.20324648171663284
uWS 0.2898464873433113
IPC 0.40304648131132126
uWS 0.42554648220539093
IPC 0.18764648586511612
uWS 0.22694648057222366
IPC 0.2190464809536934
uWS 0.2824464812874794
IPC 0.21704648435115814
uWS 0.23834648728370667
IPC 0.23144648224115372
uWS 0.30404648184776306
IPC 0.2622464820742607
uWS 0.31294648349285126
IPC 0.21604648232460022
uWS 0.25514648854732513
IPC 0.29034648835659027
uWS 0.34054648876190186
IPC 0.2523464784026146
uWS 0.30974648147821426
IPC 0.2685464844107628
uWS 0.30884648114442825
IPC 0.268746480345726
uWS 0.336746484041214
IPC 0.27074648439884186
uWS 0.2960464879870415
IPC 0.25754648447036743
uWS 0.27474648505449295
IPC 0.31034647673368454
uWS 0.333546482026577
IPC 0.2956464812159538
uWS 0.31544648855924606
IPC 0.25214648991823196
uWS 0.2938464879989624
IPC 0.275146484375
uWS 0.3102464899420738
IPC 0.26484648138284683
uWS 0.3451464846730232
IPC 0.2522464841604233
uWS 0.2492464780807495
IPC 0.29334648698568344
uWS 0.31614648550748825
IPC 0.2661464810371399
uWS 0.3149464800953865
IPC 0.37544648349285126
uWS 0.42424648255109787
IPC 0.26354648172855377
uWS 0.26104648411273956
IPC 0.29194648563861847
uWS 0.32174648344516754
IPC 0.2581464871764183
uWS 0.31774648278951645
IPC 0.29254648089408875
uWS 0.31454648822546005
IPC 0.29334647953510284
uWS 0.32344648241996765
IPC 0.2712464779615402
uWS 0.3287464901804924
IPC 0.2704464793205261
uWS 0.3173464834690094
IPC 0.30644648522138596
uWS 0.3780464828014374
IPC 0.2702464833855629
uWS 0.3026464879512787
IPC 0.26404648274183273
uWS 0.28594648092985153
IPC 0.2749464809894562
uWS 0.29334647953510284
IPC 0.2816464826464653
uWS 0.315046489238739
IPC 0.25124648213386536
uWS 0.27714648842811584
IPC 0.24384648352861404
uWS 0.2872464805841446
IPC 0.24404648691415787
uWS 0.29424647986888885
IPC 0.2579464837908745
uWS 0.3245464861392975
IPC 0.24614648520946503
uWS 0.2879464849829674

Do note that I measured the client-side JSON.stringify that I performed before sending (as the current setup doesn't have a timestamp after the stringify), and it measured roughly 10-20 ms. While this is sometimes the difference between IPC and uWS, at best case it looks like it would be the same speed rather than faster

I'm not sure why a measurable gain is necessary to support a very popular platform. We've already spent enough time talking about this, that I went ahead and made all the necessary changes to support an automated Github CI that builds the Electron binaries. The main changes were:

  1. Changing uWebSockets.js build process to use the standard NodeJS addon node-gyp build environment instead of your custom build script.
  2. Using https://github.com/electron/electron-rebuild to rebuild the node-gyp addon to use the electron headers.

Feel free to take what you need from our port here: https://github.com/trufflesuite/uWebSockets.js. You'll notice I also removed the binaries branch and made it all merge to master automatically. I also made the changes to be able to publish this to NPM to mimic a similar payload size of that of binaries.

Unless @davidmurdoch wants to pick up this discussion, my contract requires me to move forward now that I'm no longer blocked by this and no longer have time to try to bring this to the official repo. Sans being incorporated into the official repo, we will redistribute uWebSockets.js on npmjs.com with unofficial binaries including Electron versions at @trufflesuite/uwebsockets.js. As an aside, I highly recommend you distribute on npmjs.com; someone else has already forked and is publishing under the main name uwebsockets.js (rather than using a @scope/uwebsockets.js which is what we'll do as it signifies it's a fork and unofficial).

Thanks for your time and consideration.

davidmurdoch commented 3 years ago

Thanks @seesemichaelj for this! Also, thank you @alexhultman for developing and maintaining uWebSockets, and for your thoughtful consideration of @seesemichaelj proposal.

We'll be shipping uWebsockets.js in the next version of Ganache, the most popular Ethereum blockchain simulator (https://www.trufflesuite.com/dashboard). Our next version will be the first version using uWebsockets.js and we have found that this version is 3x faster in some real-world test suites (though not all of this performance improvement is from switching from node's http to uWebSockets.js, it does help!).

For further context here, Ganache ships as a command line tool, a JavaScript library, and as an Electron-based UI.

As far as @seesemichaelj's suggestion for distribution to npm goes, I totally understand your stance on the matter (and have read your recent post about it) and do not agree with his suggestion (sorry mike :-) ).

If you decide not to support electron binaries, I understand; we have publish our own to npm for our own ease-of-use, but please do let me know if you'd like our npm package's README.md changed in anyway so as not to confuse users.

Thanks again!

ghost commented 3 years ago

I'm not sure why a measurable gain is necessary to support a very popular platform.

Because if you can't show proofs of there being any significant difference to using uWS in this "weird" way (which I still don't understand), vs. using something that already works, like websockets/ws, then there is no reason to bother. I absolutely did not create this project to be popular - I made it to solve a real measurable business problem either more efficiently, more stable or simpler. Popularity is a side effect. I rather have two companies use uWS with tremendous gain than have billions of users use it because of hype and misunderstandings.

If you absolutely cannot show any motivation why uWS would be of any gain I rather pluck my belly button and play Little Nightmares 2.

Node-gyp is out of the question for many many reasons - first being that it products piss poor performance due to not applying link time optimizations second being that I hate it with passion.

I can look into this, but in the mean time you need to understand that this project is licensed as source code, not as the whole thing. If you want to distribute modifications on npm you definitely cannot use any logos, texts, names or anything that would identify this as being the original thing.

Again, show proofs of improvement and I'll consider it with high priority but if there is nothing that's really better by using uWS it makes no sense to expand to this platform.

I could say, we need to be on Nintendo Switch because it is a popular platform. Okay but why? What are the reasons?

ghost commented 3 years ago

https://www.trufflesuite.com/ganache Quickly fire up a personal Ethereum blockchain which you can use to run tests, execute commands, and inspect state while controlling how the chain operates.

So some kind of WS server spins up and you then can run a bunch of client to it. Okay, I get that.

Our next version will be the first version using uWebsockets.js and we have found that this version is 3x faster in some real-world test suites (though not all of this performance improvement is from switching from node's http to uWebSockets.js, it does help!).

Okay, so you do have some indication of improvement here?

ghost commented 3 years ago

All you really need is to fetch from https://www.electronjs.org/headers/v11.2.3/node-v11.2.3-headers.tar.gz insteaad from nodejs.org. That's literally it. So a minor change to current build can fix this.

davidmurdoch commented 3 years ago

Okay, so you do have some indication of improvement here?

I've only measured the performance improvements of our library and CLI applications. This next version is a rewrite, and since I've done a lot of performance-related work in this rewrite I really can't provide you with concrete numbers on uWebSocket.js's performance contributions :-(

But yes, we do, but to be clear, I haven't tested the performance of our Electron application. But to be honest, performance is not really all that important there, as the majority use it as an exploratory tool, not as a high-performance testing tool (though many still do run their test suites against it, much to my dismay :facepalm:). But I guess it doesn't matter how our users use our Electron app; they will most definitely enjoy a speed boost, too.

That said, the reason I'd like for uWebSockets.js to build Electron binaries is because our Electron application uses our published library internally.

ghost commented 3 years ago

I have minimal changes that supports building for both Electron and Node.js but Electron headers are missing openssl (boringssl). This is a bug at their end (unless they can say where they are?).

You built without SSL: https://github.com/trufflesuite/uWebSockets.js/blob/master/binding.gyp#L78

This is kind of crippling the product, so this should be reported as a bug in electron headers. They really should ship the boringssl headers like nodejs ships their openssl headers.

My diff is like 5 line added so this is really minimal

ghost commented 3 years ago

They have acknowledged this to be a bug over at Electron now. So I'll wait until they solve it on their end then I can add this support.

davidmurdoch commented 3 years ago

Thanks @alexhultman!

ghost commented 3 years ago

From what the Electron developers said, Electron is pretty far from Node.js and does not expose the same toolset as Node.js. This project might swap to its own bundled BoringSSL binary in the future but right now it is not possible to support Electron since they don't support the dependencies we need.