Skip to content

Commit 7afdfdd

Browse files
perf(javascript): read a regexp repeating a capture group name in linear time
Validating a named capture group compares its branch against every branch already recorded for that name, and acorn compares two branches by walking one chain per node of the other — quadratic in the nesting depth, per pair. A pattern of 160 alternatives each nesting a same-named group 160 deep took 7.4s where the same pattern with distinct names took 0.09s. A branch chain holds at most one branch per disjunction, so marking each disjunction the first chain passed answers the comparison in one walk of the second. The same pattern now validates in 0.12s (acornjs/acorn#1445).
1 parent 5144997 commit 7afdfdd

5 files changed

Lines changed: 91 additions & 6 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Read a regular expression repeating a capture group name in linear time.

‎cspell.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
"absolutify",
99
"accesskey",
1010
"acircular",
11+
"acornjs",
1112
"akait",
1213
"amaro",
1314
"amdmodule",

‎lib/javascript/regexp.js‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ const hasProp = (obj) => {
3434
return false;
3535
};
3636

37+
// Which comparison last marked a disjunction, so that a walk's marks are
38+
// told apart from the previous walk's without clearing them.
39+
let branchWalk = 0;
40+
3741
/**
3842
* Which disjunction branch a named group sits in, so that the same name in two
3943
* alternatives of one disjunction is allowed and a repeat in one is not.
@@ -47,24 +51,41 @@ class BranchID {
4751
this.parent = parent;
4852
/** @type {BranchID} */
4953
this.base = base || this;
54+
// which walk last marked this disjunction, and with which of its branches
55+
this.walk = 0;
56+
/** @type {BranchID | null} */
57+
this.taken = null;
5058
}
5159

5260
/**
61+
* Two branches are separated when some disjunction they both sit under took
62+
* a different alternative for each, so nothing can match both.
5363
* @param {BranchID} alt the other branch
5464
* @returns {boolean} whether the two are alternatives of one disjunction
5565
*/
5666
separatedFrom(alt) {
67+
// WHY: a chain holds at most one branch per disjunction — siblings share
68+
// a parent, so a chain takes one of them — so marking each disjunction
69+
// this chain passed answers the whole comparison in one walk of the
70+
// other. acorn walks one chain per node of the other, which is quadratic
71+
// in the nesting depth and re-walked per same-named group (acorn#1445).
72+
const walk = ++branchWalk;
5773
/** @type {BranchID | null} */
5874
let self = this;
5975
while (self) {
60-
/** @type {BranchID | null} */
61-
let other = alt;
62-
while (other) {
63-
if (self.base === other.base && self !== other) return true;
64-
other = other.parent;
65-
}
76+
self.base.walk = walk;
77+
self.base.taken = self;
6678
self = self.parent;
6779
}
80+
/** @type {BranchID | null} */
81+
let other = alt;
82+
while (other) {
83+
const disjunction = /** @type {BranchID} */ (other.base);
84+
if (disjunction.walk === walk && disjunction.taken !== other) {
85+
return true;
86+
}
87+
other = other.parent;
88+
}
6889
return false;
6990
}
7091

‎test/unitCases/RegExpValidator.unittest.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ const PATTERNS = [
4949
["(?:(?<x>a)|(?<x>b))\\k<x>", "u"],
5050
["(?<x>a)(?:(?<x>b))", "u"],
5151
["(?<x>a)(?:(?<x>b)|c)", "u"],
52+
// a name repeated across nested disjunctions, where the two are separated
53+
// only when one alternative of one disjunction rules the other out
54+
["(?:(?<x>a)|b)(?:(?<x>c)|d)", "u"],
55+
["(?:(?:(?<x>a)|b)|(?:(?<x>c)|d))", "u"],
56+
["(?:(?<x>a)|(?:(?<x>b)|c))", "u"],
57+
["(?:(?<x>a)|(?:b(?<x>c)))|(?<x>d)", "u"],
58+
["(?:(?:(?:(?<x>a))))|(?:(?:(?:(?<x>b))))", "u"],
59+
["(?:(?:(?:(?<x>a))))(?:(?:(?:(?<x>b))))", "u"],
5260
["(?<n>a)\\k<m>", ""],
5361
["(?<n>a)\\k<m>", "u"],
5462
["\\k<n>", ""],
@@ -351,4 +359,47 @@ describe("RegExpValidator", () => {
351359
/Unterminated group/
352360
);
353361
});
362+
363+
it("reads a name repeated across alternatives at the cost of distinct ones", () => {
364+
// Both patterns are legal and the same size; only the duplicate one asks
365+
// which alternatives rule each other out, which acorn answers by walking
366+
// one branch chain per node of the other (acornjs/acorn#1445).
367+
/**
368+
* @param {number} depth how many disjunctions enclose the group
369+
* @param {string} name the group's name
370+
* @returns {string} one named group, nested that deep
371+
*/
372+
const nest = (depth, name) =>
373+
`${"(?:".repeat(depth)}(?<${name}>a)${")".repeat(depth)}`;
374+
375+
/**
376+
* @param {number} count how many alternatives to write
377+
* @param {number} depth how deep each one nests its group
378+
* @param {boolean} duplicate whether every group carries the same name
379+
* @returns {string} the program holding that regexp literal
380+
*/
381+
const pattern = (count, depth, duplicate) =>
382+
`/${Array.from({ length: count }, (_, i) =>
383+
nest(depth, duplicate ? "x" : `x${i}`)
384+
).join("|")}/v;`;
385+
386+
/**
387+
* @param {string} source the program to parse
388+
* @returns {number} the fastest of three parses, in nanoseconds
389+
*/
390+
const best = (source) => {
391+
let fastest = Infinity;
392+
for (let run = 0; run < 3; run++) {
393+
const started = process.hrtime.bigint();
394+
Parser.parse(source, { ecmaVersion: "latest" });
395+
fastest = Math.min(fastest, Number(process.hrtime.bigint() - started));
396+
}
397+
return fastest;
398+
};
399+
const distinct = best(pattern(160, 160, false));
400+
const repeated = best(pattern(160, 160, true));
401+
// The two were ~80x apart before the rewrite and are within noise of each
402+
// other after it, so the bound only has to catch the quadratic walk.
403+
expect(repeated).toBeLessThan(distinct * 10);
404+
});
354405
});

‎types.d.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,6 +1270,13 @@ declare interface Bootstrap {
12701270
declare abstract class BranchID {
12711271
parent: null | BranchID;
12721272
base: BranchID;
1273+
walk: number;
1274+
taken: null | BranchID;
1275+
1276+
/**
1277+
* Two branches are separated when some disjunction they both sit under took
1278+
* a different alternative for each, so nothing can match both.
1279+
*/
12731280
separatedFrom(alt: BranchID): boolean;
12741281
sibling(): BranchID;
12751282
}

0 commit comments

Comments
 (0)