-
Notifications
You must be signed in to change notification settings - Fork 127
feat[btrblocks]: add a dynamic btr blocks compressor #6207
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
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
Benchmarks: TPC-H SF=1 on NVMESummary
Detailed Results Table
|
Benchmarks: Random AccessSummary
|
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: Clickbench on NVMESummary
Detailed Results Table
|
Benchmarks: CompressionSummary
Detailed Results Table
|
a054b7a to
68c8d87
Compare
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
68c8d87 to
1536f8a
Compare
Benchmarks: FineWeb NVMeSummary
Detailed Results Table
|
| /// This struct is passed through recursive compression calls to specify | ||
| /// which schemes should be excluded at each level. | ||
| #[derive(Debug, Clone, Copy, Default)] | ||
| pub struct Excludes<'a> { |
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.
it might be nice to make this owned and then this is what gets passed into the schemes? needing to build new_excludes is a bit messy
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.
Wanted to avoid the move/alloc? What would you do instead?
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.
yea that's fair. maybe we can flup but i think having a stack-allocated set might clean things up here
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.
what do you mean? This allows that?
|
approach here looks fine overall |
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.
we probably want to make it possible to plugin new int/float/string schemes in the future?
|
Yep, should be easier to do that in a follow up |
No description provided.