Repository navigation
perf: drop the list-form probes in ExpressionList - #2877
Merged
manticore-projects merged 1 commit intoOct 31, 2026
Merged
manticore-projects merged 1 commit into
manticore-projects merged 1 commit into
Conversation
`ExpressionList()` ran three 3-token lookaheads to choose between the complex, simple and parenthesed list forms, and `ParenthesedExpressionList()` scanned into the list on the parse path to tell an empty body from a non-empty one. Together they took about 5% of parser CPU on `performance.sql`. The probes decided nothing: `ComplexExpressionList()` accepts every list the other two forms do, including `()` and bracketed operands such as `( 1 + a ) / ( 1 + b )`, so it won wherever they could have. It is now called directly, and the parenthesed body is guarded by the token kind of the closing bracket. Parser CPU per parse drops from 29.0 ms to 27.7 ms (-4.5%, three interleaved pairs, on top of JSQLParser#2876), and `./gradlew jmh` from 32.67 to 28.54 ms/op.
knutwannheden
force-pushed
the
perf/expression-list-lookahead
branch
from
October 29, 2026 11:30
92bfd04 to
85690dc
Compare
Contributor
|
Thank you very much! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After #2876, the largest remaining lookahead cost in parsing
performance.sqlsits in the callers ofComplexExpressionList():ExpressionList()ran three 3-tokenLOOKAHEAD(3)probes to choose between the complex, simple and parenthesed list forms, andParenthesedExpressionList()scanned into the list to decide whether its body is empty. Together they took about 5% of parser CPU, net of lexing, and both decisions have a token-level answer.The three probes in
ExpressionList()decided nothing, becauseComplexExpressionList()accepts every list the other two forms do:Expression()accepts everySimpleExpression(), so the simple form is a subset.Expression() → PrimaryExpression() → "(" → ParenthesedExpressionList(), the same tokens the parenthesed probe walks, andPrimaryExpression()'s subselect alternatives are semantic guards that hold inside a scan as well.So the complex alternative won wherever either of the others could have, and the fallback alternative behind the probes, added in ca2b3d7 for bracketed operands such as
( 1 + a ) / ( 1 + b ), was the same production once more.ExpressionList()now callsComplexExpressionList()directly.ParenthesedExpressionList()'s optional body is now[ LOOKAHEAD({ getToken(1).kind != CLOSING_BRACKET }) ComplexExpressionList() ]. Anything else that is not a list start fails insideExpression()instead of at the), as in #2876. JavaCC drops that semantic guard inside nested scans, so the saving is on the parse path, where the profile samples were.Which alternative wins matters for the AST: the block after the choice re-wraps a plain list holding one parenthesed list, but returns a parenthesed list holding one as it is. The reasoning above says the complex alternative already won in every case that parsed, and the full suite agrees.
FunctionTest.testBracketedOperandAndEmptyListArgumentspins the two shapes only the removed alternatives looked responsible for,f((1 + a) / (1 + b), (c + d) * 2)andf(()), which ca2b3d7 shipped without a test, plusVALUES ()for a list that reachesExpressionList()outside a function call.Parser-thread CPU per parse against #2876 (e8243dc), three interleaved pairs of JVMs, 200 warmup and 300 measured parses each:
./gradlew jmh -PjmhProfiler=gcon the same machine, #2876 first:The 5.4 control moved 3.6% between the two runs, so the CPU table above is the conservative figure; the direction is the same in both.
This branch is based on master after #2876 merged; the baseline column above was taken on #2876's branch commit e8243dc, which is the same tree as its merge commit a0e0861. It also rewords one comment that #2876 adds in
ComplexExpressionList(), and the JMH pair was taken on an earlier iteration whose only difference is a fallback alternative that ran when the parser was interrupted.