Skip to content

Commit c567572

Browse files
dmitriplotnikovcopybara-github
dmitriplotnikov
authored and
copybara-github
committed
Iteratively parse nested parentheses in PrattParser
Replace the lookahead-based grouping parentheses counter (`countGroupingParentheses`) with a bottom-up iterative approach in `PrattParser.parsePrimary()` to eliminate quadratic $O(N^2)$ lookahead scans while keeping $O(1)$ call-stack frames on deeply nested parenthesized expressions. ### Benchmarks (`CelParserBenchmark`, `-c opt`, `parserType=PRATT`, `parseOnly`) | Benchmark Case | Before (Mean) | After (Mean) | Change / Speedup | | :--- | ---: | ---: | ---: | | `NESTED_PARENS` (N=200) | 8,528.69 ns | 5,119.12 ns | -40.0% (1.67x faster) | | `NESTED_LEFT_PARENS_CALC` (N=200) | 951,027.39 ns | 27,998.44 ns | -97.1% (34.0x faster) | | Benchmark Case (Allocations) | Before (Bytes / Objs) | After (Bytes / Objs) | Change | | :--- | ---: | ---: | ---: | | `NESTED_PARENS` (N=200) | 29,200 B / 855 objs | 16,336 B / 453 objs | -44.1% B / -47.0% objs | | `NESTED_LEFT_PARENS_CALC` (N=200) | ~2,690,000 B / ~84,500 objs | 127,016 B / 4,577 objs | -95.3% B / -94.6% objs | PiperOrigin-RevId: 984184714
1 parent 491a179 commit c567572

4 files changed

Lines changed: 50 additions & 98 deletions

File tree

‎parser/src/main/java/dev/cel/parser/PrattParser.java‎

