Skip to content

GROOVY-12398: Improve lexer and parser performance - #2921

Open
daniellansun wants to merge 5 commits into
masterfrom
GROOVY-12398
Open

GROOVY-12398: Improve lexer and parser performance#2921
daniellansun wants to merge 5 commits into
masterfrom
GROOVY-12398

Conversation

@daniellansun

@daniellansun daniellansun commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

https://issues.apache.org/jira/browse/GROOVY-12398

Flatten number-literal fragments to character classes, split ASCII identifiers onto a predicate-free DFA path, replace regex-based lexer predicates with direct character tests, and gate annotation element values so AdaptivePredict does not also explore assignment expressions.

Local parse-only measurement (SLL then LL) of src/main/groovy + src/test-resources/core (245 files, approximately 570k characters):

Run Best Mean
before 0.231s 0.401s
after 0.146s 0.169s

Best run is about 1.6x faster. Mean improves more because prediction is more stable after DFA warmup.

@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 81.48148% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.1934%. Comparing base (71afcf1) to head (b6ddcb4).

Files with missing lines Patch % Lines
...pache/groovy/parser/antlr4/SemanticPredicates.java 80.0000% 2 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2921        +/-   ##
==================================================
+ Coverage     71.1883%   71.1934%   +0.0051%     
- Complexity      37708      37729        +21     
==================================================
  Files            1584       1584                
  Lines          136493     136500         +7     
  Branches        25358      25361         +3     
==================================================
+ Hits            97167      97179        +12     
+ Misses          30556      30551         -5     
  Partials         8770       8770                
Files with missing lines Coverage Δ
...org/apache/groovy/parser/antlr4/AbstractLexer.java 95.7895% <100.0000%> (+0.0905%) ⬆️
...pache/groovy/parser/antlr4/SemanticPredicates.java 87.3563% <80.0000%> (+3.2100%) ⬆️

... and 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

JMH summary — classic (commit 73c828f)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group Speedup Calibrated n
bench 0.964 × 1.002 × 124
core 1.170 × 1.102 × 108
grails 1.336 × 1.069 × 80

No benchmark is ≥1.5× slower than its 90-day baseline.

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-hz, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.

Runner calibration (this run vs baseline hardware): bench 0.97× (27 rulers) · core-ag 0.98× (3 rulers) · core-hz 1.23× (3 rulers) · grails-ad 1.07× (3 rulers) · grails-ez 1.42× (3 rulers)

Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

JMH summary — indy (commit 73c828f)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group Speedup Calibrated n
bench 1.072 × 1.092 × 124
core 4.953 × 4.914 × 108
grails 2.770 × 2.938 × 80

No benchmark is ≥1.5× slower than its 90-day baseline.

Runner calibration (this run vs baseline hardware): bench 1.00× (27 rulers) · core-ag 1.04× (3 rulers) · core-hz 0.96× (3 rulers) · grails-ad 0.96× (3 rulers) · grails-ez 0.93× (3 rulers)

Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data

@paulk-asert

Copy link
Copy Markdown
Contributor

Initial AI read (my initial leaning is master only to start with and potentially backport to 6 after some settling time):

Assessment: PR 2921 (GROOVY-12398, lexer/parser performance)

Verdict: this is ~95% mechanically equivalent refactoring, but not 100% — there are three genuine behavior deltas, two of which are bug fixes and one of which is a deliberate parsing-strategy change. My recommendation: merge to master, but don't pull it into the 6.0 RC line.

I read the full diff and hand-verified each change against the current grammar. What's in it:

Verified mechanically equivalent (no behavior change):

  • The number-literal fragment flattening (Digits, HexDigits, OctalDigits, BinaryDigits, ExponentPart, OctalEscape, the Dot/Zero fragment inlining) — I compared each old fragment chain against its new character class and they match exactly. All references to the removed fragments are replaced in the diff.
  • isFollowedByWhiteSpaces and isFollowedByJavaLetterInGString regex-to-char-test rewrites — equivalent, including the subtle cases: Java's ASCII-only \s (so \u00A0 still counts as non-space), lone low surrogates, and EOF (the old code reached isJavaIdentifierPart(-1) which happens to return false; the new code guards it explicitly).
  • The ASCII identifier split ([A-Z] vs [a-z$_]) — the classes are disjoint and their union equals the old ASCII JavaLetter, and the BMP-non-ASCII alternatives reproduce the old predicate logic exactly.

