Niqnil opened a new issue, #10468:
URL: https://github.com/apache/arrow-rs/issues/10468
**Which part of Arrow is this about?** Parquet (`parquet::record`, the
row/record API).
### Is your feature request related to a problem or challenge?
`Reader::read` rebuilds per-row bookkeeping that is constant for the whole
file. Reading an
N-row, C-column file performs `N × (1 + C)` allocations before any user code
touches a value.
Three separate costs, in `parquet/src/record/reader.rs` and
`parquet/src/record/api.rs` on `main`:
**1. `Vec` without `with_capacity`** — `reader.rs:425` (and again at `:459`
for the nested/group
path). The final length is exactly `readers.len()`, known immediately:
```rust
Reader::GroupReader(_, _, ref mut readers) => {
let mut fields = Vec::new(); // <- capacity is known:
readers.len()
for reader in readers {
fields.push((String::from(reader.field_name()),
reader.read_field()?));
}
Ok(Row::new(fields))
}
```
**2. A fresh `String` per column name, per row** — `reader.rs:427` (and
`:464`, `:467`). Column
names are fixed by the schema and cannot change between rows.
`Reader::field_name` (`reader.rs:532-543`)
returns `&str` borrowed from a `TypePtr` — i.e. an `Arc<Type>` — so the name
is *already*
refcounted upstream; `String::from` discards that and heap-allocates a copy
for every row.
**3. A full byte-copy for every UTF8 field** — `api.rs:753`:
```rust
let value = String::from_utf8(value.data().to_vec())?;
Field::Str(value)
```
`ByteArray` is backed by `Option<Bytes>` (`data_type.rs:178-180`), which is
refcounted, so the copy
is not required by ownership — only by `Field::Str`'s `String` payload. This
is most visible on
dictionary-encoded low-cardinality columns, where the same handful of values
are re-materialised on
every row.
### Measurement
Real-world shape: single-symbol market-data tick files, 4 flat primitive
columns
(`ts INT64/TIMESTAMP_MICROS`, `symbol BYTE_ARRAY/UTF8`, `bid DOUBLE`, `ask
DOUBLE`), one row group,
ZSTD, all columns `RLE_DICTIONARY`. Synthetic data, `--release` with
`opt-level=3, lto=true,
codegen-units=1`, 7 iterations, steady-state (cold first runs excluded):
| | 370,000 rows | 65,000 rows |
|---|---|---|
| **A** — `RowIter` + `RowAccessor`, full decode | ~687 ns/row | ~674 ns/row
|
| **B** — low-level `ColumnReader::read_records` into reused buffers,
*identical* per-row conversions | ~472 ns/row | ~466 ns/row |
| **A′** — `RowIter`, row materialised then dropped, no accessor called |
~280 ns/row | ~278 ns/row |
| **A − B (row-materialisation overhead)** | **~215 ns/row (~31%)** | **~208
ns/row (~31%)** |
A and B perform the same downstream work per row (UTF8 comparison,
`DateTime::from_timestamp_micros`,
two `Decimal::try_from(f64)`), so `A − B` isolates exactly the three costs
above. The figure holds to
within 1 percentage point across a 5.7× row-count difference, so it is a
stable per-row constant
rather than measurement noise. A′ shows roughly 40% of the row-API path is
spent building `Row`
before a single value is read.
Concretely, in that 370,000-row file the `symbol` column occupies **639
compressed bytes** — it is one
repeated value, fully dictionary-encoded — yet the record API allocates
370,000 short-lived `String`s
for it, plus 370,000 more for the four column names, plus 370,000 `Vec`s.
### Describe the solution you'd like
Splitting these by API impact, since they are not equally free:
**(1) is non-breaking:** `Vec::with_capacity(readers.len())` at
`reader.rs:425` and `:459`. No API
change, no semantic change.
**(2) and (3) are not, because `Row`'s payload type is public.**
`Row::new(fields: Vec<(String, Field)>)` (`api.rs:56`) and
`RowColumnIter::Item = (&'a String, &'a Field)` (`api.rs:129`) both expose
`String`, so:
- For (2), storing `Arc<str>` (cloned once per file from the existing
`TypePtr`) instead of `String`
would remove the per-row allocation entirely, but changes `Row::new`'s
parameter type and
`RowColumnIter`'s item to `(&'a str, &'a Field)`.
- For (3), having `Field::Str` retain the refcounted `ByteArray` and expose
`&str` on access would
remove the copy, but changes `Field`'s public shape.
Both look like candidates for a major release rather than a patch, which is
why this is an issue
rather than a PR. **Is any of the three something you'd want changed, and in
what form?** If the
breaking variants aren't wanted, a fully non-breaking alternative for
(2)/(3) is to leave the types
alone and document the row API's per-row allocation cost, so callers can
make an informed choice
between it and the column API.
### Describe alternatives you've considered
- **Using the low-level `ColumnReader` API** (benchmark B above) — works,
~31% faster, but gives up
the record API's transparent multi-row-group iteration and its
`Row::is_null` handling of optional
columns, which is a meaningful amount of correctness surface to
re-implement per project.
- **Using `parquet::arrow`'s `ParquetRecordBatchReader`** — not viable for
us: it requires the `arrow`
feature, pulling
`arrow-array`/`-buffer`/`-data`/`-schema`/`-select`/`-ipc` plus `base64`, and we
deliberately depend on `parquet` with `default-features = false` to keep
that stack out.
### Reproducing
Everything below is synthetic; no real data is needed.
**Fixture** — write with `SerializedFileWriter` +
`WriterProperties::builder().set_compression(Compression::ZSTD(Default::default()))`,
one row group, default dictionary settings:
```
message schema {
REQUIRED INT64 ts (TIMESTAMP_MICROS);
REQUIRED BINARY symbol (UTF8);
REQUIRED DOUBLE bid;
REQUIRED DOUBLE ask;
}
```
- `ts`: `1_774_000_000_000_000 + i * 137` (strictly ascending micros)
- `symbol`: the constant `"EURUSD"` on every row — this is the single-symbol
case, and it is what
makes the per-row `String::from_utf8` copy so visible
- `bid`: `1.10000 + (i % 1000) as f64 * 1e-5`; `ask`: `bid + 0.00012`
- Row counts: 370,000 and 65,000
**Three variants**, each timing only the decode loop (file open and
`SerializedFileReader::new`
outside the timer), 7 iterations, discarding cold runs:
- **A** — `reader.get_row_iter(None)`, and per row: `get_string(1)` compared
against `"EURUSD"`,
`get_timestamp_micros(0)` → `DateTime::from_timestamp_micros`,
`get_double(2)` and `get_double(3)`
→ `Decimal::try_from`, pushed into a `Vec` of a 3-field struct.
- **B** — `get_row_group(0)`, then
`get_typed_column_reader::<T>(...).read_records(num_rows, None, None, &mut buf)`
into one pre-allocated `Vec` per column (`Vec<i64>`, `Vec<ByteArray>`,
`Vec<f64>` ×2), then zip by
index performing *exactly* A's per-row conversions — except `symbol` is
compared as raw bytes via
`ByteArray::data()` rather than through a `String`.
- **A′** — A's iterator, but `black_box(&row)` with no accessor called at
all.
`std::hint::black_box` on the accumulated output in every variant to prevent
elision.
**Environment for the numbers above:** rustc 1.95.0, `parquet` 59.1.0 with
`default-features = false, features = ["zstd"]`, release profile with
`opt-level = 3, lto = true, codegen-units = 1`, AMD Ryzen 7 5800U, Linux.
### Additional context
`parquet/benches/` doesn't appear to cover per-row allocation on the record
API today, so this cost
is currently invisible to CI.
--
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]
