Skip to content

perf: cache feature configuration lookups on the parse hot path - #2828

Closed
fudianchn wants to merge 4 commits into
JSQLParser:masterfrom
fudianchn:perf/lexer-config-overhead
Closed

fudianchn wants to merge 4 commits into
JSQLParser:masterfrom
fudianchn:perf/lexer-config-overhead

Conversation

@fudianchn

@fudianchn fudianchn commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

AI disclosure: this change was prepared with AI coding agents, reviewed and revised line by line by me.

What

Remove repeated feature-configuration lookups from per-token and per-expression paths, use typed getter fast paths, and reduce allocation and predicate work in primary expressions.

Why

Lexical actions repeatedly read the same options. Cache these derived flags while the configuration remains unchanged, and avoid converting already typed getter values through strings. Configuration changes and parser reuse must preserve the existing parsing behavior.

How

  • getAsBoolean / getAsLong / getAsInt / getAsInteger return stored Boolean / Long / Integer values directly and keep string parsing for other stored types, including null.
  • FeatureConfiguration.setValue updates a modification stamp exposed by getModificationCount(). Both derived-flag caches check configuration identity and this stamp, so in-place changes and ReInit reuse refresh the flags. Configuration subclasses are refreshed on each use to preserve overridden values.
  • The token manager reads its public configuration; hash comments retain their existing stream-wiring requirement.
  • innerSigns is allocated only for chained signs. PostgreSQL JSON access rejects non-operator images before the equals chain, and opaque statement capture checks its enabled flag before the loop.

Root cause

The original draft's identity-only caches missed in-place changes between parses. Reading only input_stream.featureConfiguration also ignored options set directly on the public token manager. Both regressions are covered by the added tests.

Testing

  • Complete suites executed on a remote Linux server over SSH, JDK 17.0.20: mvn clean verify passed on base bdf8ac31 (10,334 tests) and fixed head 0f911fa7 (10,367); Gradle check passed with 10,352 and 10,385 tests respectively. Each run has zero failures/errors and the same 25 project-disabled tests. The Mockito suites previously blocked by local agent attachment execute successfully there.
  • 33 lifecycle and public-lexer regression checks pass on master and the fixed branch; 24 fail on the original draft 643b2adf. Four targeted cache/guard mutants fail as expected.
  • Serialized semantic ASTs for performance.sql are byte-identical to master: 54 statements, 486,434 bytes.
  • Four-version release/snapshot comparison for the performance concern in #2785, executed serially through Mac ssh dev on the same Linux server and JDK 17.0.20. The harness is the unmodified project JSQLParserBenchmark.parseSQLStatements and DynamicParserRunner from bdf8ac31; every parser JAR uses the same isolated URLClassLoader(parent=null), including the snapshots. Configuration is SIMPLE (withAllowComplexParsing(false)). No production code or corpus was changed for this experiment.
  • The unchanged performance.sql corpus has SHA-256 e8546f42fa84bbd9e142490cdfcbcd4cc37110cccebda3b059ff77c1c06df8f8. Each state parses all 54 non-null statements and produces the same restored SQL. The three snapshot states have identical semantic field values. The 5.4 model is older: shared semantic fields agree, while snapshot fields additionally record STANDARD escaping and FILTER WHERE presence. Release-to-snapshot timings therefore include model/schema evolution and do not isolate equal AST work.
  • Two windows per version, in forward/reverse order: 5.4 → master → PR 2827 → PR 2828 → PR 2828 → PR 2827 → master → 5.4. Each window uses 3 forks, 3 × 3-second warmup iterations, 10 × 1-second measurement iterations, one thread, and -Xms1g -Xmx1g -Xss4m -Dfile.encoding=UTF-8: 30 samples per window, 60 per version.
Version Commit Window 1, ms/op ± 99.9% CI half-width Window 2, ms/op ± 99.9% CI half-width
5.4 tag e847e94b 21.678 ± 0.319 21.709 ± 0.237
master bdf8ac31 22.493 ± 0.318 22.332 ± 0.269
PR 2827 68c035bc 19.227 ± 0.353 19.317 ± 0.331
This PR (2828) 0f911fa7 22.736 ± 0.316 22.649 ± 0.277

For this PR, latency is +1.08% / +1.42% versus master, with overlapping score confidence intervals. These measurements do not demonstrate a speedup; passing the correctness regression tests does not establish an optimization benefit. No recovery of the observed release-to-snapshot performance gap is demonstrated by this PR on this corpus.

Versus 5.4, this PR has +4.88% / +4.33% latency; master has +3.76% / +2.87%. Their score intervals do not overlap with 5.4 in either window, subject to the model-evolution qualification above. Percentages compare point estimates; no confidence interval for these ratios is claimed.

Verification of the original issue

Not applicable (no issue).

Signed-off-by: 付典 <fudianchn@gmail.com>
The token actions query the wired FeatureConfiguration for every identifier,
string literal and quoted identifier; PrimaryExpression pays new predicate
checks after every expression. Cache the derived dialect/flag answers on the
wired configuration's identity (it is only mutated before parsing starts),
lazily allocate the DuckDB chained-sign list, reject non-operator images
before the PostgreSQL json-access equals chain, and hoist the unsupported
statements flag out of the capture loop.

Signed-off-by: 付典 <fudianchn@gmail.com>
Signed-off-by: 付典 <fudianchn@gmail.com>
Track configuration mutations as well as object identity, preserve the public token manager configuration, and avoid caching dynamic subclass values. Cover parser reuse, in-place changes, replacement, and direct lexer APIs with 33 regression checks.

Signed-off-by: 付典 <fudianchn@gmail.com>
@fudianchn

Copy link
Copy Markdown
Contributor Author

@manticore-projects, the remote benchmarks on the 54-statement corpus didn't show a measurable speedup from this PR. I don't think the added cache-invalidation complexity and maintenance cost are justified by these results, so I'm closing it. If there's a specific workload where this approach could help, we can revisit it with evidence.

@fudianchn fudianchn closed this Oct 16, 2026
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.

1 participant