apache / apache/arrow-rs

parquet: record API allocates a Vec plus one String per column, per row, in Reader::read

Open
#10,468 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 14h
Merged PRs (30d)
167

Description

**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` — 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` (`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` (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::(...).read_records(num_rows, None, None, &mut buf)`
into one pre-allocated `Vec` per column (`Vec`, `Vec`, `Vec` ×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.

Contributor guide

Open the contributing guide

Research direction

Start by reading parquet/src/record/reader.rs and parquet/src/record/api.rs, especially the cited Vec, field-name, and UTF8 conversion paths, then reproduce the issue's three benchmark variants. The desired API shape is not decided: done depends on maintainer agreement about a non-breaking optimization, a major-release change, or documentation and benchmark coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
data-engineering
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.