Skip to content

PatchedArray: basics and wiring#7002

Open
a10y wants to merge 7 commits intodevelopfrom
aduffy/patched-array
Open

PatchedArray: basics and wiring#7002
a10y wants to merge 7 commits intodevelopfrom
aduffy/patched-array

Conversation

@a10y
Copy link
Contributor

@a10y a10y commented Mar 17, 2026

Summary

This is the first PR in a series addressing the PatchedArray RFC: vortex-data/rfcs#27

This PR adds a new PatchedArray array variant, which is slated to only be used with FastLanes array types. The design is largely documented in the RFC, but briefly

  • Wraps another child array
  • Holds a buffer of offsets and patch values. They are both sorted by chunk/lane so there is efficient random as well as bulk access for CUDA

We are able to pushdown the following at reduce time:

  • Filters pushes through to the child, and we trim the patch indices to the range of chunks covered by the filter
  • Slicing that reduces the chunk range
  • Compare always pushes through to the child
  • Take pushes through to the inner then constructs a hashmap of the patches

There will be follow ups to add the wiring into CUDA and to update how BitPacked and ALP arrays are written.

Testing

There are unit tests for all of the reducers and kernels

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 17, 2026

Merging this PR will not alter performance

✅ 1009 untouched benchmarks
⏩ 1515 skipped benchmarks1


Comparing aduffy/patched-array (35bfb5f) with develop (8efe1dc)

Open in CodSpeed

Footnotes

  1. 1515 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@a10y a10y added the changelog/feature A new feature label Mar 18, 2026
@a10y a10y marked this pull request as ready for review March 18, 2026 21:21
@a10y a10y requested a review from joseph-isaacs March 18, 2026 21:21
Comment on lines +51 to +53
pub(super) indices: BufferHandle,
/// patch values corresponding to the indices. The ptype is specified by `values_ptype`.
pub(super) values: BufferHandle,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really think we should reconsider this.

This array (+ constant) should be used to replace the sparse array so I think we really want to store things other than PTypes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this array is well-suited for non-primitive types. How many lanes does a List<Struct{u32,utf8}> have? I was tempted to actually put this into the vortex-fastlanes crate tbh

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've updated to use child array for values, which should give us some flexibility to change things in the future

return Ok(None);
}

let sliced = array.slice_chunks(chunk_start..chunk_stop)?;
Copy link
Contributor

@joseph-isaacs joseph-isaacs Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you cannot do this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? this doesn't access buffers just slices

a10y added 6 commits March 19, 2026 09:24
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
@a10y a10y force-pushed the aduffy/patched-array branch from c3d7434 to 6c1d7aa Compare March 19, 2026 13:24
Signed-off-by: Andrew Duffy <andrew@a10y.dev>
Comment on lines +27 to +31
AllOr::Some(indices) => {
let first = indices[0];
let last = indices[indices.len() - 1];

(first / 1024, last.div_ceil(1024))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You really want to take(1) and on the slices iterator and backwards slice iterator here

Comment on lines +52 to +53
/// indices within a 1024-element chunk. The PType of these MUST be u16
pub(super) indices: BufferHandle,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my feeling is that we want this to be an array too in-case we later do want to compress this array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/feature A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants