Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: avoid unnecessary clone #3117

Merged
merged 2 commits into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions benchmarks/fetch/body-arraybuffer.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { group, bench, run } from 'mitata'
import { Response } from '../../lib/web/fetch/response.js'

const settings = {
small: 2 << 8,
middle: 2 << 12,
long: 2 << 16
}

for (const [name, length] of Object.entries(settings)) {
const buffer = Buffer.allocUnsafe(length).map(() => (Math.random() * 100) | 0)
group(`${name} (length ${length})`, () => {
bench('Response#arrayBuffer', async () => {
return await new Response(buffer).arrayBuffer()
})

// for comparison
bench('Response#text', async () => {
return await new Response(buffer).text()
})
})
}

await run()
20 changes: 11 additions & 9 deletions lib/web/fetch/body.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ function bodyMixinMethods (instance) {
// Return a Blob whose contents are bytes and type attribute
// is mimeType.
return new Blob([bytes], { type: mimeType })
}, instance)
}, instance, false)
},

arrayBuffer () {
Expand All @@ -318,20 +318,21 @@ function bodyMixinMethods (instance) {
// given a byte sequence bytes: return a new ArrayBuffer
// whose contents are bytes.
return consumeBody(this, (bytes) => {
return new Uint8Array(bytes).buffer
}, instance)
// Note: arrayBuffer already cloned.
return bytes.buffer
}, instance, true)
},

text () {
// The text() method steps are to return the result of running
// consume body with this and UTF-8 decode.
return consumeBody(this, utf8DecodeBytes, instance)
return consumeBody(this, utf8DecodeBytes, instance, false)
},

json () {
// The json() method steps are to return the result of running
// consume body with this and parse JSON from bytes.
return consumeBody(this, parseJSONFromBytes, instance)
return consumeBody(this, parseJSONFromBytes, instance, false)
},

formData () {
Expand Down Expand Up @@ -383,7 +384,7 @@ function bodyMixinMethods (instance) {
throw new TypeError(
'Content-Type was not one of "multipart/form-data" or "application/x-www-form-urlencoded".'
)
}, instance)
}, instance, false)
}
}

Expand All @@ -399,8 +400,9 @@ function mixinBody (prototype) {
* @param {Response|Request} object
* @param {(value: unknown) => unknown} convertBytesToJSValue
* @param {Response|Request} instance
* @param {boolean} [shouldClone]
*/
async function consumeBody (object, convertBytesToJSValue, instance) {
async function consumeBody (object, convertBytesToJSValue, instance, shouldClone) {
webidl.brandCheck(object, instance)

// 1. If object is unusable, then return a promise rejected
Expand Down Expand Up @@ -432,13 +434,13 @@ async function consumeBody (object, convertBytesToJSValue, instance) {
// 5. If object’s body is null, then run successSteps with an
// empty byte sequence.
if (object[kState].body == null) {
successSteps(new Uint8Array())
successSteps(Buffer.allocUnsafe(0))
return promise.promise
}

// 6. Otherwise, fully read object’s body given successSteps,
// errorSteps, and object’s relevant global object.
await fullyReadBody(object[kState].body, successSteps, errorSteps)
await fullyReadBody(object[kState].body, successSteps, errorSteps, shouldClone)

// 7. Return promise.
return promise.promise
Expand Down
15 changes: 11 additions & 4 deletions lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1043,7 +1043,7 @@ function iteratorMixin (name, object, kInternalIterator, keyIndex = 0, valueInde
/**
* @see https://fetch.spec.whatwg.org/#body-fully-read
*/
async function fullyReadBody (body, processBody, processBodyError) {
async function fullyReadBody (body, processBody, processBodyError, shouldClone) {
// 1. If taskDestination is null, then set taskDestination to
// the result of starting a new parallel queue.

Expand All @@ -1069,8 +1069,7 @@ async function fullyReadBody (body, processBody, processBodyError) {

// 5. Read all bytes from reader, given successSteps and errorSteps.
try {
const result = await readAllBytes(reader)
successSteps(result)
successSteps(await readAllBytes(reader, shouldClone))
} catch (e) {
errorSteps(e)
}
Expand Down Expand Up @@ -1118,8 +1117,9 @@ function isomorphicEncode (input) {
* @see https://streams.spec.whatwg.org/#readablestreamdefaultreader-read-all-bytes
* @see https://streams.spec.whatwg.org/#read-loop
* @param {ReadableStreamDefaultReader} reader
* @param {boolean} [shouldClone]
*/
async function readAllBytes (reader) {
async function readAllBytes (reader, shouldClone) {
const bytes = []
let byteLength = 0

Expand All @@ -1128,6 +1128,13 @@ async function readAllBytes (reader) {

if (done) {
// 1. Call successSteps with bytes.
if (bytes.length === 1) {
const { buffer, byteOffset, byteLength } = bytes[0]
if (shouldClone === false) {
return Buffer.from(buffer, byteOffset, byteLength)
}
return Buffer.from(buffer.slice(byteOffset, byteOffset + byteLength), 0, byteLength)
}
return Buffer.concat(bytes, byteLength)
}

Expand Down
Loading