nodejs/undici

fetch: limit web streams usage in body mixin methods

Open

#2164 opened on Jun 18, 2023

View on GitHub
 (9 comments) (0 reactions) (0 assignees)JavaScript (5,319 stars) (444 forks)batch import
good first issue

Description

The fetch spec requires us to create a new web Readable stream for every Response and Request object, which can likely be avoided in most cases.

Given the following examples:

new Response('abc')
new Response(new TextEncoder().encode('abc'))
new Response(new Blob(['abc']))
// etc.

A web stream is created and then converted back to a string (or blob, arraybuffer, etc.)! The following diff improves .text() performance when the body passed is a string by 10%, as a simple demo (we could further limit web stream usage for more performance gains!!!).

diff --git a/lib/fetch/response.js b/lib/fetch/response.js
index 1029dbef..c2d2bd3d 100644
--- a/lib/fetch/response.js
+++ b/lib/fetch/response.js
@@ -154,7 +154,7 @@ class Response {
     // 4. If body is non-null, then set bodyWithType to the result of extracting body.
     if (body != null) {
       const [extractedBody, type] = extractBody(body)
-      bodyWithType = { body: extractedBody, type }
+      bodyWithType = { body: extractedBody, type, source: body }
     }

     // 5. Perform initialize a response given this, init, and bodyWithType.
diff --git a/lib/fetch/util.js b/lib/fetch/util.js
index 400687ba..9e4302a9 100644
--- a/lib/fetch/util.js
+++ b/lib/fetch/util.js
@@ -824,6 +824,11 @@ function fullyReadBody (body, processBody, processBodyError) {
   //    with taskDestination.
   const errorSteps = (error) => queueMicrotask(() => processBodyError(error))

+  if (typeof body.source === 'string') {
+    successSteps(new TextEncoder().encode(body.source))
+    return
+  }
+
   // 4. Let reader be the result of getting a reader for body’s stream.
   //    If that threw an exception, then run errorSteps with that
   //    exception and return.

cc @anonrig

Contributor guide