Skip to content

Commit 834cef8

Browse files
authored
Implement faster trimming of control characters
Due to a V8 bug (https://issues.chromium.org/issues/42204424), negative end-of-string matches are slow, as shown in #286. We can avoid triggering this slowdown for certain inputs by just implementing the trimming using loops. Closes #286. Closes #288 by superseding it.
1 parent 807353d commit 834cef8

4 files changed

Lines changed: 41 additions & 7 deletions

File tree

eslint.config.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export default [
44
{
55
ignores: [
66
"coverage/",
7+
"_site/",
78
"test/web-platform-tests/",
89
"live-viewer/whatwg-url.mjs",
910
"lib/VoidFunction.js",

lib/url-state-machine.js

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,22 @@ function domainToASCII(domain, beStrict = false) {
441441
return result;
442442
}
443443

444-
function trimControlChars(url) {
445-
return url.replace(/^[\u0000-\u001F\u0020]+|[\u0000-\u001F\u0020]+$/ug, "");
444+
function trimControlChars(string) {
445+
// Avoid using regexp because of this V8 bug: https://issues.chromium.org/issues/42204424
446+
447+
let start = 0;
448+
let end = string.length;
449+
for (; start < end; ++start) {
450+
if (string.charCodeAt(start) > 0x20) {
451+
break;
452+
}
453+
}
454+
for (; end > start; --end) {
455+
if (string.charCodeAt(end - 1) > 0x20) {
456+
break;
457+
}
458+
}
459+
return string.substring(start, end);
446460
}
447461

448462
function trimTabAndNewline(url) {

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@
3333
"prepare": "node scripts/transform.js",
3434
"pretest": "node scripts/get-latest-platform-tests.js && node scripts/transform.js",
3535
"build-live-viewer": "esbuild --bundle --format=esm --sourcemap --outfile=live-viewer/whatwg-url.mjs index.js",
36-
"test": "node --test test/*.js"
36+
"test": "node --test test/*.js",
37+
"bench": "node scripts/benchmark.js"
3738
},
3839
"c8": {
3940
"reporter": [

scripts/benchmark.js

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
"use strict";
22
const { URL } = require("../");
33
const Benchmark = require("benchmark");
4-
const testData = require("../test/web-platform-tests/resources/urltestdata.json");
54

5+
const testData = require("../test/web-platform-tests/resources/urltestdata.json");
66
const testInputs = testData.filter(c => typeof c === "object").map(c => c.input);
77

8-
const benchmark = new Benchmark(() => {
8+
runBenchmark("URL constructor with WPT data", () => {
99
for (const input of testInputs) {
1010
try {
1111
// eslint-disable-next-line no-new
@@ -16,5 +16,23 @@ const benchmark = new Benchmark(() => {
1616
}
1717
});
1818

19-
benchmark.on("cycle", e => console.log(e.target.toString()));
20-
benchmark.run();
19+
runBenchmark("long input not starting or ending with control characters (GH-286)", () => {
20+
try {
21+
// eslint-disable-next-line no-new
22+
new URL(`!!${"\u0000".repeat(100000)}A\rA`);
23+
} catch {
24+
// intentionally empty
25+
}
26+
});
27+
28+
function runBenchmark(name, fn) {
29+
new Benchmark(name, fn, {
30+
onComplete(event) {
31+
console.log(`${name}:`);
32+
console.log(` ${event.target.hz.toFixed(0)} ops/second`);
33+
console.log(` ±${event.target.stats.rme.toFixed(2)}% relative margin of error`);
34+
console.log(` ${event.target.stats.sample.length} samples`);
35+
console.log("");
36+
}
37+
}).run();
38+
}

0 commit comments

Comments
 (0)