perf(java): skip runtime NON_EMPTY checks for final JSON property types

[pavel-ptashyts]

Why?

Since #4074 the generated JSON writer calls JsonFieldInfo.isEmpty(value, wpN.writeTypeInfo(), writer) for every omit-empty property whose declared type is not a CharSequence, array, Collection, Map or Optional*. For boxed numbers, Boolean, enums and other final types that call can only ever return false, yet every write pays the instanceof chain in JsonFieldInfo.isEmpty plus an interface call to the codec. With NON_EMPTY as the default inclusion this makes writes of number-heavy beans about 2.2 times slower than in 1.7.3 (details and a reproducer in #4105).

What does this PR do?

• JsonWriterCodegen.neverEmpty decides the emptiness check while the writer is generated: when JsonFieldInfo.mayBeEmpty() rules out every built-in empty type (a final or enum declared type) and neither the property's write codec class nor any of its superclasses or superinterfaces declares isEmpty (only the JsonValueCodec default applies), the generated String and UTF-8 writers emit only the null check, and fieldWriteSize no longer counts a check that is not emitted.
• Everything else keeps the runtime call: non-final declared types, whose runtime value may still be a string, collection or map (this includes BigDecimal and BigInteger, which are not final), and codecs that override isEmpty, whether they come from registerCodec, a codec factory, @JsonCodec or a Mixin.
• The decision stays in code generation on purpose. Native images resolve types at run time without reflection metadata for codec methods, so the reflective isEmpty lookup must not run in JsonFieldInfo.resolveTypes.
• Generated writer classes are shared across ForyJson instances. The decision depends only on the declared type, fixed by the owner class, and on the write codec class, which the generated class key already carries (EXACT_CODEC, FACTORY, MIXIN); GeneratedCodecKeyBuilder.addRegistration now notes that dependency. The mayBeEmpty and isEmpty javadocs point at each other, since the built-in empty types must stay in sync.
• Tests in JsonGeneratedCodecTest:
  • nonEmptyFinalValues inspects the generated String and UTF-8 writer sources (no runtime check for Integer, Long, a plain enum, an enum with constant bodies and a final bean; the check kept for a final type with an isEmpty-overriding @JsonCodec, for BigDecimal and for Object), checks the written JSON, and compares every fixture, including a 31-property bean that moves the generated group boundaries, with the interpreted writer. The wide bean's source is checked too, and it is split into member helpers, so the grouped writer shape is covered. It fails without the codegen change.
  • nonEmptyRegisteredCodec builds two instances in one JVM, one with a default-emptiness codec and one with an isEmpty-overriding codec for the same final type, through registerCodec, a codec factory, a Mixin, a codec inheriting isEmpty from an interface and one inheriting it from an abstract base class. It checks that both instances run different generated UTF-8 writers, that only the overriding one keeps the runtime check, and that it still omits the empty value after the other one generated its writer. It fails if the codec override, or the interface or superclass part of the walk, is ignored.
  • builtInEmptyTypes asserts mayBeEmpty() for every type JsonFieldInfo.isEmpty tests itself, including an enum implementing CharSequence, so the two lists cannot drift.

Related issues

• Fixes [Java] JSON NON_EMPTY checks boxed, enum and BigDecimal properties at runtime since #4074 #4105

AI Contribution Checklist

• Substantial AI assistance was used in this PR: yes
• If yes, I included a completed AI Contribution Checklist in this PR description and the required AI Usage Disclosure.
• If yes, my PR description includes the required ai_review summary and screenshot evidence or equivalent persisted links of the final clean AI review results from both fresh reviewers described in AI_POLICY.md, the Fory-guided reviewer and the independent general reviewer, on the current PR diff or current HEAD after the latest code changes.

Completed checklist from AI_POLICY.md section 9:

• Substantial AI assistance was used in this PR: yes
• I included the standardized AI Usage Disclosure block below.
• I can explain and defend all important changes without AI help.
• I reviewed AI-assisted code changes line by line before submission.
• I completed line-by-line self-review first and fixed issues before requesting AI review.
• I ran two fresh AI review agents on the current PR diff or current HEAD after the latest code changes: one Fory-guided reviewer using AGENTS.md and .agents/ci-and-pr.md, and one independent general reviewer in a separate clean-context review session that was not pointed to .agents/ci-and-pr.md or any copied Fory-specific review checklist.
• I addressed all AI review comments and repeated the review loop until both ai reviewers reported no further actionable comments.
• I attached equivalent persisted links of the final clean AI review results from both fresh reviewers on the current PR head in this PR body.
• I ran adequate human verification and recorded evidence (checks run locally, pass/fail summary, and confirmation I reviewed results).
• I added/updated tests and specs where required.
• I validated protocol/performance impacts with evidence when applicable.
• I verified licensing and provenance compliance.

