JingsongLi commented on PR #10226:
URL: https://github.com/apache/paimon/pull/10226#issuecomment-5944772984
Requirement fit: SUPPORTED. Implementation: FINDINGS at `37857ace36`.
Reusing decoded dictionaries has value, but these two regressions should be
fixed before merging.
**[P2] Adopt the dictionary once per top-level reconstruction**
`ShreddingUtils.java:201` calls `cachedMetadata.adopt()` for every partially
shredded nested object. Each call compares the entire top-level metadata
buffer. With an array of N objects having distinct leftover keys, metadata
grows with N, so rebuilding one value now scans O(N²) bytes. This affects
ordinary full-variant/JSON reconstruction through `BaseVariantReader` and
`assembleVariantBatch`, even when every row shares metadata.
I benchmarked actual `PaimonShreddingUtils.assembleVariant` against the
exact base implementation on JDK 8/G1. The schema is `{items:
array<Row<typed:int>>}`, and each input element contains `typed` plus a
distinct `leftover_i` field. Base/head outputs have identical canonical JSON
and SHA-256. After warmup, median reconstruction times in a second JVM fork
were:
| Elements | Base | This PR |
| ---: | ---: | ---: |
| 1,000 | 233 µs | 582 µs |
| 5,000 | 1,448 µs | 11,647 µs |
| 10,000 | 2,393 µs | 46,520 µs |
A separate fork reproduced approximately 2.5×/9.3×/20.9× slowdowns. These
are reconstruction timings, not claimed storage-throughput measurements. Keep
the cross-row cache, but compare/adopt at most once after each `setCurrent()`;
preserve detection of reused backing arrays between rows.
**[P2] Compare direct buffers independently of their byte order**
`BinaryRowDataUtil.java:125-126` compares numeric `getLong()` results using
each buffer's byte order. A little-endian snapshot and a default big-endian
direct buffer with reversed eight-byte chunks can therefore compare equal
despite different bytes, causing stale dictionary keys to be used.
I reproduced wrong output through the existing public
`BaseVariantReader.read(row, ByteBuffer)` API using valid canonical 16-byte
metadata:
```text
A = [1,1,0,12,12,0,1,1,97,98,99,100,101,102,103,104]
B = [1,1,0,12,12,0,1,1,104,103,102,101,100,99,98,97]
```
Both contain one valid UTF-8 key. Warm the reader with A, then provide B in
a direct buffer with its default byte order: the shared reader returns A's
field name, while a fresh reader and the base implementation return B's correct
field name. The rows were created through `castShredded` using an empty object
shredding schema; no truncated or padded metadata is involved. Internal vector
callers explicitly use little endian, so this is the narrower public ByteBuffer
path. Compare raw bytes or normalize duplicate buffers to the same order
without mutating caller state, and cover both false matches and false misses.
Validation: on both JDK 8 and JDK 11, the relevant common/serializer and
real Parquet shredding write/read suites executed 106 tests (105 passed, 1
skipped) with normal Maven checks. Current Java/integration CI jobs were
cancelled, so completed green integration results are still needed after fixing
the regressions.
--
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]
