Conversation
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
- Fix reduce() signature mismatch in dyn_.rs - Fix as_any() ambiguity in downcast helper Signed-off-by: Nicholas Gates <nick@nickgates.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Merging this PR will degrade performance by 25.97%
Performance Changes
Comparing 
Footnotes
|
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com>
Signed-off-by: Nicholas Gates <nick@nickgates.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Benchmarks: TPC-H SF=10 on S3Verdict: No clear signal (environment too noisy confidence) datafusion / vortex-file-compressed (0.825x ➖, 5↑ 0↓)
datafusion / vortex-compact (0.856x ➖, 2↑ 0↓)
datafusion / parquet (0.673x ✅, 13↑ 0↓)
duckdb / vortex-file-compressed (0.961x ➖, 1↑ 0↓)
duckdb / vortex-compact (1.037x ➖, 0↑ 1↓)
duckdb / parquet (0.888x ➖, 1↑ 0↓)
Full attributed analysis
|
Contributor
Benchmarks: Statistical and Population GeneticsVerdict: No clear signal (low confidence) duckdb / vortex-file-compressed (0.969x ➖, 1↑ 0↓)
duckdb / vortex-compact (0.963x ➖, 0↑ 0↓)
duckdb / parquet (0.963x ➖, 0↑ 0↓)
Full attributed analysis
|
github-actions
bot
removed
the
action/benchmark
Contributor
Polar Signals Profiling ResultsLatest Run
Previous Runs (2)
Powered by Polar Signals Cloud |
Contributor
Benchmarks: Random AccessVortex (geomean): 0.841x ✅ unknown / unknown (0.920x ➖, 12↑ 1↓)
|
lwwmanning
reviewed
Apr 2, 2026
| return Ok(None); | ||
| }; | ||
|
|
||
| if cfg!(debug_assertions) { | ||
| vortex_ensure!( | ||
| result.as_ref().len() == parent.len(), | ||
| result.len() == parent.len(), |
Contributor
There was a problem hiding this comment.
probably worth doing this check even outside the debug_assertions block? dtype comparison can be expensive, but length should be quite fast
lwwmanning
approved these changes
Apr 2, 2026
Contributor
lwwmanning
left a comment
lwwmanning
left a comment
There was a problem hiding this comment.
minor nits, but worth merging asap as long as all the benchmark suites succeed / continue to be flat / better. it's a clear improvement from a code concepts point of view
github-actions
bot
removed
the
action/benchmark
Merged
a10y
added a commit
that referenced
this pull request
Apr 2, 2026
#7215 dropped PatchedArray and left this very descriptive doc comment hanging. I moved it to be a module level comment. Signed-off-by: Andrew Duffy <andrew@a10y.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #7181
This is a large break that changes how Vortex arrays are held in memory. Previously they were modeled as Arc where all array impls used a vtable macro to unsafely transmute into a dyn Array.
This was cute, but meant that we were performing a lot of unnecessary clones and Arc allocations for things that should never have required them. We were also limited by how expressive we could make APIs since Arc is considered a foreign type.
This PR makes ArrayRef a struct, makes typed arrays a struct of
Array<V: VTable>, and adds anArrayView<'a, V>. This means all arrays are Arc allocated up-front in any of these forms, and we pass the Arc allocation around everywhere allowing us to cheaply clone in either the type-erased or typed structs.The main user-facing change is that array constructors should now live on the vtable struct, e.g.
Primitive::new(...)instead ofPrimitiveArray::new(...)