fails-components / webtransport

Http/3 webtransport support for node
Other
149 stars 22 forks source link

No byob buffer support #6

Closed martenrichter closed 1 year ago

martenrichter commented 2 years ago

Currently the streams are not a byte stream and byob is not support. May be buffer handling should be according to spec.

martenrichter commented 2 years ago

First implementation here https://github.com/fails-components/webtransport/tree/byobreader can be a regression compared to old implementation.

davedoesdev commented 2 years ago

I'm finding webtransport streams to be quite slow compared to http2 streams. Maybe byob would help, or is it the nature of the napi implementation with posted tasks and results?

martenrichter commented 2 years ago

Byob was not faster and the pre napi stuff was more or less the same. The question is if it is/was a good idea to have a separate event loop for the webtransport stuff and not to use the node event loop. On the other hand at least for reading, nothing is blocking the flow of packages to javascript for writing it is a different story, there it is blocked and for small buffers, this may have a bad impact on performance. So instead of one promise in the writable, one may choose to have many.

Also, there is another thing quiche does many things for http3 in userland, that other protocols do on the kernel layer. For example, tcpip is done on the kernel, and the whole quic stuff is done in userland and causes a much higher CPU load.

martenrichter commented 2 years ago

Another question, what do you mean by slow? Data rate? Or delay? With delay, it may again be a problem with the node event loop that is sleeping.....

davedoesdev commented 2 years ago

Data rate. I think it's fine actually. As a library to get things working while Node works on WebTransport in core, this project is great.

martenrichter commented 2 years ago

As a library to get things working while Node works on WebTransport in core, this project is great.

And that is the whole intention..... I wrote it since I did not want to wait and I also did not want to base my new project on old protocols... And I also do not mind if other developers step into the project and help to improve. And just need a working webtransport implementation for my avsrouter (https://github.com/fails-components/avsrouter) and client.

But anyway if it is datarate then one may try to look if the datarate depends on arraybuffer size, that could give a hint, where the bottleneck ist.

martenrichter commented 1 year ago

https://github.com/fails-components/webtransport/tree/byobreader

martenrichter commented 1 year ago

Support via byobreader using a shared memory buffer is coming. The new implementation has one additional memcopy, but does not malloc mem for every incoming buffer, and may reduce overhead.

martenrichter commented 1 year ago

https://github.com/fails-components/webtransport/commit/981d5aa6398233db12057552fbb74002bcc502d3

frabera commented 1 year ago

Awesome! Is it implemented also for datagrams?

martenrichter commented 1 year ago

No, the webtransport spec does not say anything about byobbuffer for datagrams. So for datagrams as before, a piece of memory is allocated for every datagram. However, one can think of using a shared buffer also for this transfer and use the javascript side for allocating memory.

martenrichter commented 1 year ago

I have digged a bit. It seems chrome implements byob for datagrams already, but they forget at first to change the spec, but the change is already document and will soon go into spec, so I should be able to add datagram support as well.

martenrichter commented 1 year ago

Basic implementation is on the way https://github.com/fails-components/webtransport/pull/186 more or less the same way Chrome does it, no it does not offer zero copy.