perf: add fast path for uniform fill values in `array_resize` by lyne7-sc · Pull Request #20617 · apache/datafusion

lyne7-sc · 2026-02-28T14:58:07Z

Which issue does this PR close?

Closes #.

Rationale for this change

array_resize currently does extra per-row work when resizing list arrays. This pr optimizes the common fast path where the fill value is uniform.

What changes are included in this PR?

Add a fast path in array_resize for uniform fill values.
Precompute the maximum required growth and reuse a single fill buffer in the uniform-fill path.

Benchmarks

group                                                main                                   optimized
-----                                                ----                                   ---------
array_resize_i64/grow_default_null_fill_10_to_500    11.24     3.2±0.07ms        ? ?/sec    1.00   283.9±20.10µs        ? ?/sec
array_resize_i64/grow_uniform_fill_10_to_500         4.15  1648.0±38.12µs        ? ?/sec    1.00   397.2±20.46µs        ? ?/sec
array_resize_i64/grow_variable_fill_10_to_500        1.00  1667.7±50.31µs        ? ?/sec    1.02  1692.8±61.54µs        ? ?/sec
array_resize_i64/mixed_grow_shrink_1000x_100         4.83    373.0±8.91µs        ? ?/sec    1.00     77.1±5.90µs        ? ?/sec
array_resize_i64/shrink_uniform_fill_500_to_10       1.00      8.1±0.51µs        ? ?/sec    1.06      8.5±0.50µs        ? ?/sec

Are these changes tested?

Yes

Are there any user-facing changes?

No

neilconway

Nice work!

Can you check that the SLT tests cover the new fast-path?

neilconway · 2026-03-01T13:29:46Z

+
+    // Fast path: at least one row needs to grow and all rows share
+    // the same fill value.
+    if is_uniform_fill {


The naming is a bit confusing: is_uniform_fill is false if the fill value is uniform but there are no rows that need to grow. How about use_batch_fill or use_bulk_fill or similar?

I think use_bulk_fill is pretty clear, I renamed it to that, thanks

neilconway · 2026-03-01T13:30:23Z

+            if extra > max_extra {
+                max_extra = extra;
+            }


Suggested change

if extra > max_extra {

max_extra = extra;

}

max_extra = max_extra.max(extra);

neilconway · 2026-03-01T13:39:00Z

+            if start + count > offset_window[1] {
+                let extra_count = (start + count - offset_window[1]).to_usize().unwrap();
+                let end = offset_window[1];
+                mutable.extend(0, (start).to_usize().unwrap(), (end).to_usize().unwrap());


(Minor) Why (start)? Parens unnecessary, here and below.

neilconway · 2026-03-01T13:40:47Z

+
+        let mut null_builder = NullBufferBuilder::new(array.len());
+
+        for (row_index, offset_window) in array.offsets().windows(2).enumerate() {


The fast path and slow path are doing very similar work and there's a bunch of duplicate code. I wonder if it would be possible to refactor them -- the key difference is just how the fill itself is done, no?

yes, the main difference is the fill behavior, and the surrounding control flow is very similar. I’ll take a look to see what can be cleanly refactored there.

I refactored it to pull the shared loop out of the fast and slow paths.

neilconway · 2026-03-01T13:47:54Z

+        let default_element = fill_scalar.to_array_of_size(max_extra)?;
+        let default_value_data = default_element.to_data();
+
+        let capacity = Capacities::Array(original_data.len() + default_value_data.len());


Is this right? default_value_data.len() is the per-row growth, I think we want the total estimated output size.

thanks for the reminder. I added capacity calculation in the pre-scan and use for the allocation.

lyne7-sc · 2026-03-01T16:13:33Z

Thanks for the review @neilconway

The existing SLTs already hit the fast path, but I added another array test case to cover the multi-row case more explicitly.

neilconway · 2026-04-03T15:40:06Z

@lyne7-sc Can you resolve the merge conflicts?

lyne7-sc · 2026-04-07T06:28:23Z

@lyne7-sc Can you resolve the merge conflicts?

Sure, the conflicts have been resolved.

lyne7-sc · 2026-04-11T06:37:06Z

Hi @alamb — gentle ping, this PR has been open for a while. I’ve rebased and resolved conflicts, would appreciate a review when you have time.

alamb

Thanks @lyne7-sc and @neilconway -- this looks good to me

alamb · 2026-04-11T09:54:24Z

-                        "array_resize: failed to convert size to i64"
-                    )
-                })?;
+            let extra_count = (start + count - offset_window[1]).to_usize().unwrap();


