From d243115f417883f30ab7d7d459cf3fa3f235619a Mon Sep 17 00:00:00 2001
From: Yagiz Nizipli <yagiz@nizipli.com>
Date: Mon, 3 Apr 2023 11:59:46 -0400
Subject: [PATCH] url: improve URLSearchParams creation performance

PR-URL: https://github.com/nodejs/node/pull/47190
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
---
 lib/internal/url.js                           | 70 ++++++++++++-------
 ...twg-url-custom-searchparams-constructor.js |  4 ++
 2 files changed, 49 insertions(+), 25 deletions(-)

diff --git a/lib/internal/url.js b/lib/internal/url.js
index 147ab7f0164400..e89e02eb4bdf32 100644
--- a/lib/internal/url.js
+++ b/lib/internal/url.js
@@ -2,6 +2,7 @@
 
 const {
   Array,
+  ArrayIsArray,
   ArrayPrototypeJoin,
   ArrayPrototypeMap,
   ArrayPrototypePush,
@@ -156,13 +157,18 @@ function isURLSearchParams(self) {
 }
 
 class URLSearchParams {
+  [searchParams] = [];
+
+  // "associated url object"
+  [context] = null;
+
   // URL Standard says the default value is '', but as undefined and '' have
   // the same result, undefined is used to prevent unnecessary parsing.
   // Default parameter is necessary to keep URLSearchParams.length === 0 in
   // accordance with Web IDL spec.
   constructor(init = undefined) {
-    if (init === null || init === undefined) {
-      this[searchParams] = [];
+    if (init == null) {
+      // Do nothing
     } else if (typeof init === 'object' || typeof init === 'function') {
       const method = init[SymbolIterator];
       if (method === this[SymbolIterator]) {
@@ -170,38 +176,55 @@ class URLSearchParams {
         // shortcut to avoid having to go through the costly generic iterator.
         const childParams = init[searchParams];
         this[searchParams] = childParams.slice();
-      } else if (method !== null && method !== undefined) {
+      } else if (method != null) {
+        // Sequence<sequence<USVString>>
         if (typeof method !== 'function') {
           throw new ERR_ARG_NOT_ITERABLE('Query pairs');
         }
 
-        // Sequence<sequence<USVString>>
-        // Note: per spec we have to first exhaust the lists then process them
-        const pairs = [];
+        // The following implementationd differs from the URL specification:
+        // Sequences must first be converted from ECMAScript objects before
+        // and operations are done on them, and the operation of converting
+        // the sequences would first exhaust the iterators. If the iterator
+        // returns something invalid in the middle, whether it would be called
+        // after that would be an observable change to the users.
+        // Exhausting the iterator and later converting them to USVString comes
+        // with a significant cost (~40-80%). In order optimize URLSearchParams
+        // creation duration, Node.js merges the iteration and converting
+        // iterations into a single iteration.
         for (const pair of init) {
-          if ((typeof pair !== 'object' && typeof pair !== 'function') ||
-              pair === null ||
-              typeof pair[SymbolIterator] !== 'function') {
+          if (pair == null) {
             throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
-          }
-          const convertedPair = [];
-          for (const element of pair)
-            ArrayPrototypePush(convertedPair, toUSVString(element));
-          ArrayPrototypePush(pairs, convertedPair);
-        }
+          } else if (ArrayIsArray(pair)) {
+            // If innerSequence's size is not 2, then throw a TypeError.
+            if (pair.length !== 2) {
+              throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
+            }
+            // Append (innerSequence[0], innerSequence[1]) to querys list.
+            ArrayPrototypePush(this[searchParams], toUSVString(pair[0]), toUSVString(pair[1]));
+          } else {
+            if (((typeof pair !== 'object' && typeof pair !== 'function') ||
+                typeof pair[SymbolIterator] !== 'function')) {
+              throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
+            }
 
-        this[searchParams] = [];
-        for (const pair of pairs) {
-          if (pair.length !== 2) {
-            throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
+            let length = 0;
+
+            for (const element of pair) {
+              length++;
+              ArrayPrototypePush(this[searchParams], toUSVString(element));
+            }
+
+            // If innerSequence's size is not 2, then throw a TypeError.
+            if (length !== 2) {
+              throw new ERR_INVALID_TUPLE('Each query pair', '[name, value]');
+            }
           }
-          ArrayPrototypePush(this[searchParams], pair[0], pair[1]);
         }
       } else {
         // Record<USVString, USVString>
         // Need to use reflection APIs for full spec compliance.
         const visited = {};
-        this[searchParams] = [];
         const keys = ReflectOwnKeys(init);
         for (let i = 0; i < keys.length; i++) {
           const key = keys[i];
@@ -223,13 +246,10 @@ class URLSearchParams {
         }
       }
     } else {
-      // USVString
+      // https://url.spec.whatwg.org/#dom-urlsearchparams-urlsearchparams
       init = toUSVString(init);
       this[searchParams] = init ? parseParams(init) : [];
     }
-
-    // "associated url object"
-    this[context] = null;
   }
 
   [inspect.custom](recurseTimes, ctx) {
diff --git a/test/parallel/test-whatwg-url-custom-searchparams-constructor.js b/test/parallel/test-whatwg-url-custom-searchparams-constructor.js
index 878caed43ff0ab..1b7409680b2a2a 100644
--- a/test/parallel/test-whatwg-url-custom-searchparams-constructor.js
+++ b/test/parallel/test-whatwg-url-custom-searchparams-constructor.js
@@ -47,6 +47,10 @@ function makeIterableFunc(array) {
   assert.throws(() => new URLSearchParams([null]), tupleError);
   assert.throws(() => new URLSearchParams([{ [Symbol.iterator]: 42 }]),
                 tupleError);
+
+  assert.throws(() => new URLSearchParams(
+    makeIterableFunc([['key', 'val', 'val2']])
+  ), tupleError);
 }
 
 {