Behavior deltas:

  1. Supplementary-plane uppercase identifiers reclassify from Identifier to CapitalizedIdentifier. The old CapitalizedIdentifier predicate tested isUpperCase(_input.LA(-1)), which for a surrogate pair sees only the low surrogate — always false. The new rule tests the real code point. So an identifier starting with, say, Deseret 𐐀 or mathematical 𝔄 (U+1D504) now lexes as CapitalizedIdentifier. I checked the parser: the bare Identifier token appears only inside the identifier rule (which accepts both), so nothing that parses today breaks — but className accepts only CapitalizedIdentifier, so something like 𝔄 x = 1 that today parses as a command expression could now parse as a typed variable declaration. Realistically this touches no real-world code, and the new behavior is arguably the correct one, but it is technically a parse change, not pure performance.

  2. The elementValues predicate gate (@Foo(a = 1) no longer explored as an assignment expression). This one I checked hard because it's the scariest on paper: the new ELEMENT_VALUE_PAIR_NAME_TYPES BitSet is exactly the token set of identifier ∪ keywords (all 60 tokens including MODULE, which is in identifier but not keywords — Daniel got that right). And since the alternative order already made elementValuePairs win any ambiguity, and any name = expr input viable as a single element value is also viable as a pair (element values are a superset that includes expressions), the gate cannot reject anything that parses today — worst case is a different error message on already-invalid input. Safe.

  3. Invalid-octal error offset fix: the old invalidDigitCount field was never reset between tokens, so a second invalid octal in the same file got a wrong caret position. The new getText().length() form is stateless and correct. Error-reporting change only, and a fix.

Other observations: CI is fully green across the entire matrix (all LTS JDKs, three OSes, lint gate, CodeQL, SonarCloud). The PR adds decent targeted tests for exactly the risky spots (identifier split incl. Unicode, flattened literals incl. 07 invalid-octal boundary, annotation forms incl. @Foo(class = 1) and @Foo(a + 1)). The ANTLR dependency is me.sunlan:antlr4 — Daniel's own fork — so prediction-time predicate semantics for the new gate are under the author's control. The claimed 1.6× is parse-only; end-to-end compile improvement will be a modest fraction of that, though it does chip at the Groovy 6 vs 5.0.6 parse-phase gap we've catalogued.

Recommendation given RC phase: By your own criterion this doesn't qualify as "strictly performance" — items 1 and 3 change observable behavior in edge cases, and grammar/ATN rewrites are inherently the highest-blast-radius category of perf change (a lexer DFA subtlety that the test suite doesn't cover would ship as a parse regression). None of the deltas endanger realistic code, so I'd merge it to master for the next train without hesitation. For 6.0 in RC, I'd defer: the win is parse-time only, and the cost of being wrong during RC outweighs it. If you want it in 6.0 anyway, the minimum bar I'd suggest is one more RC cycle after it lands plus a compile sweep of a large corpus (e.g. the usual grails/spock reproducer set) comparing ASTs before/after.

@daniellansun

Copy link
Copy Markdown
Contributor Author

Thanks Paul — that's a much closer reading than mine was, and I agree with the recommendation.

Master only for now; I would not put this in the 6.0 RC line. The 1.6× figure is parse-only, and a grammar/ATN change is the wrong kind of surprise during RC. A backport after some settling time (and a corpus AST sweep if we do go there) sounds right.

On the three deltas:

  1. Supplementary-plane capitalization. You are right that isUpperCase(LA(-1)) after a surrogate pair only saw the low surrogate, so everything in that plane used to lex as Identifier. Using the decoded code point is a real parse change for className. I think the new behaviour is the one we want, but it is not “strictly performance”. I have extracted lastConsumedCodePoint() so that mistake is harder to reintroduce, and added tests (Deseret capital vs small, type parameter vs command, isInvalidLocalVariableDeclaration on the code point). Follow-up: e27a4dabbd.

  2. elementValues gate. Glad that checked out. I added a sweep over identifier ∪ keywords (including MODULE) so the BitSet cannot drift silently, plus a note on those two parser rules.

  3. Invalid-octal caret. Agreed it was a latent counter bug. The follow-up reports from the token start (same pattern as unclosed comments), so a suffix or a second 08 in the file cannot shift the caret. Tests cover 08, 08L, 08 09, and errorIgnored.

Happy to drop or reword any of that if you would rather keep the old supplementary-plane classification for this PR and treat the code-point fix separately.

@daniellansun

Copy link
Copy Markdown
Contributor Author

Parser performance verification: GROOVY-12398

Question. Does commit 4c89f6774d5cbb431940b0ca9d1581a9434b0d1e parse a fixed Groovy corpus faster than commit 4aa7b94b83bd29814b1449cde1b239bdbe8c0ff0, when both use the same parse-only path that production uses (SLL, then LL on failure)?

Answer. Yes. On this host, with a frozen corpus of 245 files (570 533 characters), the target parser used about 35% less wall time than the baseline (pooled mean 1.53×). Every post-warmup sample of the target was faster than every post-warmup sample of the baseline. Parse outcomes (SLL success, LL fallback, the one expected failure) are identical.

