Skip to content

Commit

Permalink
perf: use class instead of object literals with getters (#3138)
Browse files Browse the repository at this point in the history
* perf: use class instead of object literals with getters

* may not be needed

* update

* remove realm from headers and signal

* use class in Response

* fix lint

* Update lib/web/fetch/request.js

* use shared realm

* fix lint

* use Environment Settings Object baseUrl

* Update lib/web/fetch/util.js

* Update lib/web/fetch/util.js
  • Loading branch information
tsctx committed Apr 21, 2024
1 parent 437732a commit ad9b5bd
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 30 deletions.
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
17 changes: 16 additions & 1 deletion lib/web/fetch/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,20 @@ function utf8DecodeBytes (buffer) {
return output
}

class EnvironmentSettingsObjectBase {
baseUrl = getGlobalOrigin()

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

policyContainer = makePolicyContainer()
}

class EnvironmentSettingsObject {
settingsObject = new EnvironmentSettingsObjectBase()
}

module.exports = {
isAborted,
isCancelled,
Expand Down Expand Up @@ -1620,5 +1634,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)
})

0 comments on commit ad9b5bd

Please sign in to comment.