apache / apache/arrow-rs

[Epic] Replace `ArrayData` with direct Array construction, when possible

Open
#9,298 0 comments 0 reactions 0 assignees View on GitHub
enhancement performance
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

**Is your feature request related to a problem or challenge? Please describe what you are trying to do.**
- related to https://github.com/apache/arrow-rs/issues/9061

While we work on micro micro optimizations, we have seen a common pattern where older parts of the arrow-rs codebase use `ArrayData` to create new arrays.

An ArrayData has at least one extra allocation (for the Vec that holds `Buffer`s) as well as a bunch of dynamic function calls. While this overhead is small individually, it is paid for every array so in aggregate it can be substantial

It also typically requires an `unsafe` call which is unnecessary as the new APIs can be checked by the compiler.

Quoting @tustvold

> My 2 cents is it would be better to move the codepaths relying on ArrayData over to using the typed arrays directly, this should not only cut down on allocations but unnecessary validation and dispatch overheads.

**Describe the solution you'd like**
Change relying on ArrayData over to creating the typed arrays directly, this should not only cut down on allocations but unnecessary validation and dispatch overheads.

**Describe alternatives you've considered**
Here are some example PRs
- https://github.com/apache/arrow-rs/pull/9122
- https://github.com/apache/arrow-rs/pull/9120

the old, less efficient pattern looks like this (note the `vec![buffer]` to create a buffer).

```rust
let data = unsafe {
ArrayData::new_unchecked(T::DATA_TYPE, len, None, Some(null), 0, vec![buffer], vec![])
};
PrimitiveArray::from(data)
```

or

```rust
let array_data = ArrayDataBuilder::new(arrow_data_type)
.len(self.record_reader.num_values())
.add_buffer(record_data)
.null_bit_buffer(self.record_reader.consume_bitmap_buffer());

let array_data = unsafe { array_data.build_unchecked() };
```

The new pattern looks like this (note no unsafe or allocations)

```rust
// Create nulls directly (note the `filter` to avoid nulls)
let nulls =
Some(NullBuffer::new(BooleanBuffer::new(null, 0, len))).filter(|n| n.null_count() > 0);
// Create Primitive Array directly
PrimitiveArray::new(ScalarBuffer::from(buffer), nulls)
```

** Note the only tricky thing I have seen is that `ArrayDataBuilder` automatically checks / drops NullBuffers that have no nulls. When updating the code we need to follow a similar pattern

**Additional context**
- [ ] https://github.com/apache/arrow-rs/issues/9128

Contributor guide

Open the contributing guide

Research direction

Start by locating ArrayData and ArrayDataBuilder construction sites across the arrow-rs codebase, using PRs 9122 and 9120 as examples of the intended migration. Replace eligible paths with direct typed-array construction while preserving null-buffer handling; done when the relevant paths avoid unnecessary unsafe calls and allocations without changing behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
performance
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.