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

Activity

1. shihyuho

   added a parent issue on Jul 16, 2026

   • Codebase review fix plan — 2026-07-16 #172
2. shihyuho

   added the

   Task

   issue type on Jul 16, 2026
3. github-actions

   added

   status: waiting-for-triage

   on Jul 16, 2026
4. shihyuho

   added

   type: bugSomething isn't workingSomething isn't working

   status: pending-design-work動工前需要更多的設計或研究動工前需要更多的設計或研究

   and removed

   status: waiting-for-triage

   on Jul 16, 2026
5. shihyuho commented on Jul 16, 2026

   

   shihyuho

   on Jul 16, 2026

   MemberAuthor

   More actions

   This was generated by AI during triage.

   Triage Notes - needs human

   Category: bug (performance)

   Summary: On the object-to-Specification mapping hot path, the debug AST is fully built and stringified on every call regardless of log level, and every join annotation forces SELECT DISTINCT by default — both cost CPU/allocation (or DB work) with no benefit unless the maintainer settles two open design/policy calls.

   Current behavior

   • PERF-03 (AST always built): SpecMapper.toSpec(Object) unconditionally constructs a SpecAST, records the root node (SpecAST.add, which runs String.format), and for every field/resolver pair invokes ASTNode.preVisit/postVisit. postVisit formats the resolved Specification with %s, and the root node likewise formats the whole resulting spec with %s. Because CompoundSpecification.toString emits "specs=" + specs and SimpleSpecification.toString emits its path/value, that %s recursively stringifies the entire spec tree, including all bound values. SpecAST.write then joins every node into one string. The only check on whether logging is actually enabled lives at the very end, in Slf4jDebugWriter.write, which calls logger.isDebugEnabled() and discards everything when DEBUG is off. So with debug disabled the full tree walk, formatting, list growth, and string join are all paid and thrown away.
   • PERF-04 (always-on DISTINCT): @Join and @JoinFetch both declare boolean distinct() default true. domain/Join.toPredicate and domain/JoinFetch.toPredicate call query.distinct(distinct) unconditionally (JoinFetch after its count/delete short-circuit, which delegates to Join passing distinct through). Any POJO using a join annotation without explicitly setting distinct=false therefore emits SELECT DISTINCT, even for to-one joins that cannot produce duplicate roots — extra DB sort/hash-dedup, and outright failure on some databases when DISTINCT spans LOB columns. This default is deliberate and documented in the annotation Javadoc ("Defaults to true to prevent duplicate records").

   Desired behavior

   • PERF-03: When AST/debug output will not be consumed (target logger DEBUG disabled), the mapper must not perform the AST string formatting or the recursive Specification.toString walk. When DEBUG is enabled, the emitted AST log must be byte-for-byte identical to today's output.
   • PERF-04: A join on a to-one association should not be forced to pay a DB-side DISTINCT it cannot benefit from — via a changed/narrowed default, a detection mechanism, or clearer guidance — without silently reintroducing duplicate rows for collection joins.

   Key interfaces

   • SpecMapper.toSpec(Object) and SpecMapper.toSpec(Context, Object); SpecMapper.resolveSpec(...).
   • SpecAST.add(int, String, Object...), SpecAST.print(), SpecAST.write(Writer); the AST interface.
   • ASTNode.preVisit(SpecInvocation) / postVisit(SpecInvocation, Specification).
   • ASTWriterFactory (public SPI: createWriter(Object, Specification), static factories domain() and impersonation()); package-private Slf4jDebugWriter.
   • CompoundSpecification.toString(), SimpleSpecification.toString().
   • Annotations tw.com.softleader.data.jpa.spec.annotation.Join / JoinFetch (distinct()); domain/Join.toPredicate / domain/JoinFetch.toPredicate.

   Acceptance criteria

   • With the target logger at a level above DEBUG, mapping an object does not invoke toString() on the resolved Specification tree (verifiable via a resolver that returns a Specification whose toString increments a counter / throws) and does not build the AST string.
   • With DEBUG enabled, the logged AST output is unchanged from current behavior (existing AST-logging tests still pass).
   • For PERF-04: the chosen resolution is implemented consistently across @Join, @JoinFetch, domain/Join, and domain/JoinFetch, with tests covering both the distinct and non-distinct query paths, and collection-join de-duplication behavior is preserved (or the behavior change is explicitly accepted and documented).
   • No change to the public API surface beyond what the maintainer approves for the chosen approach.

   Out of scope

   • Reworking the resolver ordering or the broader SpecMapper traversal algorithm.
   • Changing the AST log format or wording.

   Needs human because:

   1. PERF-03 requires a public-SPI / API-shape decision. The clean fix needs the mapper to learn up front whether output will be written, but enablement depends on the logger the ASTWriterFactory chooses (domain() → SpecMapper's logger; impersonation() → the mapped object's logger), and the current terminal gate (isDebugEnabled) is buried inside the package-private Slf4jDebugWriter behind the public Writer return type. ASTWriterFactory is a public SPI (users supply custom factories via SpecMapperBuilder.astWriterFactory(...)). Every viable fix — adding an isEnabled(rootObject) method to the SPI, or having SpecMapper probe the target logger — changes or extends that public contract. The reviewer's own suggested fix offered these as an unresolved "or". A maintainer should choose the SPI shape (and whether/how existing lambda factories domain()/impersonation() are reshaped).
   2. PERF-04 is a documented-default / breaking-change policy decision. distinct=true is a deliberate, Javadoc-documented default that prevents duplicate roots for collection joins. Narrowing or flipping it (default to false, auto-detect to-one vs collection cardinality, or only set the flag when explicitly requested) risks reintroducing duplicate rows and is a behavioral/breaking change whose acceptability is a maintainer call. The reviewer explicitly noted this is "a deliberate and javadoc-documented default." An agent cannot pick the default policy without that judgment.

   An agent could safely implement PERF-03 once the SPI approach is fixed, and PERF-04 once the default policy is decided; both blockers are design/policy sign-offs rather than infrastructure or access limitations.
6. shihyuho commented on Jul 20, 2026

   

   shihyuho

   on Jul 20, 2026

   MemberAuthor

   More actions

   Superseded by #201 in the execution map #204, which carries the same findings (PERF-03, PERF-04). This 2026-07-16 static plan is being retired in favour of that map.
7. shihyuho

   closed this as not plannedon Jul 20, 2026