stevenschlansker opened a new pull request, #3717:
URL: https://github.com/apache/fory/pull/3717
## Why?
Avoid redundant re-computation of compact codec offsets layout, saving cpu
and memory allocations
and
fix(format): honor input offset when copying inline-struct bytes
## What does this PR do?
perf(format): cache compact row layout per nested slot
Hoist offsets, widths, and nullability into CompactRowLayout, built
once per encoder. getStruct does no per-call lookup; inline-struct
reads go directly to the parent buffer; the writer reads schema-
derived state from the cache; compact rows skip the unused extData[]
allocation. For List<Struct> fields the parent layout also carries
the element layout so getArray(i) does not rebuild it.
Fix a latent inline-width bug in CompactBinaryRowWriter that was
masked by the prior slicing getStruct: writeUnaligned and
writeAlignedBytes dropped the input offset, so re-emitting a child
row via BinaryWriter.write(int, BinaryRow) copied the wrong bytes.
## Related issues
## AI Contribution Checklist
- [ ] Substantial AI assistance was used in this PR: `yes` / `no`
AI Usage Disclosure
- substantial_ai_assistance: yes
- scope: all
- affected_files_or_subsystems: Java row format
- ai_review: :heavy_check_mark:
- ai_review_artifacts:
> One commit, perf(format): cache compact row layout per nested slot. Caches
fixedOffsets, per-field fixedWidths, allFieldsNotNullable, and
bitmapWidthInBytes in a new CompactRowLayout value, computed once per array
(element type is fixed) or memoized per parent extData slot.
> The getStruct hot path previously rebuilt fixedOffsets and re-walked
allNotNullable on every call.
> Recommendation: Approve. Numbers in the commit message match a real
bottleneck (the int[] + allNotNullable walk per nested decode), the
implementation preserves the existing wire behavior and ownership model, no
public API regression, all format tests pass, and the new caching follows the
existing extData-slot pattern rather than inventing a new mechanism.
> — Reviewed by Claude (Opus 4.7), 2026-05-29
> Skill used: I ran this through the fory-code-review skill, which delegated
the diff inspection to a read-only review subagent loaded with the Fory review
checklist, the lesson-derived red flags, and the Java runtime rules.
> Verdict
> No blockers. Recommend submission. Both commits are coherent and tested:
> - d5cbdc2b is a real, narrow bugfix: the three put/copyFrom(..., 0,
numBytes) sites in CompactBinaryRowWriter were copying from the start of the
source buffer instead of from baseOffset. After schema sorting, nested inline
structs land at non-zero offsets, so this corrupted writes for any non-trivial
schema. CompactCodecTest.testCopyInlineStructViaWriter exercises that exact
failure mode. Wire format unchanged — only the source byte range was wrong.
> - 2b1825d7 precomputes a CompactRowLayout once per writer instead of
recomputing field offsets and nested layouts on every write. Cache is
per-CompactBinaryRowWriter instance, no map, no growth, no thread-safety
surface (writers are already not thread-safe). CompactRowLayoutTest pins
identity via assertSame at multiple nesting levels and covers the
allFieldsNotNullable bitmap-skip path. CompactBinaryRow.getStruct also drops a
per-call buffer.slice() allocation by pointing to the parent buffer at the
right offset instead.
- human_verification: :heavy_check_mark:
- performance_verification: :heavy_check_mark:
- provenance_license_confirmation: :heavy_check_mark:
## Does this PR introduce any user-facing change?
Compact codec user enjoys more CPU and memory on business logic without even
realizing it.
No public API or wire changes.
## Benchmark
```
RowFormatAllocationProbe bytes/op vs apache/main, compact mode,
median of five:
root 100208 -> 74305 (-25.9%)
array 9032 -> 6629 (-26.6%)
matrix 72005 -> 52760 (-26.7%)
map 7744 -> 6401 (-17.3%)
encode 55423 -> 41762 (-24.6%)
Standard mode unchanged within noise.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
