Open aslakhellesoy opened 10 years ago
If there are any changes required in Primus in order to make this function I would love to help out if needed.
@3rd-Eden I wasn't actually suggesting to implement Primus support in this library (transport-adapters).
I was questioning whether this library is need at all, if we make ShareJS work with Streams.
If primus was used in ShareJS this lib would become obsolete and make me very happy :) I just have two concerns, will @josephg be ok with this and what is the client side overhead of primus.
It's not really a matter of supporting primus per se, but rather to support the Node.js Stream interface on both the server and client.
I have a working prototype on my stream branch.
In fact, this code could be implemented without any changes to ShareJS itself. You'd just have to overwrite bindToSocket
on the prototype before instantiating a connection:
Connection.prototype.bindToSocket = // ... a function that can handle a stream instead of a socket.
On the server there isn't anything special to do - streams are already supported.
So while nothing is really needed in ShareJS, it would be nice to be able to construct a Connection with a Stream (as I'm currently doing on my branch).
what is the client side overhead of primus.
I haven't done any benchmarks, but from glancing over the code it seems to just be a Facade around a WebSocket or BrowserChannel, providing a uniform stream-based API.
It uses custom JavaScript events to communicate internal events like connection status and message flow. I don't know if that has any overhead. Maybe each event causes a second trip through the event loop before it's processed?
I'm sure @3rd-Eden can answer.
When you create an abstraction on top of a framework there will always be some overhead. The code for the primus client can be found here:
https://github.com/primus/primus/blob/master/primus.js
It's around 1200 lines of code and 35kb.. This the core weight of the Primus client so the client code for each transformer and it's library still has to be added to that. But I do have to note here that the Primus client is heavily commented so when you minify your code there shouldn't really be that much overhead left. But Primus isn't just a Facade around WebSockets, BrowserChannel it also fixes a lot issues and crashes for real-time communication. For example, older FireFox browsers will close WebSockets connection when you press ESC. No framework prevents or protects them from this behaviour, but Primus does. So it might add a little of weight on the client side, it does ultimately result in a more stable connection with the server. Isn't that something we all want ;)?
As @aslakhellesoy correctly noticed, the internals of Primus communicate with each other using and EventEmitter. As we depend on this functionality heavily, I made the decision of creating a high performance event emitter instead of using and porting Node's stock EventEmitter. The EventEmitter that Primus uses is EventEmitter3 and after alot of benchmarking I've concluded that this is currently the fastest EventEmitter available for Node & browsers. So there might a 2/3 functions call overhead for each received/send message but that's about it. Other than that, I've tried to keep the core a small as possible by allowing it to be extended through a plugin interface.
@Dignifiedquire @3rd-Eden I think I have a working Primus-ShareJS plugin working now. It's over at https://github.com/aslakhellesoy/share-primus
It would be great if you could play around with it and give me some feedback/bugfixes before I announce it on the ShareJS list.
I haven't tested reconnect extensively. It appears that I get reconnect
events for WebSocket, but not for BrowserChannel...
/cc @mattwynne, @jbpros
Cool :)
Browserchannel has its own small reconnect module - if you simply fail to pass reconnect:false
to the socket, the client-side node-browserchannel library will automatically reconnect.
I've recently been playing with websockets in sharejs and I've found that the sharejs 0.7 client currently makes a few assumptions about the connection its using (eg, onmessage(e) should recieve an object with e.data:
. I'm going to fix these problems soon in both libraries to make everything more compatible.
Primus is an abstraction for various web-based streaming protocols. If we could make ShareJS work with Primus, perhaps that would be a superiour solution to the cross-transport problem that what this repo aims to provide?
//cc @3rd-Eden