feat(streams): limit usage in body mixin #2164#5010
feat(streams): limit usage in body mixin #2164#5010rozzilla wants to merge 1 commit intonodejs:mainfrom
Conversation
Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5010 +/- ##
==========================================
- Coverage 92.93% 92.93% -0.01%
==========================================
Files 110 110
Lines 35739 35755 +16
==========================================
+ Hits 33214 33228 +14
- Misses 2525 2527 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
KhafraDev
left a comment
There was a problem hiding this comment.
The issue with optimizations like these (if it's even more performant, I'm not sure that it is) is that it encourages people to only use string or Uint8Array bodies to squeeze more performance out of node. I don't want to recreate CrankshaftScript.
|
How much is more performant? I think that in most cases, those optimizations are good for Node.js. |
|
It wouldn't matter. The implementation is incorrect as it doesn't perform the same validation readAllBytes does (ensuring the result of reader.read() is a Uint8Array). After that I'm not sure, 1 less promise, for the extra maintenance burden and guaranteed bugs there will be? |
This relates to...
Fixes #2164
Rationale
When a
ResponseorRequestbody is created from astringorUint8Array, the body mixin methods (text(), json(), arrayBuffer(), blob(), bytes()) currently pipe the source through aReadableStreameven though the bytes are already available inbody.source. This adds unnecessary overhead from stream construction, pump microtasks, and chunk collection.Changes
consumeBody(), before callingfullyReadBody(), check ifbody.sourceis astringorUint8Array. If so, bypass the stream round-trip and pass the source bytes directly tosuccessStepsstringsources (all 5 body mixin methods, Request, multibyte, empty),Uint8Arraysources (text, arrayBuffer, bytes, blob), body unusability after consumption (bodyUsed, double-read rejection), and non-shortcuttable sources (ReadableStream, Blob, null) to ensure the normal path is unaffected.Features
N/A
Bug Fixes
N/A
Breaking Changes and Deprecations
N/A
Status