Repository navigation
http: merge chunks during chunked encoding - #47630
gjgmenendez wants to merge 4 commits into
Conversation
|
Review requested:
|
nodejs-github-bot
added
http
|
What is the context for this? |
tniessen
added
the
performance
| ); | ||
| } | ||
|
|
||
| let chunkBuffer = Buffer.alloc(0); |
There was a problem hiding this comment.
Having this as a global variable will cause concurrent requests to interleave and become corrupt. The buffer needs to be scoped to the response object itself.
| msg._send(crlf_buf, null, null); | ||
| msg._send(chunk, encoding, null, len); | ||
| ret = msg._send(crlf_buf, null, callback); | ||
| chunkBuffer = Buffer.concat([chunkBuffer, Buffer.from(chunk)]); |
There was a problem hiding this comment.
Concatenating buffers is very slow. They require a memory allocation + memory copy. Just keep an Array of buffers.
| if (!this._headerSent && Buffer.byteLength(chunkBuffer)) { | ||
| this._send(NumberPrototypeToString(Buffer.byteLength(chunkBuffer), 16), 'latin1', null); | ||
| this._send(crlf_buf, null, null); | ||
| this._send(chunkBuffer, null, null); | ||
| this._send(crlf_buf, null, callback); | ||
| chunkBuffer = Buffer.alloc(0) | ||
| } |
There was a problem hiding this comment.
This should be done in nextTick (i.e., connectionCorkNT) otherwise, there is no point to all of this... the number of buffered chunks would always be 1.
| this.socket.uncork(); | ||
| } | ||
| this[kCorked] = 0; | ||
|
|
| }) | ||
| ); | ||
|
|
||
| server.listen(3000, () => {}); |
There was a problem hiding this comment.
This doesn't test anything related to the problem we are trying to solve.
|
This pull request has been marked as stale due to 90 days of inactivity. |
github-actions
Bot
added
the
stale
Refs: nodejs/performance#57