constant array metadata for small scalars#6363
Merged
onursatici merged 1 commit intodevelopfrom Feb 9, 2026
Merged
Conversation
Signed-off-by: Onur Satici <onur@spiraldb.com>
Merging this PR will improve performance by 10.65%
Performance Changes
Comparing 
Footnotes
|
onursatici
added
changelog/performance
github-actions
bot
removed
the
action/benchmark-sql
Contributor
Benchmarks: PolarSignals ProfilingSummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=1 on S3Summary
Detailed Results Table
|
Contributor
Benchmarks: TPC-DS SF=1 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on NVMESummary
Detailed Results Table
|
Contributor
Benchmarks: FineWeb S3Summary
Detailed Results Table
|
Contributor
Benchmarks: Statistical and Population GeneticsSummary
Detailed Results Table
|
Contributor
Benchmarks: TPC-H SF=10 on S3Summary
Detailed Results Table
|
Comment on lines
+94
to
+102
| // Prefer reading the scalar from inlined metadata to avoid device-to-host copies. | ||
| let sv = if let Some(ref proto_bytes) = metadata.scalar_value { | ||
| ScalarValue::from_protobytes(proto_bytes)? | ||
| } else { | ||
| if buffers.len() != 1 { | ||
| vortex_bail!("Expected 1 buffer, got {}", buffers.len()); | ||
| } | ||
| let buffer = buffers[0].clone().try_to_host_sync()?; | ||
| ScalarValue::from_protobytes(&buffer)? |
Contributor
There was a problem hiding this comment.
so we duplicate them?
Contributor
Author
There was a problem hiding this comment.
for small constants yea, to keep forward compat
Contributor
Author
There was a problem hiding this comment.
also if not and we need the scalar in the device we would need to issue a h2d copy anyway, these values are small enough to just duplicate I think
Contributor
There was a problem hiding this comment.
does this ever actually get triggered?
Contributor
Author
There was a problem hiding this comment.
I don't think it does for clickbench or tpch
Contributor
Author
There was a problem hiding this comment.
but we do have constant arrays and they can't be read to the gpu without an extra host roundtrip, so constantarray::build is definitely triggered on a clickbench scan
Contributor
There was a problem hiding this comment.
can we find out what scalar this is
Contributor
Benchmarks: Clickbench on NVMESummary
Detailed Results Table
|
joseph-isaacs
approved these changes
Feb 9, 2026
gatesn
pushed a commit
that referenced
this pull request
Feb 11, 2026
Changes the metadata for `ConstantArray` to `Option<Scalar>`. #6363 added functionality for inlining the constant scalar if it was small enough in the array metadata. This change adds to that by using a scalar directly for this. It is optional since we do not always want to do this. Tests incoming! Also, I think it is worth considering if we should _not_ write the scalars to the buffers if we make this optimization. As is, this is doing double work. --------- Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
added a commit
that referenced
this pull request
Feb 12, 2026
connortsui20
added a commit
that referenced
this pull request
Feb 13, 2026
connortsui20
added a commit
that referenced
this pull request
Feb 13, 2026
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.
scalar value would be stored in the metadata for small scalars as well as in the buffer, this would help decoding a constant array in the gpu without forcing the buffer value to host, and gpu kernels can still use the constant value from the buffer