AI Usage Disclosure
- substantial_ai_assistance: yes
- scope: investigation, code drafting, tests, benchmark harness, PR text
- affected_files_or_subsystems: java/fory-json codegen (JsonWriterCodegen) and its generated-codec test
- ai_review: line-by-line self-review of the AI-drafted change is pending by the contributor (see unchecked items above). Two-reviewer loop on fresh sessions for every round, thirteen rounds. Fixed from review: reuse of JsonFieldInfo.mayBeEmpty instead of a second type list; no `&& true` / `if (true)` in generated code; group size estimate aligned with the emitted code; the decision kept in code generation after the Fory-guided reviewer found that moving it into resolveTypes breaks native-image resolution; tests for String and UTF-8 sources, enums with constant bodies, final beans, BigDecimal, a wide bean, registered/factory/Mixin codecs across instances, built-in empty type drift, and interpreted-writer equivalence. The codec override check walks the codec class, its superclasses and all superinterfaces instead of relying on Class.getMethod's choice between interface defaults. Kept by decision: BigDecimal/BigInteger stay on the runtime check (non-final), a Kotlin/Scala fixture for the requiresNonNullWrite arm is out of this Java-only change, no extra cache-key part (the write codec class is already pinned). Final round on the current head: both reviewers report no further actionable comments.
- ai_review_artifacts: Fory-guided reviewer: https://github.com/apache/fory/pull/4111#issuecomment-5926057531 ; independent general reviewer: https://github.com/apache/fory/pull/4111#issuecomment-5926057955 (both on commit 48dd8d80ee5624dc0341a71eff4dbc75ebefd9fd, final round, no further actionable comments; the current head 384b3824 is the same patch rebased onto main 7919241b with no conflicts, `git range-diff` reports it identical; earlier artifact comments refer to superseded commits)
- human_verification: see "Verification" below
- performance_verification: see "Benchmark" below
- provenance_license_confirmation: all code written for this PR against the Apache Fory sources; no third-party code introduced

Verification

JDK 25.0.2, Windows 11 x86_64, from java/:

• mvn -pl fory-json -am install -DskipTests: success
• mvn -pl fory-json test: 1272 tests, 0 failures, 0 errors, 0 skipped
• mvn -pl fory-json test -Dtest='JsonGeneratedCodecTest#nonEmptyFinalValues' with the JsonWriterCodegen change reverted: fails, as intended
• JsonGeneratedCodecTest#nonEmptyRegisteredCodec with the codec-override check disabled: fails, as intended
• JsonGeneratedCodecTest#builtInEmptyTypes with OptionalInt removed from mayBeEmpty, or with an early enum return in mayBeEmpty: fails, as intended
• JsonGeneratedCodecTest#nonEmptyRegisteredCodec with the interface or the superclass part of the override walk removed: fails, as intended
• mvn -pl fory-json spotless:check checkstyle:check: success, 0 Checkstyle violations

Does this PR introduce any user-facing change?

• Does this PR introduce any public API change? No.
• Does this PR introduce any binary protocol compatibility change? No. The JSON output is unchanged; only whether the writer asks the codec at runtime changes.

Benchmark

Standalone harness from #4105 (not JMH): a bean with 10 String, 20 Integer, 10 Long, 2 Double, 2 Boolean, 3 enum, 2 BigDecimal properties and one List<String>, all non-null, serialized with ForyJson.toJsonBytes after a 20 s warm-up; median of 5 rounds of 2,000,000 writes; one JVM per configuration, run one after another on an otherwise idle host; JDK 25.0.2, -Xms1g -Xmx1g. ns per write, two repetitions:

Build	NON_EMPTY	NON_NULL
main 341f732	761 / 753	339 / 331
this PR	349 / 341	330 / 349

The JSON written is identical between the two builds in both modes (compared by digest of the output JSON line). The two BigDecimal properties still take the runtime check; the remaining NON_EMPTY cost over NON_NULL is within noise here.

🤖 Generated with Claude Code

[pavel-ptashyts]

AI review artifact - Fory-guided reviewer (fresh session, prompted to follow AGENTS.md, .agents/ci-and-pr.md and .agents/languages/java.md; read-only, static review) on commit c65a64e, final round:

Reviewed commit c65a64ed6cf4d64adb79b9b31c9b5965e2df6ba2 as the Fory-guided reviewer.

No further actionable comments.