Lines changed: 38 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ private boolean expect(Lexer.TokenType type, String msg) {
253253
nextToken();
254254
return true;
255255
}
256-
if (isRecoveryLimitExceeded()) {
256+
if (recursionLimitExceeded || isRecoveryLimitExceeded()) {
257257
return false;
258258
}
259259
if (peekToken.type != Lexer.TokenType.ERROR) {
@@ -274,7 +274,7 @@ private boolean expect(Lexer.TokenType type, String msg) {
274274

275275
// Find the next delimiter to prevent a cascade of spurious secondary errors.
276276
private void synchronizeOnDelimiter() {
277-
if (isRecoveryLimitExceeded()) {
277+
if (recursionLimitExceeded || isRecoveryLimitExceeded()) {
278278
peekToken = END_TOKEN;
279279
return;
280280
}
@@ -412,6 +412,7 @@ private void reportRecursionLimit(int position) {
412412
LOCALE,
413413
"Expression recursion limit exceeded. limit: %d",
414414
options.maxParseRecursionDepth()));
415+
peekToken = END_TOKEN;
415416
}
416417
}
417418

@@ -429,11 +430,14 @@ private CelExpr parseExpr() {
429430
return expr;
430431
}
431432

432-
@SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table
433433
private CelExpr parseBinaryAndTernary(int minPrec) {
434-
CelExpr lhs = parseSelectorChain();
434+
return parseBinaryAndTernaryFromLhs(parseSelectorChain(), minPrec);
435+
}
436+
437+
@SuppressWarnings("EnumOrdinal") // Using ordinal for O(1) binary operator lookup table
438+
private CelExpr parseBinaryAndTernaryFromLhs(CelExpr lhs, int minPrec) {
435439
int chainDepth = currentLhsDepth;
436-
while (true) {
440+
while (!recursionLimitExceeded && !isRecoveryLimitExceeded()) {
437441
Lexer.TokenType tok = peekToken.type;
438442
if (tok == Lexer.TokenType.QUESTION && minPrec <= 0) {
439443
lhs = parseTernary(lhs);
@@ -771,16 +775,39 @@ private CelExpr parsePrimary() {
771775
switch (peekToken.type) {
772776
case LEFT_PAREN:
773777
{
774-
int groupingParenCount = countGroupingParentheses();
775-
if (checkRecursion(groupingParenCount, peekToken)) {
778+
if (recursionLimitExceeded || isRecoveryLimitExceeded()) {
776779
return ERROR;
777780
}
778-
for (int i = 0; i < groupingParenCount; ++i) {
781+
// To avoid deep call-stack recursion on heavily nested parentheses (e.g. "((((a))))" or
782+
// "((((a + 1) + 1) + 1))"), consume all consecutive leading '(' tokens upfront, parse the
783+
// innermost expression once, and then iteratively unwind each enclosing '(' from
784+
// innermost to outermost. After consuming each matching ')', if more enclosing '(' remain
785+
// open and the next token is not another ')', continue parsing any trailing selectors or
786+
// binary/ternary operators belonging to that enclosing parenthesized level using the
787+
// already-parsed inner expression as the LHS.
788+
int openParens = 0;
789+
while (peekToken.type == Lexer.TokenType.LEFT_PAREN) {
790+
openParens++;
791+
if (checkRecursion(openParens, peekToken)) {
792+
return ERROR;
793+
}
779794
nextToken();
780795
}
781-
CelExpr expr = parseExpr();
782-
for (int i = 0; i < groupingParenCount; ++i) {
796+
recursionDepth += openParens;
797+
CelExpr expr = parseBinaryAndTernary(0);
798+
for (int i = 0; i < openParens; ++i) {
783799
expect(Lexer.TokenType.RIGHT_PAREN, "");
800+
recursionDepth--;
801+
if (i < openParens - 1 && peekToken.type != Lexer.TokenType.RIGHT_PAREN) {
802+
currentLhsDepth = 0;
803+
Lexer.TokenType tok = peekToken.type;
804+
if (tok == Lexer.TokenType.DOT
805+
|| tok == Lexer.TokenType.LEFT_BRACKET
806+
|| tok == Lexer.TokenType.LEFT_BRACE) {
807+
expr = parseSelectorChainTail(expr);
808+
}
809+
expr = parseBinaryAndTernaryFromLhs(expr, 0);
810+
}
784811
}
785812
return expr;
786813
}
@@ -1098,8 +1125,7 @@ private Optional<CelExpr> tryExpandMacro(
10981125
return Optional.empty();
10991126
}
11001127
if (nodeLimitExceeded) {
1101-
reportError(
1102-
getPosition(exprId), "could not expand macro: expression node limit exceeded");
1128+
reportError(getPosition(exprId), "could not expand macro: expression node limit exceeded");
11031129
return Optional.empty();
11041130
}
11051131

@@ -1178,76 +1204,6 @@ private CelExpr buildMacroCallArgs(CelExpr expr) {
11781204
return expr;
11791205
}
11801206

1181-
private int countGroupingParentheses() {
1182-
if (peekToken.type != Lexer.TokenType.LEFT_PAREN) {
1183-
return 0;
1184-
}
1185-
1186-
// Fast path: if the next non-whitespace character is not '(', leading open parens is 1.
1187-
int pos = peekToken.end;
1188-
int size = content.size();
1189-
while (pos < size) {
1190-
int c = content.get(pos);
1191-
if (c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\f' && c != 11) {
1192-
if (c == '/') {
1193-
// A comment might precede another '('.
1194-
break;
1195-
}
1196-
if (c == '(') {
1197-
break;
1198-
}
1199-
// Next significant token is definitely not '('.
1200-
return 1;
1201-
}
1202-
pos++;
1203-
}
1204-
1205-
int savedPos = lexer.savePosition();
1206-
try {
1207-
int leadingOpenParens = 1;
1208-
Lexer.Token tok = nextSignificantToken(/* reportError= */ false);
1209-
while (tok.type == Lexer.TokenType.LEFT_PAREN) {
1210-
leadingOpenParens++;
1211-
tok = nextSignificantToken(/* reportError= */ false);
1212-
}
1213-
if (leadingOpenParens == 1) {
1214-
return 1;
1215-
}
1216-
1217-
int openParens = leadingOpenParens;
1218-
int consecutiveLeadingClosed = 0;
1219-
1220-
while (openParens > 0) {
1221-
if (tok.type == Lexer.TokenType.END || tok.type == Lexer.TokenType.ERROR) {
1222-
return 1;
1223-
}
1224-
1225-
if (tok.type == Lexer.TokenType.LEFT_PAREN) {
1226-
openParens++;
1227-
consecutiveLeadingClosed = 0;
1228-
} else if (tok.type == Lexer.TokenType.RIGHT_PAREN) {
1229-
if (leadingOpenParens == openParens) {
1230-
leadingOpenParens--;
1231-
consecutiveLeadingClosed++;
1232-
} else {
1233-
consecutiveLeadingClosed = 0;
1234-
}
1235-
openParens--;
1236-
} else {
1237-
consecutiveLeadingClosed = 0;
1238-
}
1239-
1240-
if (openParens > 0) {
1241-
tok = nextSignificantToken(/* reportError= */ false);
1242-
}
1243-
}
1244-
1245-
return Math.max(1, consecutiveLeadingClosed);
1246-
} finally {
1247-
lexer.restorePosition(savedPos);
1248-
}
1249-
}
1250-
12511207
private final class PrattMacroExprFactory extends CelMacroExprFactory {
12521208
private final ArrayDeque<Integer> macroPositions = new ArrayDeque<>(1);
12531209

‎parser/src/test/java/dev/cel/parser/CelParserImplTest.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,18 @@ private enum MaxParseRecursionDepthTestCase {
229229
+ " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !="
230230
+ " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !="
231231
+ " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20] !="
232-
+ " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]");
232+
+ " a[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20]"),
233+
TERNARY(
234+
"a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :"
235+
+ " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :"
236+
+ " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b :"
237+
+ " a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : a ? b : c"),
238+
TERNARY_TRUE_BRANCH_PARENS(
239+
"a ? ((((((((((((((((((((((((((((((((b)))))))))))))))))))))))))))))))) : c"),
240+
NESTED_LEFT_PARENS_WITH_CALC(
241+
"((((((((((((((((((((((((((((((((7) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1)"
242+
+ " + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) + 1) +"
243+
+ " 1) + 1) + 1) + 1)");
233244

234245
static final int MAX_RECURSION_LIMIT = 32;
235246
final String source;

‎parser/src/test/resources/parser_errors.baseline‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1098,9 +1098,6 @@ E/A: Expression recursion limit exceeded. limit: 250
10981098
E/P: ERROR: <input>:1:251: Expression recursion limit exceeded. limit: 250
10991099
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
11001100
| ..........................................................................................................................................................................................................................................................^
1101-
ERROR: <input>:1:251: Syntax error: expected ']'
1102-
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
1103-
| ..........................................................................................................................................................................................................................................................^
11041101

11051102
I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
11061103
»»»[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['too many']]]]]]]]]]]]]]]]]]]]]]]]]]]]
@@ -1110,19 +1107,13 @@ E/A: Expression recursion limit exceeded. limit: 32
11101107
E/P: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
11111108
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
11121109
| ................................^
1113-
ERROR: <input>:1:33: Syntax error: expected ']'
1114-
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
1115-
| ................................^
11161110

11171111
I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
11181112
=====>
11191113
E/A: Expression recursion limit exceeded. limit: 32
11201114
E/P: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
11211115
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
11221116
| ................................^
1123-
ERROR: <input>:1:33: Syntax error: expected ']'
1124-
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
1125-
| ................................^
11261117

11271118
I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H
11281119
=====>

‎parser/src/test/resources/pratt_parser_errors.baseline‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -398,18 +398,12 @@ I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
398398
E: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
399399
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
400400
| ................................^
401-
ERROR: <input>:1:33: Syntax error: expected ']'
402-
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
403-
| ................................^
404401

405402
I: [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
406403
=====>
407404
E: ERROR: <input>:1:33: Expression recursion limit exceeded. limit: 32
408405
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
409406
| ................................^
410-
ERROR: <input>:1:33: Syntax error: expected ']'
411-
| [[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[['not fine']]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]
412-
| ................................^
413407

414408
I: a.b.c.d.e.f.g.h.i.j.k.l.m.n.o.p.q.r.s.t.u.v.w.x.y.z.A.B.C.D.E.F.G.H
415409
=====>

0 commit comments

Comments
 (0)