This is a parse-only measurement. It does not claim the same factor for end-to-end groovyc.


1. What was compared

Baseline Target
Commit 4aa7b94b83bd29814b1449cde1b239bdbe8c0ff0 4c89f6774d5cbb431940b0ca9d1581a9434b0d1e
Subject prepare for version 7 (cont'd) GROOVY-12398: classify supplementary-plane identifiers by code point
Raw jar SHA-256 83130ad2d6ce26e1fee7eccca7a9f387151142defb923d3af1087f324cb7f8c7 17946a113b5123939a6ecff762a3ffa62535bbb8d0444e39a928acf7a736cbe8

The range contains two commits:

  1. cfcc6d50fc — lexer/parser grammar and predicate work (the performance change).
  2. 4c89f6774d — review follow-up (code-point capitalization, invalid-octal caret, tests). That follow-up is not expected to move the hot path; the measurement is of the full GROOVY-12398 series against the parent of the first commit.

Each commit was built in its own Git worktree (./gradlew :jar). The jars were checked before timing: baseline still contains invalidDigitCount and java.util.regex.Pattern; target contains lastConsumedCodePoint and isIdentifierAssign.


2. Method

Corpus

src/main/groovy + src/test-resources/core, *.groovy only, walked in sorted path order.

The tree was frozen from the baseline commit and reused for every run, so a change in source text cannot be mistaken for a parser change. The same two directories at the target commit hash to the same SHA-256 (39 + 206 files). Input is therefore identical in both trees.

Root Files UTF-16/UTF-8 chars Bytes
src/main/groovy 39 290 264 290 270
src/test-resources/core 206 280 269 280 277
Total 245 570 533 570 547

Parse path

Matches AstBuilder:

  1. GroovyLangLexer + CommonTokenStream + GroovyLangParser
  2. BailErrorStrategy, no console listeners
  3. PredictionMode.SLL first
  4. On failure: tokens.seek(0), parser.reset(), PredictionMode.LL
  5. New lexer/parser per file; ATN/DFA shared through ParserAtnManager (production-like cache warmup)

ANTLR runtime: me.sunlan:antlr4-runtime:4.13.2.16 (the runtime Groovy already uses).

Timing protocol

  • Independent JVM per invocation (-Xms1g -Xmx1g -XX:+AlwaysPreTouch).
  • 5 untimed warmup passes (DFA fill + JIT), then 15 timed full-corpus passes.
  • Wall time is System.nanoTime() around the whole corpus, not per file.
  • Invocation order was interleaved A B A B A B A (baseline / target / baseline / …) so a slow host cannot be blamed on “target ran first while the machine was idle”.
  • 4 baseline JVMs × 15 = 60 timed samples; 3 target JVMs × 15 = 45 timed samples.

Host

Kernel Linux 6.15.5 x86_64 (hera)
CPU AMD EPYC 7763, 6 visible CPUs, 1 thread/core
Memory 24 GiB
JVM Corretto 25.0.4+7-LTS, mixed mode
Date 2026-09-11 (UTC)

This is a shared developer VM. Variance across baseline JVMs is reported rather than hidden.


3. Correctness (same corpus, both parsers)

Outcome Baseline Target
SLL success 241 241
LL fallback 3 3
Hard failure 1 1

LL fallback (both parsers):

  • src/test-resources/core/Command_01.groovy
  • src/test-resources/core/Command_05.groovy
  • src/test-resources/core/Lambda_01x.groovy

Hard failure (both parsers): src/test-resources/core/Unicode_01.groovyGroovySyntaxError: Illegal escape character: '\u'. That file is an illegal-escape fixture, not a regression.

The speedup is not from fewer LL retries. The SLL/LL split is unchanged.


4. Results

Times are seconds to parse the whole corpus after warmup. Lower is better.

Per JVM (15 timed rounds each)

Run Role Min Median Mean Stdev Max
A baseline 0.218082 0.233826 0.242895 0.022486 0.293010
B target 0.148911 0.177118 0.177364 0.019880 0.211886
A2 baseline 0.254599 0.295294 0.300075 0.036579 0.381228
B2 target 0.147442 0.165518 0.166205 0.009369 0.185643
A3 baseline 0.222311 0.236567 0.242742 0.018481 0.291574
B3 target 0.147241 0.162760 0.167201 0.016246 0.204381
A4 baseline 0.219386 0.260223 0.255703 0.027844 0.326735

A2 is a noisy baseline JVM (mean 0.300 s vs ~0.243 s for A and A3). It is kept in the pool rather than discarded.

Pooled post-warmup samples

n Min Median Mean Stdev Max
Baseline 60 0.218082 0.252503 0.260354 0.035573 0.381228
Target 45 0.147241 0.165395 0.170257 0.016237 0.211886

Speedup

Estimator Baseline Target Factor Time saved
Mean 0.260354 s 0.170257 s 1.53× 34.6%
Median 0.252503 s 0.165395 s 1.53× 34.5%
Best (min) 0.218082 s 0.147241 s 1.48× 32.5%

Throughput at the pooled means: 941 → 1 439 files/s, 2.19e6 → 3.35e6 chars/s.

Separation

The slowest target round (0.211886 s) is still faster than the fastest baseline round (0.218082 s). The two sample sets do not overlap.

Cohen’s d on the pooled samples is 3.1 (large). That is a description of this data set, not a claim about every machine.

Conservative cross-check

Against the fastest baseline JVM mean (A3, 0.242742 s) and the slowest target JVM mean (B, 0.177364 s), the factor is still 1.37×. That is the least flattering pairing of JVM means in this run.


5. What this does and does not show

Shown

  • On this host, the GROOVY-12398 parser is consistently faster on this corpus than its parent, by roughly one and a half times in parse-only wall time.
  • The improvement is in the SLL-successful majority (241/245 files), not in a change of prediction mode mix.
  • The corpus and the parse outcomes are the same, so the delta is in the recognisers, not in “different files” or “more failures skipped”.

Not shown

  • End-to-end compile time (semantic analysis, classgen, I/O). Parse is only part of groovyc; a 1.5× parse-only gain will shrink to a smaller fraction of a full compile, as already noted on the PR.
  • Other hardware, JDK 17/21, or a Grails/Spock-sized corpus. Those would be the right next measurement if the change is considered for 6.0.
  • That every future Groovy file will see 1.5×. Command-heavy scripts that already fall back to LL (three files here) are not the bulk of this corpus.

Noise

Baseline JVM A2 was ~24% slower than A/A3. The machine is not a quiet benchmark box. Interleaving and pooling are how that is handled; dropping A2 would only make the target look better, so it was not dropped.


6. Conclusion

Relative to 4aa7b94b83, commit 4c89f6774d (GROOVY-12398 as it stands on that branch) delivers a real, repeatable parse-only speedup of about 1.5× (≈35% less time) on src/main/groovy + src/test-resources/core. Parse behaviour on that corpus is unchanged.

That is sufficient evidence for a master merge from a performance standpoint. It is not by itself a case for slipping the same grammar/ATN rewrite into a 6.0 RC without a further compile-level corpus check.


Appendix: how to reproduce

# worktrees
git worktree add --detach /tmp/parse-verify/wt-baseline 4aa7b94b83bd29814b1449cde1b239bdbe8c0ff0
git worktree add --detach /tmp/parse-verify/wt-target   4c89f6774d5cbb431940b0ca9d1581a9434b0d1e
( cd /tmp/parse-verify/wt-baseline && ./gradlew :jar --offline -q )
( cd /tmp/parse-verify/wt-target   && ./gradlew :jar --offline -q )

# freeze corpus from baseline
mkdir -p /tmp/parse-verify/corpus
cp -a /tmp/parse-verify/wt-baseline/src/main/groovy          /tmp/parse-verify/corpus/main-groovy
cp -a /tmp/parse-verify/wt-baseline/src/test-resources/core  /tmp/parse-verify/corpus/test-resources-core

ANTLR=$HOME/.gradle/caches/modules-2/files-2.1/me.sunlan/antlr4-runtime/4.13.2.16/9c7caa836c70ef09ee7b70f77c11b803f75e19c0/antlr4-runtime-4.13.2.16.jar
# ParseBench.java as used for this report (5 warmup, 15 rounds, SLL then LL)
java -Xms1g -Xmx1g -XX:+AlwaysPreTouch -Dbench.label=... \
  -cp ".:<raw-jar>:$ANTLR" ParseBench \
  /tmp/parse-verify/corpus/main-groovy \
  /tmp/parse-verify/corpus/test-resources-core

@testlens-app

This comment has been minimized.

Flatten number-literal fragments to character classes, split ASCII
identifiers onto a predicate-free DFA path, replace regex-based lexer
predicates with direct character tests, and gate annotation element
values so AdaptivePredict does not also explore assignment expressions.
Decode surrogate pairs before isUpperCase so CapitalizedIdentifier is
correct, report invalid octals at the token start, and cover the
annotation pair-name set plus those edge cases with tests.
Compute FIRST(elementValuePairName) with LL1Analyzer at startup so
identifier and keywords no longer need a parallel Java token list.
The empty file was added by accident and fails Apache RAT
(unapproved license). GROOVY-12400 already inlined that corpus
into ParserNegativeSyntaxTest.
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants