-
Notifications
You must be signed in to change notification settings - Fork 115
switch Buffer -> BufferHandle in PrimitiveArray #5929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
67fabe3 to
5d5797b
Compare
Merging this PR will improve performance by ×2.9
Performance Changes
Comparing Footnotes
|
|
I wonder if we can remove at lot of these buffer calls from tests? |
|
I don't think there's another way to get the values out now |
9e2ba3b to
ca0b46b
Compare
Benchmarks: Random AccessSummary
|
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
Benchmarks: TPC-H SF=1 on S3Summary
Detailed Results Table
|
Benchmarks: TPC-DS SF=1 on NVMESummary
Detailed Results Table
|
Benchmarks: TPC-H SF=10 on NVMESummary
Detailed Results Table
|
Benchmarks: FineWeb S3Summary
Detailed Results Table
|
Benchmarks: Statistical and Population GeneticsSummary
Detailed Results Table
|
Benchmarks: TPC-H SF=10 on S3Summary
Detailed Results Table
|
Benchmarks: CompressionSummary
Detailed Results Table
|
Benchmarks: Clickbench on NVMESummary
Detailed Results Table
|
gatesn
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is quite right
f673643 to
fe957da
Compare
Signed-off-by: Andrew Duffy <[email protected]>
Fleshes out the BufferHandle type more.
I hid the inner enum and introduced some extra methods to build/unwrap BufferHandles.
This removes the
PrimitiveArray::as_slicemethod, replacing it with aninto_buffer::<T>method which may allocate and copy a new buffer (if the handle points to device memory), or else provides a new Buffer pointing to the existing host memory.This PR is large but a lot of it is just adding
&'s and updating test code to use thebuffer!macro