Residual testing gaps:

- Kotlin/Scala `requiresNonNullWrite` coverage is explicitly deferred.
- No native-image execution evidence was supplied.
- Review was static; contributor-reported tests and benchmarks were not independently rerun.

[pavel-ptashyts]

AI review artifact - independent general reviewer (separate clean-context session, not pointed to .agents/ci-and-pr.md or any Fory review checklist, following the independent-review carve-out in AGENTS.md; read-only) on commit c65a64e, final round:

Reviewed commit: c65a64ed6cf4d64adb79b9b31c9b5965e2df6ba2 (perf(java): skip runtime NON_EMPTY checks for final JSON property types)

No further actionable comments.

---

For transparency, the things I attacked and what resolved them:

Soundness of the predicate. mayBeEmpty() keys off writeTypeRef.getRawType() while JsonFieldInfo.isEmpty tests the runtime value. The over-approximation holds because neverEmpty only fires when the declared type is final (runtime class is the declared class) or an enum (runtime class is a constant body that cannot add interfaces), and every built-in branch of isEmpty is tested earlier in mayBeEmpty via isAssignableFrom/isArray on the declared type. writeRawType() used by nonEmptyValue equals writeTypeRef.getRawType() on this path, so the two never disagree about which branch would have been generated.

No NPE / no unreachable-type hazard. neverEmpty is only ever reached behind a short-circuited property.omitEmpty(), and omitEmpty is set only when writeRawType != null && !isPrimitive() && !writeUnboxedRequired (JsonFieldInfo.java:459). That guarantees writeTypeInfo was assigned in resolveTypes (JsonFieldInfo.java:946), and JsonTypeInfo.valueCodec is final and never null. void.class is isPrimitive(), so the KIND_NULL property never reaches it. Unwrapped group declaration properties (the ones without a write type info) only reach addWriteInclusion/omitDefault, never writeProp/fieldWriteSize.

The reflective lookup. getMethod("isEmpty", JsonWriter.class, Object.class) is the right probe, not a trap: any class that overrides isEmpty(JsonWriter, T) for a narrowed T necessarily carries a compiler-generated bridge with the erased descriptor declared in that class (JsonInclusionTest.EmptyObjectCodec is exactly this shape, and the new test asserts the check survives for it). AbstractJsonValueCodec does not declare isEmpty, so plain subclasses correctly resolve to the interface default. bindCodec is a pure cast — no wrapper can hide an override.

Generated-class sharing. generatedClassCache is static, so key completeness is load-bearing. Every way a property's valueCodec class can vary across ForyJson instances is pinned: registerCodec(type, codec) → exactCodecClass (a user codec can never be an ObjectCodec — private constructor); registerCodec(type, factory) and withModule → factoryKey, whose contract already requires it to cover "every option that can change the created codec class"; registerMixin → MIXIN; field/class @JsonCodec/@JsonFormat → fixed by the owner class, itself pinned by TARGET_MIXIN. The remaining exactCodecs defaults vary only with writeLongAsString / byteArrayFormat, both already writer key parts. The Scala module's isEmpty overrides (Option, None.type, collections, ranges) all arrive with a factory key.

Behavior-preserving rewrite. The writeNull and requiresNonNullWrite arms reuse the same write expression and the same two-armed Expression.If shape that already exists at JsonWriterCodegen.java:1560 and :1588, so no new codegen shape is introduced. wp<id> fields are still declared unconditionally in addWriterFields, so field/constructor layout is unchanged. The writeNull && omitEmpty combination is Kotlin/Scala-only (WRITE_NULL_MASK is set in the constructor only for Include.ALWAYS; includeNullWrite() is the only other setter), consistent with your scoping.

Sizing. fieldWriteSize dropping 12 for never-empty properties also feeds Utf8WriterCodegen.inlineSchemaWrites, whose predicate is a band (size <= 325 && size + expanded > 325), so some beans will change source shape in either direction. That is the designed consequence of a now-accurate estimate, and WideNonEmptyValues (31 properties ≈ 744) does exercise the grouped path, including the Invalid generated writer group boundaries invariant.

[pavel-ptashyts]

AI review artifact - Fory-guided reviewer (fresh session, prompted to follow AGENTS.md, .agents/ci-and-pr.md and .agents/languages/java.md; read-only, static review) on commit 48dd8d8, final round. Supersedes the earlier artifact comment.

Reviewed commit `48dd8d80ee5624dc0341a71eff4dbc75ebefd9fd` against `341f732b` as the Fory-guided reviewer.

No further actionable comments.

Residual testing gaps:

