perf(parquet/pqarrow): use synchronous path for serial reads - #1235
perf(parquet/pqarrow): use synchronous path for serial reads#1235fallintoplace wants to merge 2 commits into
Conversation
zeroshade
left a comment
There was a problem hiding this comment.
The serial-path performance and allocation improvements are reproducible, and the standard tests pass. However, the synchronous path removes an existing panic-recovery boundary, causing malformed decoder or extension-array input to panic callers rather than return an error.
| return nil, err | ||
| } | ||
|
|
||
| data, err := fr.ReadColumn(rowGroups, rdr) |
There was a problem hiding this comment.
ReadColumn previously ran inside the worker’s deferred recovery boundary, which converted decoder panics into panic while reading: … errors. Calling it directly here lets those panics escape from ReadTable.
I reproduced this through the public FileReader flow with a malformed extension implementation: the base returns the recovered error, while this head panics. Please preserve the recovery boundary for synchronous reads—ideally through a shared guarded helper—and add regression coverage.
Summary
ArrowReadProperties.Parallelis false by default.GetFieldReadersnow uses a direct loop for serial reads.ReadRowGroupsnow reads columns directly for serial reads.Benchmark
Apple M1 Pro. Median of 5 runs. The benchmark uses
Parallel=false.Command:
Tests
go test ./parquet/pqarrow -count=1go test -race ./parquet/pqarrow -run '^(TestGetFieldReadersReleasesPartialReadersOnError|TestRecordReaderSerial|TestRecordReaderSeekToRow|TestRecordReaderParallel)$' -count=1go vet ./parquet/pqarrow