Skip to content

perf(mapper): skip AST build when the writer is not listening - #209

Open
shihyuho wants to merge 1 commit into
jakartafrom
perf/wp7-hot-path-allocation
Open

perf(mapper): skip AST build when the writer is not listening#209
shihyuho wants to merge 1 commit into
jakartafrom
perf/wp7-hot-path-allocation

Conversation

@shihyuho

Copy link
Copy Markdown
Member

Closes #201

Work package WP7 — findings PERF-03 and PERF-04.

PERF-03 — eager AST build on every toSpec

SpecMapper.toSpec(Object) built a SpecAST, formatted a node string for every
resolved field, joined the whole tree into one string, and handed it to a
Writer that dropped it on the floor whenever the logger sat above DEBUG.
Pure allocation and CPU on the mapping hot path.

The gate could not simply be log.isDebugEnabled() inside SpecMapper, because
SpecMapper does not own the decision: the built-in impersonation() factory —
the starter's default — writes to the mapped object's logger, not
SpecMapper's. Gating on SpecMapper's own logger would have silently killed
AST output for every Spring Boot user who enables DEBUG on their criteria
class.

So the check lives where the knowledge is:

  • ASTWriterFactory gains default boolean isEnabled(Object rootObject)
    returning true. Additive and non-breaking — a custom factory that does not
    override it keeps receiving every AST exactly as before.
  • domain() and impersonation() now share a small Slf4jDebugWriterFactory
    that reports isEnabled from the same logger it would have written to.
  • When isEnabled is false, SpecMapper puts a no-op AST in the context and
    returns early: no node strings are formatted, no tree is joined, and
    createWriter is never called. Resolvers still call ast.add(...)
    unconditionally, so no resolver had to change.

Scope note: this touches ASTWriterFactory.java, one file beyond the package's
nominal allowlist. It is unavoidable — the enablement decision is per-factory,
and no other WP in flight touches this file.

How the DEBUG gating is verified

mapper/src/test/java/tw/com/softleader/data/jpa/spec/ASTWriterFactoryTest.java
(4 tests) drives real logback levels against a Mockito spy of the real built-in
factories, plus a resolver that captures the AST instance the mapper handed
down:

test asserts
noAstWhenDebugDisabled level INFOverify(factory, never()).createWriter(any(), any()) and the captured AST's print() is empty — the tree itself was never built
astWrittenWhenDebugEnabled level DEBUGcreateWriter called once, AST content present
impersonationHonoursMappedObjectLogger SpecMapper at INFO, criteria class at DEBUG → still written (guards the regression described above)
customFactoryKeepsReceivingAstByDefault a custom factory that does not override isEnabled still gets the AST with everything at INFO

Confirmed non-vacuous: reverting only the SpecMapper gate makes
noAstWhenDebugDisabled fail with NeverWantedButInvoked.

PERF-04 — always-on DISTINCT

Restricted to the documentation change, as the plan explicitly authorizes.
@Join#distinct keeps its true default (flipping it is a breaking behavior
change) and its Javadoc now states that only a to-many association can multiply
rows, so a to-one join can set distinct = false to skip the needless
sort/dedup.

The optional optimization — omitting DISTINCT when no to-many join
participates — was not attempted, for two reasons:

  1. Correctness. Deciding to-many-ness requires JPA metamodel inspection at
    predicate-build time. Getting it wrong in the permissive direction silently
    returns duplicate rows — a wrong-results bug, against a finding rated low.
  2. Cross-package collision. The per-join DISTINCT flag is accumulated in
    domain/Join.java and SpecJoinContext.java, which are concurrently being
    reworked by [WP3] Join machinery: silent-wrong-result edge cases + missing edge/error tests #198. Building the optimization on top of code being rewritten
    underneath it would produce a conflict-prone change resting on an unstable
    base.

To-many join behavior is therefore unchanged, and no existing distinct/join test
was modified.

Verification

make test green — mapper 102 tests, starter 15 tests, BUILD SUCCESS.
Built under JDK 17 (Temurin 17.0.6); the ambient JDK 25 breaks
google-java-format inside spotless, which is a pre-existing toolchain issue
unrelated to this change — no pom versions were touched.

🤖 Generated with Claude Code

The debug AST was built and stringified on every toSpec call, then thrown
away whenever the logger sat above DEBUG — pure allocation on the hot path.

ASTWriterFactory gains a default isEnabled(rootObject) hook so the gate can
live where the knowledge does: domain() checks SpecMapper's logger,
impersonation() checks the mapped object's logger. Custom factories keep the
true default, so their behavior is unchanged. When disabled, SpecMapper hands
resolvers a no-op AST and skips the writer entirely.

Also document that to-one joins can set @Join(distinct = false); the default
stays true since flipping it would be a breaking behavior change.

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
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.

[WP7] Mapper hot-path allocation: eager AST build + always-on DISTINCT

1 participant