- Native-image and additional JDK validation evidence was not supplied.
- Kotlin/Scala `requiresNonNullWrite` coverage is explicitly deferred.
- The wide `NON_EMPTY` fixture checks generated source and output, but not generated method bytecode sizes.

Review was static only. No files were changed, and no git, builds, tests, benchmarks, or linters were run.

[pavel-ptashyts]

AI review artifact - independent general reviewer (separate clean-context session, not pointed to .agents/ci-and-pr.md or any Fory review checklist, following the independent-review carve-out in AGENTS.md; read-only) on commit 48dd8d8, final round. Supersedes the earlier artifact comment.

I reviewed 48dd8d80ee5624dc0341a71eff4dbc75ebefd9fd (perf(java): skip runtime NON_EMPTY checks for final JSON property types).

What I checked, and why I did not raise it:

Soundness of the generation-time decision. The decision is !mayBeEmpty() && !declaresIsEmpty(writeTypeInfo().valueCodec().getClass()), which has to be equivalent to what JsonFieldInfo.isEmpty would compute at run time.

• The built-in half holds. isEmpty dispatches on the runtime class, mayBeEmpty on the declared raw type; the final/enum clause is the only place that gap could open. A final declared type's runtime class is itself, and an enum constant body (Operation$1) is an anonymous subclass of the enum that cannot add interfaces and is never an array, so neither can become CharSequence/Collection/Map/Optional*/array. JsonInclusionTest.TextEnum — an enum that does implement CharSequence — is caught by the earlier CharSequence clause, and builtInEmptyTypes pins that. BuiltInEmptyValues covers every branch isEmpty currently tests.
• omitEmpty is only set when !writeUnboxedRequired (JsonFieldInfo.java:459-464), so writeRawType == writeTypeRef.getRawType() on this path and mayBeEmpty/nonEmptyValue agree on the class.
• The codec half holds. JsonTypeInfo.valueCodec is final and is not replaced when generated writer/reader capabilities are installed (JsonTypeInfo.java:63-64,103), so the codec read at generation is the one isEmpty dispatches to at run time. declaresIsEmpty walks the class, superclasses and the full interface closure, so bridge-only overrides, interface defaults and generic base classes are all found; the only false answers it can give are conservative ones (a static/unrelated isEmpty(JsonWriter, …) keeps the check). Only JsonValueCodec's default and ScalarCodecs' dynamic Object codec override isEmpty in main, and the latter is only reachable for a non-final declared type.

NPE risk on writeTypeInfo(). neverEmpty dereferences it only when mayBeEmpty() is false. Every call site (fieldWriteSize via groupEnds/inlineSchemaWrites, writeProp, presentValue) is fed from ObjectCodec.writeFields() or JsonUnwrappedInfo.writeFields() (DIRECT leaves only — JsonUnwrappedInfo.java:313-327), both of which have resolved write type info; GeneratedCodecKeyBuilder:165 already dereferences it unconditionally for the same arrays. Unwrapped group declarations, which do lack write type info, reach only the null/isDefault guard at JsonWriterCodegen.java:865-899 and never neverEmpty.

Key coverage. Beyond the scoped-out verification, I walked the public registration surface: registerCodec(Class, JsonValueCodec) → customTypeInfo → newRegisteredTypeInfo(..., null) sets exactCodecClass (EXACT_CODEC); registerCodec(Class, JsonCodecFactory) and modules carry a factoryKey (FACTORY); registerMixin is MIXIN plus TARGET_MIXIN; @JsonCodec/@JsonFormat/@JsonByteArray/@JsonValue are static and pinned by the owner class. Settings that swap a built-in codec class (writeLongAsString, byteArrayFormat) are key parts; onContentNullRead is not on the writer key, but it only re-binds ContainerJsonCodecs, which are always mayBeEmpty. addRegistration increments occurrence even when it emits nothing, so positional collisions can't occur.

Behaviour deltas. fieldWriteSize shrinking only affects method-splitting/inlining shape; firstGroupMember and the commaKnown predicates became more conservative than reality (a missed optimisation, not a miscount). The writeNull-plus-omitEmpty and requiresNonNullWrite arms behave as the commit intends.

Test. Both polarities of the "JsonFieldInfo.isEmpty(" source assertion are asserted in the same methods, so neither can go vacuous if the emitted spelling drifts; generatedUtf8WriterClass throws rather than returning null, so assertNotSame really does prove both instances ran generated writers; the generated-class cache is static (JsonCodegen.java:88), so the plain-then-registered ordering is a real shared-cache exercise; and generated output is cross-checked against the interpreted writer for every fixture.

No further actionable comments.