-
Notifications
You must be signed in to change notification settings - Fork 174
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
[CHORE] Refactor local hash joins + pipeline connections #2719
Conversation
CodSpeed Performance ReportMerging #2719 will improve performances by 51.6%Comparing Summary
Benchmarks breakdown
|
Ok(Box::pin(mp_stream)) | ||
} | ||
|
||
fn chunk_tables_into_micropartition_stream( |
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.
No need to chunk in source anymore, since ops do their own buffering.
177c072
to
7c76de2
Compare
&self, | ||
idx: usize, | ||
input: &PipelineResultType, | ||
state: Option<&mut Box<dyn IntermediateOperatorState>>, |
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 think this api can also be Option<&mut dyn IntermediateOperatorState>
instead of the Box of one
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 tried, but had a bit of difficulty. I believe this is because performing a as_deref_mut
on Option<Box<dyn IntermediateOperatorState>>
returns Option<&mut dyn IntermediateOperatorState + 'static>>
, which outlives the original Option<Box<dyn IntermediateOperatorState>>
. https://users.rust-lang.org/t/whats-the-lifetime-of-the-output-of-as-deref-mut-of-option-box-dyn-trait/105002/2
Pipeline Channel
instead of nothing.