Skip to content

Commit cb5d1cc

Browse files
anonrigdanielleadams
authored andcommitted
url: improve URLSearchParams creation performance
PR-URL: #47190 Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
1 parent 66de3ba commit cb5d1cc

File tree

2 files changed

+49
-25
lines changed

2 files changed

+49
-25
lines changed

lib/internal/url.js

+45-25
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
const {
44
Array,
5+
ArrayIsArray,
56
ArrayPrototypeJoin,
67
ArrayPrototypeMap,
78
ArrayPrototypePush,
@@ -157,52 +158,74 @@ function isURLSearchParams(self) {
157158
}
158159

159160
class URLSearchParams {
161+
[searchParams] = [];
162+
163+
// "associated url object"
164+
[context] = null;
165+
160166
// URL Standard says the default value is '', but as undefined and '' have
161167
// the same result, undefined is used to prevent unnecessary parsing.
162168
// Default parameter is necessary to keep URLSearchParams.length === 0 in
163169
// accordance with Web IDL spec.
164170
constructor(init = undefined) {
165-
if (init === null || init === undefined) {
166-
this[searchParams] = [];
171+
if (init == null) {
172+
// Do nothing
167173
} else if (typeof init === 'object' || typeof init === 'function') {
168174
const method = init[SymbolIterator];
169175
if (method === this[SymbolIterator]) {
170176
// While the spec does not have this branch, we can use it as a
171177
// shortcut to avoid having to go through the costly generic iterator.
172178
const childParams = init[searchParams];
173179
this[searchParams] = childParams.slice();
174-
} else if (method !== null && method !== undefined) {
180+
} else if (method != null) {
181+
// Sequence<sequence<USVString>>
175182
if (typeof method !== 'function') {
176183
throw new ERR_ARG_NOT_ITERABLE('Query pairs');
177184
}
178185

179-
// Sequence<sequence<USVString>>
180-
// Note: per spec we have to first exhaust the lists then process them
181-
const pairs = [];
186+
// The following implementationd differs from the URL specification:
187+
// Sequences must first be converted from ECMAScript objects before
188+
// and operations are done on them, and the operation of converting
189+
// the sequences would first exhaust the iterators. If the iterator
190+
// returns something invalid in the middle, whether it would be called
191+
// after that would be an observable change to the users.
192+
// Exhausting the iterator and later converting them to USVString comes
193+
// with a significant cost (~40-80%). In order optimize URLSearchParams
194+
// creation duration, Node.js merges the iteration and converting
195+
// iterations into a single iteration.
182196
for (const pair of init) {
183-
if ((typeof pair !== 'object' && typeof pair !== 'function') ||
184-
pair === null ||
185-
typeof pair[SymbolIterator] !== 'function') {
197+
if (pair == null) {
186198
throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
187-
}
188-
const convertedPair = [];
189-
for (const element of pair)
190-
ArrayPrototypePush(convertedPair, toUSVString(element));
191-
ArrayPrototypePush(pairs, convertedPair);
192-
}
199+
} else if (ArrayIsArray(pair)) {
200+
// If innerSequence's size is not 2, then throw a TypeError.
201+
if (pair.length !== 2) {
202+
throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
203+
}
204+
// Append (innerSequence[0], innerSequence[1]) to querys list.
205+
ArrayPrototypePush(this[searchParams], toUSVString(pair[0]), toUSVString(pair[1]));
206+
} else {
207+
if (((typeof pair !== 'object' && typeof pair !== 'function') ||
208+
typeof pair[SymbolIterator] !== 'function')) {
209+
throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
210+
}
193211

194-
this[searchParams] = [];
195-
for (const pair of pairs) {
196-
if (pair.length !== 2) {
197-
throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
212+
let length = 0;
213+
214+
for (const element of pair) {
215+
length++;
216+
ArrayPrototypePush(this[searchParams], toUSVString(element));
217+
}
218+
219+
// If innerSequence's size is not 2, then throw a TypeError.
220+
if (length !== 2) {
221+
throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
222+
}
198223
}
199-
ArrayPrototypePush(this[searchParams], pair[0], pair[1]);
200224
}
201225
} else {
202226
// Record<USVString, USVString>
203227
// Need to use reflection APIs for full spec compliance.
204228
const visited = {};
205-
this[searchParams] = [];
206229
const keys = ReflectOwnKeys(init);
207230
for (let i = 0; i < keys.length; i++) {
208231
const key = keys[i];
@@ -224,13 +247,10 @@ class URLSearchParams {
224247
}
225248
}
226249
} else {
227-
// USVString
250+
// https://url.spec.whatwg.org/#dom-urlsearchparams-urlsearchparams
228251
init = toUSVString(init);
229252
initSearchParams(this, init);
230253
}
231-
232-
// "associated url object"
233-
this[context] = null;
234254
}
235255

236256
[inspect.custom](recurseTimes, ctx) {

test/parallel/test-whatwg-url-custom-searchparams-constructor.js

+4
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ function makeIterableFunc(array) {
4747
assert.throws(() => new URLSearchParams([null]), tupleError);
4848
assert.throws(() => new URLSearchParams([{ [Symbol.iterator]: 42 }]),
4949
tupleError);
50+
51+
assert.throws(() => new URLSearchParams(
52+
makeIterableFunc([['key', 'val', 'val2']])
53+
), tupleError);
5054
}
5155

5256
{

0 commit comments

Comments
 (0)