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: use class instead of object literals with getters #3138

Merged
merged 12 commits into from
Apr 21, 2024
8 changes: 8 additions & 0 deletions benchmarks/fetch/request-creation.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { bench, run } from 'mitata'
import { Request } from '../../lib/web/fetch/request.js'

const input = 'https://example.com/post'

bench('new Request(input)', () => new Request(input, undefined))

await run()
19 changes: 3 additions & 16 deletions lib/web/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const {
isValidHTTPToken,
sameOrigin,
normalizeMethod,
makePolicyContainer,
EnvironmentSettingsObject,
normalizeMethodRecord
} = require('./util')
const {
Expand All @@ -27,7 +27,6 @@ const {
const { kEnumerableProperty } = util
const { kHeaders, kSignal, kState, kGuard, kRealm, kDispatcher } = require('./symbols')
const { webidl } = require('./webidl')
const { getGlobalOrigin } = require('./global')
const { URLSerializer } = require('./data-url')
const { kHeadersList, kConstruct } = require('../../core/symbols')
const assert = require('node:assert')
Expand Down Expand Up @@ -55,16 +54,8 @@ class Request {
init = webidl.converters.RequestInit(init)

// https://html.spec.whatwg.org/multipage/webappapis.html#environment-settings-object
this[kRealm] = {
settingsObject: {
baseUrl: getGlobalOrigin(),
get origin () {
return this.baseUrl?.origin
},
policyContainer: makePolicyContainer()
}
}

// Note: Slow initialization of object literals with getters.
this[kRealm] = new EnvironmentSettingsObject()
// 1. Let request be null.
let request = null

Expand Down Expand Up @@ -366,7 +357,6 @@ class Request {
// (https://dom.spec.whatwg.org/#dom-abortsignal-any)
const ac = new AbortController()
this[kSignal] = ac.signal
this[kSignal][kRealm] = this[kRealm]

// 29. If signal is not null, then make this’s signal follow signal.
if (signal != null) {
Expand Down Expand Up @@ -436,7 +426,6 @@ class Request {
this[kHeaders] = new Headers(kConstruct)
this[kHeaders][kHeadersList] = request.headersList
this[kHeaders][kGuard] = 'request'
this[kHeaders][kRealm] = this[kRealm]

// 31. If this’s request’s mode is "no-cors", then:
if (mode === 'no-cors') {
Expand Down Expand Up @@ -881,11 +870,9 @@ function fromInnerRequest (innerRequest, signal, guard, realm) {
request[kState] = innerRequest
request[kRealm] = realm
request[kSignal] = signal
request[kSignal][kRealm] = realm
request[kHeaders] = new Headers(kConstruct)
request[kHeaders][kHeadersList] = innerRequest.headersList
request[kHeaders][kGuard] = guard
request[kHeaders][kRealm] = realm
return request
}

Expand Down
17 changes: 7 additions & 10 deletions lib/web/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ const {
isBlobLike,
serializeJavascriptValueToJSONString,
isErrorLike,
isomorphicEncode
isomorphicEncode,
EnvironmentSettingsObject
} = require('./util')
const {
redirectStatusSet,
Expand All @@ -21,7 +22,6 @@ const {
const { kState, kHeaders, kGuard, kRealm } = require('./symbols')
const { webidl } = require('./webidl')
const { FormData } = require('./formdata')
const { getGlobalOrigin } = require('./global')
const { URLSerializer } = require('./data-url')
const { kHeadersList, kConstruct } = require('../../core/symbols')
const assert = require('node:assert')
Expand All @@ -33,8 +33,7 @@ const textEncoder = new TextEncoder('utf-8')
class Response {
// Creates network error Response.
static error () {
// TODO
const relevantRealm = { settingsObject: {} }
const relevantRealm = new EnvironmentSettingsObject()

// The static error() method steps are to return the result of creating a
// Response object, given a new network error, "immutable", and this’s
Expand Down Expand Up @@ -62,7 +61,7 @@ class Response {

// 3. Let responseObject be the result of creating a Response object, given a new response,
// "response", and this’s relevant Realm.
const relevantRealm = { settingsObject: {} }
const relevantRealm = new EnvironmentSettingsObject()
const responseObject = fromInnerResponse(makeResponse({}), 'response', relevantRealm)

// 4. Perform initialize a response given responseObject, init, and (body, "application/json").
Expand All @@ -74,7 +73,7 @@ class Response {

// Creates a redirect Response that redirects to url with status status.
static redirect (url, status = 302) {
const relevantRealm = { settingsObject: {} }
const relevantRealm = new EnvironmentSettingsObject()

webidl.argumentLengthCheck(arguments, 1, { header: 'Response.redirect' })

Expand All @@ -87,7 +86,7 @@ class Response {
// TODO: base-URL?
let parsedURL
try {
parsedURL = new URL(url, getGlobalOrigin())
parsedURL = new URL(url, relevantRealm.settingsObject.baseUrl)
} catch (err) {
throw new TypeError(`Failed to parse URL from ${url}`, { cause: err })
}
Expand Down Expand Up @@ -127,7 +126,7 @@ class Response {
init = webidl.converters.ResponseInit(init)

// TODO
this[kRealm] = { settingsObject: {} }
this[kRealm] = new EnvironmentSettingsObject()

// 1. Set this’s response to a new response.
this[kState] = makeResponse({})
Expand All @@ -138,7 +137,6 @@ class Response {
this[kHeaders] = new Headers(kConstruct)
this[kHeaders][kGuard] = 'response'
this[kHeaders][kHeadersList] = this[kState].headersList
this[kHeaders][kRealm] = this[kRealm]

// 3. Let bodyWithType be null.
let bodyWithType = null
Expand Down Expand Up @@ -522,7 +520,6 @@ function fromInnerResponse (innerResponse, guard, realm) {
response[kHeaders] = new Headers(kConstruct)
response[kHeaders][kHeadersList] = innerResponse.headersList
response[kHeaders][kGuard] = guard
response[kHeaders][kRealm] = realm
return response
}

Expand Down
25 changes: 24 additions & 1 deletion lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,28 @@ function utf8DecodeBytes (buffer) {
return output
}

class EnvironmentSettingsObjectBase {
#baseUrl

get baseUrl () {
return (this.#baseUrl ??= getGlobalOrigin())
}

set baseUrl (baseUrl) {
this.#baseUrl = baseUrl
}
tsctx marked this conversation as resolved.
Show resolved Hide resolved

get origin () {
return this.baseUrl?.origin
}

policyContainer = makePolicyContainer()
}

class EnvironmentSettingsObject {
settingsObject = new EnvironmentSettingsObjectBase()
}

module.exports = {
isAborted,
isCancelled,
Expand Down Expand Up @@ -1620,5 +1642,6 @@ module.exports = {
createInflate,
extractMimeType,
getDecodeSplit,
utf8DecodeBytes
utf8DecodeBytes,
EnvironmentSettingsObject
}
2 changes: 0 additions & 2 deletions test/fetch/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,6 @@ test('fromInnerRequest', () => {
assert.strictEqual(request[kState], innerRequest)
assert.strictEqual(request[kRealm], realm)
assert.strictEqual(request[kSignal], signal)
assert.strictEqual(request[kSignal][kRealm], realm)
assert.strictEqual(request[kHeaders][kHeadersList], innerRequest.headersList)
assert.strictEqual(request[kHeaders][kGuard], 'immutable')
assert.strictEqual(request[kHeaders][kRealm], realm)
})
1 change: 0 additions & 1 deletion test/fetch/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -286,5 +286,4 @@ test('fromInnerResponse', () => {
assert.strictEqual(response[kRealm], realm)
assert.strictEqual(response[kHeaders][kHeadersList], innerResponse.headersList)
assert.strictEqual(response[kHeaders][kGuard], 'immutable')
assert.strictEqual(response[kHeaders][kRealm], realm)
})