Neuw84 opened a new pull request, #18260: URL: https://github.com/apache/iceberg/pull/18260
Closes #18259. `ParquetValueReaders.StringReader` decodes every value with `column.nextBinary().toStringUsingUTF8()`. When a page is dictionary-encoded, Parquet's dictionary reader hands back the same `Binary` instance for every occurrence of a dictionary entry. The reader still decodes each of them into a new `String`. This shows up when position deletes are loaded. `BaseDeleteLoader.readPosDeletes` reads a whole position delete file through the generic reader. `file_path` is dictionary-encoded and, since the spec sorts position deletes by `file_path`, it repeats for thousands of rows in a row. We profiled a Spark 4.1 `MERGE INTO` over a v2 table with position and equality deletes (8 executors, JFR on all of them). Loading the position deletes was 10.5 % of the target-scan stage, even with #17864 applied. Inside that load, `StringReader.read` for `file_path` was 22.9 % of the samples. **Change** `StringReader` keeps the last `Binary` it read and the `String` it produced. When the next value is the same instance, it returns that `String` without decoding. - **Results:** unchanged. Strings are immutable, and a different instance (plain pages, the next dictionary entry, a new row group's dictionary) is decoded as before. - **Reused buffers:** a `Binary` whose backing bytes may be reused (`isBackingBytesReused()`) is always decoded. - **Knock-on effect:** with the same instance coming back, the path comparison added in #17864 (`String.contentEquals`) ends at the reference check. **Results** A JMH benchmark (`PositionDeleteLoadBenchmark`, added here) loads a whole position delete file through `BaseDeleteLoader`, as an executor-cache miss does. The file has 64 data files × 30,000 positions, sorted, with object-store-length paths. Measured with 3 forks × 10 iterations of 2 s on JDK 21: | | time per load | |---|---| | `main` | 143.3 ± 0.8 ms | | this change | **113.1 ± 2.6 ms (1.27×)** | **Tests** `TestParquetStringReaderReuse`: - Repeated position-delete paths come back as one `String` instance per dictionary entry, with correct values. The identity check fails on `main`. - Plain-encoded strings, including repeated values, read correctly. - A dictionary that overflows into plain pages, across several row groups, reads correctly. Also run: - `:iceberg-parquet:test`: 759 tests, 0 failures. - `:iceberg-data` `TestGenericReaderDeletes` and `org.apache.iceberg.data.parquet.*`: 0 failures. - spotless and checkstyle. **Related:** #17864 (per-row path lookup in `toPositionIndexes`); #16440 and #16052, larger changes to position-delete reading, closed for needing more discussion. This one is limited to one reader method. -- 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]
