Rebase writing system from stream-writed in stream to buffer-writed

[scarry1992]

I want to redo the writing system as mentioned in the title. That'll give us less overhead by ws/wss connections, generate less payload when processing fewer socket frames. @mcollina, what do you think about it?

[mcollina]

The reason why we switched to this was because on tcp and ssl sockets is significantly better to avoid copying the buffers. That slows things down significantly. Ideally the .cork() and writev implementation in Node and ws should take care of chunking and avoiding the problem. There might be something to fix in websocket-stream.

[scarry1992]

@mcollina, so if I want to send packet per frame from browser, I must to implement cork() in websocket-stream. Right?

[mcollina]

I think it should be https://github.com/maxogden/websocket-stream/blob/master/stream.js#L154-L166.

Why is this not working?

[scarry1992]

@mcollina, as I see, writev turned on by options objectMode is false, but objectMode value by default is false. So, by default I getting packet per frame, but that's not realy. I'm getting all properties of packet per frame separatly. I see it on my broker.

[mcollina]

we should create it in not in object mode in MQTT.js.

[scarry1992]

What do you mean? We must to implement it in ws connector in mqtt.js? But how?

[mcollina]

In MQTT.js we need to make sure that the websocket stream is not in object mode

[scarry1992]

So, if I added property objectMode: false to wsOptions in my connection, I`m getting set up writev handler in websocket-stream. Then in mqtt-packet in generate function stream's try cork(). But writable stream which's piped with writable stream in mqtt.js can't make writev. As I see, I must to imlement writev in _setupStream and to process objectMode for creating writable stream inside that handler. Right?

[mcollina]

What do you mean with "mqtt.js can't make writev".

[scarry1992]

What I mean is:

• the writable stream (in websocket-stream) is piped with the writable stream in mqtt.js
• the latter stream (in mqtt.js) can't peform writev

[mcollina]

Which Writable? This https://github.com/mqttjs/MQTT.js/blob/master/lib/client.js#L245 is used when reading the packets, not when writing them.

[scarry1992]

Oh, I see. But I realy can't understand what can I do for to implement sending packet per message. Can you to explain to me, please?

[mcollina]

I do not know where they get chunked up, and where cork() is failing us. https://github.com/maxogden/websocket-stream/blob/master/stream.js#L76 might likely be the issue.

[scarry1992]

https://github.com/maxogden/websocket-stream/blob/master/stream.js#L76 probably you are right. Because I'm debugging my app and I see in mqtt-packet stream is Duplexify, but debug way creating Transform stream. And in Duplexify stream objectMode setted in true.

[mcollina]

I think the fix should come to WebSocket stream.

[scarry1992]

You are right. Issue in that place. I remove part of logic with duplexify stream, and I get working cork logic..

[scarry1992]

@mcollina, do you know the maintainer of websocket-stream package? He is streel alive?) He don`t have activity on github from october 2018..

[mcollina]

I have commit and publish rights Il giorno ven 25 gen 2019 alle 08:19 scarry1992 <notifications@github.com> ha scritto:

…

@mcollina <https://github.com/mcollina>, do you know the maintainer of websocket-stream package? He is streel alive?) He don`t have activity on github from october 2018.. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#48 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADL43h1y5Z6tBeiOwtxUbXSVdVL603Jks5vGr4jgaJpZM4aOMp5> .

[scarry1992]

That's great! Can you spell me: cork/uncork part of logic broken in https://github.com/maxogden/websocket-stream/blob/master/stream.js#L76. When I remove that initial stream by duplexify and leave stream = proxy only, everything works well. Can you tell me why you are doing stream by duplexify and what benefits it gives.

[mcollina]

It is used to handle the delayed open scenario. We can remove it, but we might have to change some other things, as we can’t write until the ws is open.

[scarry1992]

Duplexify method obj() set objectMode setting in true automaticaly. And if I extending obj() logic with options parameter, it setup objectMode truely, but don't working anyway. Maybe some issue in duplexify too?

[mcollina]

You might have to add our _writev implementation to the duplexify stream as well.

[scarry1992]

That's right, but if I added _writev implementation to duplexify, I'm getting packets separated on 2 chunks. (first byte in first frame, but all another bytes in second frame) W/o duplexify it's working as well (all bytes in first frame).

[mcollina]

Then we should remove duplexify and handle the connection state separately.

[scarry1992]

@mcollina, I tried remove duplexify from dependensies, but a lot of duplexify logic needed in server side. I fix it, but need you review. max-mapper/websocket-stream#147

[scarry1992]

@mcollina, and I retested all mqtt.js with that fix - everything ok