hcrosse opened a new pull request, #1444: URL: https://github.com/apache/arrow-go/pull/1444
### Rationale for this change Reading a row group in small batches repeatedly copies the same dictionary. With `SetReadDict` enabled, `pqarrow` copies the whole dictionary into every batch, so the cost grows with dictionary size and batch count. I measured synthetic input with 2M 25-byte strings in one row group, with all data pages encoded as `RLE_DICTIONARY`. I have not included the benchmark program in this PR. I can add it as a `pqarrow` benchmark if that would help. The table shows median times with `SetReadDict` enabled. `K` is the number of distinct values and `B` is the batch size. The issue describes the full setup. | `K` | `B` | Main → patched, ns/row | Change | | --- | ---: | ---: | ---: | | 10k | 8,192 | 21.480 → 5.846 | -72.78% | | 10k | 65,536 | 7.198 → 5.924 | -17.70% | | 100k | 8,192 | 187.95 → 12.68 | -93.26% | | 100k | 65,536 | 35.11 → 12.49 | -64.43% | | 1M | 8,192 | 1,972.5 → 162.4 | -91.77% | | 1M | 65,536 | 362.9 → 145.2 | -59.99% | Run-to-run spread was 7% to 39% on main. All six rows have p=0.002 in benchstat with n=6. Main and patched runs were interleaved on one 8-vCPU VM. I found no significant time change without `SetReadDict`, with one batch per row group, or with `K=100`. Allocated bytes fell by about 25% to 95% in the same six cases. For example, at `K=1M` and `B=8,192` they fell from 7,116 MiB to 366 MiB per row-group read. ### What changes are included in this PR? The dictionary builder now keeps the last full dictionary it built. It returns that dictionary again until new values are added. The builder drops it on `ResetFull`, on checkpoint restore and on its final `Release`. After the dictionary grows, the next full dictionary replaces it. `NewDelta` with a nonzero offset still copies only the new entries. I documented the sharing and retained reference on `DictionaryBuilder.NewDictionaryArray`. Five `TimestampWithOffset` tests in `arrow/extensions` never released their builders, which now hold a cached dictionary. I added the missing `Release` calls. ### Are these changes tested? I added five tests that check: - Reuse without growth, including null indices. A shared dictionary stays valid after one of its arrays is released. After growth the builder rebuilds the dictionary once, then reuses it again. - Rebuilding after `ResetFull`, even when the dictionary length is unchanged. - Empty and nonempty `NewDelta` results with a cached dictionary. - Shared dictionaries and correct values for `Int64` and `FixedSizeBinary`. - Rebuilding after checkpoint restore, even when the dictionary length is unchanged. I added a dictionary pointer check to `TestIncrementalReads`. Existing `TestReadWholeFileDict` covers resets between row groups. Each new test fails with the matching cache or lifetime bug reintroduced, including missed invalidation on reset or restore and incorrect delta handling. On Linux with the Arrow and Parquet test data configured, these pass: - `go test -tags assert,test ./arrow/array/... ./arrow/extensions/... ./parquet/...` - `go test -race -tags assert,test ./arrow/array/... ./parquet/pqarrow/...` - `go vet`, `golangci-lint` and `gofmt -l` on the three changed packages I also checked the IPC writers with a short test that is not included here. The stream writer sent one full dictionary and one delta, and the batches read back equal, as on main. The file writer accepted an unchanged dictionary and rejected a grown one with the same error as main. ### Are there any user-facing changes? Yes. Consecutive arrays from one builder can now share one dictionary `*Data`. The builder may keep a reference to that dictionary until `ResetFull` or its final `Release`. A caller that never releases its builder now retains the cached dictionary even after releasing its arrays. There is no API change. Closes https://github.com/apache/arrow-go/issues/1443. -- 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]