why was this changed to unwrap (which will panic) from a internal error?

Maybe because the calls below to unwrap() would have panic'd anyways

Yes, this would panic in the same cases as the unwrap() calls below.

a more robust option might be to add an overflow check here. Do you think that would be worth adding?

Let's add the check as a follow on PR

alamb · 2026-04-16T18:38:29Z

I merged thsi PR up from main to resolve some conflicts

…#20617) ## Which issue does this PR close?  - Closes #. ## Rationale for this change  `array_resize` currently does extra per-row work when resizing list arrays. This pr optimizes the common fast path where the fill value is uniform. ## What changes are included in this PR? - Add a fast path in `array_resize` for uniform fill values. - Precompute the maximum required growth and reuse a single fill buffer in the uniform-fill path. ### Benchmarks ``` group main optimized ----- ---- --------- array_resize_i64/grow_default_null_fill_10_to_500 11.24 3.2±0.07ms ? ?/sec 1.00 283.9±20.10µs ? ?/sec array_resize_i64/grow_uniform_fill_10_to_500 4.15 1648.0±38.12µs ? ?/sec 1.00 397.2±20.46µs ? ?/sec array_resize_i64/grow_variable_fill_10_to_500 1.00 1667.7±50.31µs ? ?/sec 1.02 1692.8±61.54µs ? ?/sec array_resize_i64/mixed_grow_shrink_1000x_100 4.83 373.0±8.91µs ? ?/sec 1.00 77.1±5.90µs ? ?/sec array_resize_i64/shrink_uniform_fill_500_to_10 1.00 8.1±0.51µs ? ?/sec 1.06 8.5±0.50µs ? ?/sec ```  ## Are these changes tested? Yes  ## Are there any user-facing changes? No   --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>

lyne7-sc added 3 commits February 28, 2026 22:38

optimize array_resize

80d7558

lint

e2ccc63

lint

9d390b4

github-actions Bot added the functions Changes to functions implementation label Feb 28, 2026

neilconway reviewed Mar 1, 2026

View reviewed changes

lyne7-sc and others added 2 commits March 2, 2026 00:01

Merge branch 'main' into perf/array_resize

8f229a6

suggestion by review

cf8d870

github-actions Bot added the sqllogictest SQL Logic Tests (.slt) label Mar 1, 2026

lyne7-sc and others added 2 commits March 2, 2026 22:35

refactor deduplicate fill paths

6ba8c7c

Merge branch 'main' into perf/array_resize

fcfff46

lyne7-sc added 2 commits April 7, 2026 13:50

Merge branch 'main' into perf/array_resize

d62c125

toml lint

3b049f7

Merge branch 'main' into perf/array_resize

628fba2

alamb approved these changes Apr 11, 2026

View reviewed changes

Merge remote-tracking branch 'apache/main' into perf/array_resize

77fc5d0

alamb enabled auto-merge April 16, 2026 18:38

alamb added this pull request to the merge queue Apr 16, 2026

Merged via the queue into apache:main with commit 1068686 Apr 16, 2026
34 checks passed


		let mut null_builder = NullBufferBuilder::new(array.len());

		for (row_index, offset_window) in array.offsets().windows(2).enumerate() {

Conversation

lyne7-sc commented Feb 28, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Benchmarks

Are these changes tested?

Are there any user-facing changes?

Uh oh!

neilconway left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lyne7-sc commented Mar 1, 2026

Uh oh!

neilconway commented Apr 3, 2026

Uh oh!

lyne7-sc commented Apr 7, 2026

Uh oh!

lyne7-sc commented Apr 11, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alamb commented Apr 16, 2026 •

edited

Loading