nodejs / undici

An HTTP/1.1 client, written from scratch for Node.js
https://nodejs.github.io/undici
MIT License
6.34k stars 555 forks source link

Reduce Buffer operations in EventSource #2630

Open Uzlopak opened 10 months ago

Uzlopak commented 10 months ago

In the EventSourceStream code of the initial EventSource implementation, alot of Buffer Operations are used to process the incoming Stream. The Buffer operations are concentrated in the _transform and parseLine method of the EventSourceStream.

I am sure, we can reduce the overhead of the buffer operations by using some simple integer variables, marking start and end of the Buffer we want to extract the subarray and then pass it accordingly parseLine method.

Maybe the Buffer.concat operation to buffer the incoming chunks can be avoided. But be careful, as the incoming stream is containing utf8 code. So you have to concat the buffers at one point to avoid utf8 issues.

In the parseLine method the field and value are also processed via .subarray. Maybe first calling toString('utf8') and then processing it makes more sense? Anyhow, some benchmarking would be useful to make a informed decision.

KhafraDev commented 10 months ago

In websocket I was inspired by lpinca/ws to add in a utility that returned a buffer that contained n bytes, rather than concatenating every buffer stored. https://github.com/nodejs/undici/commit/5165d6712088c41ebbf66cf71ba856924008e899

Uzlopak commented 10 months ago

@KhafraDev

Maybe use similar logic as in eventsource npm package? Especially https://github.com/EventSource/eventsource/pull/239

KhafraDev commented 10 months ago

It looks as though they behave similarly, with the benefit of ws' approach being able to lazily set the buffers and allocate an exact buffer size. Both would have to be benchmarked before I could give a definitive answer.

metcoder95 commented 10 months ago

The solution from the PR seems to be of help for the usage of concat, but a way to iterate over the buffer to consume it while avoiding subarrays might require the approach @KhafraDev suggests