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
[C++] Possibly an unnecessary copy when outputting join result batch #35124
Comments
westonpace
pushed a commit
that referenced
this issue
Apr 18, 2023
…35114) ### Rationale for this change Seems this https://github.com/apache/arrow/blob/913c34506000fd77444ea7b7f24dcb6402003d6b/cpp/src/arrow/acero/swiss_join_internal.h#L571 is an unnecessary copy when outputting join result batch. According to Sasha K.: > I _think_ the compiler will automatically emit the move because it sees we’re copying from an object that’ll never be used again [1], but adding the std::move would be good just to remove any ambiguity. ### What changes are included in this PR? Adding `std::move()` to disambiguate/eliminate the unnecessary copy. ### Are these changes tested? No. This change is possibly undetectable, and the existing tests should cover it well. ### Are there any user-facing changes? No. * Closes: #35124 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Weston Pace <weston.pace@gmail.com>
liujiacheng777
pushed a commit
to LoongArch-Python/arrow
that referenced
this issue
May 11, 2023
…ult (apache#35114) ### Rationale for this change Seems this https://github.com/apache/arrow/blob/913c34506000fd77444ea7b7f24dcb6402003d6b/cpp/src/arrow/acero/swiss_join_internal.h#L571 is an unnecessary copy when outputting join result batch. According to Sasha K.: > I _think_ the compiler will automatically emit the move because it sees we’re copying from an object that’ll never be used again [1], but adding the std::move would be good just to remove any ambiguity. ### What changes are included in this PR? Adding `std::move()` to disambiguate/eliminate the unnecessary copy. ### Are these changes tested? No. This change is possibly undetectable, and the existing tests should cover it well. ### Are there any user-facing changes? No. * Closes: apache#35124 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Weston Pace <weston.pace@gmail.com>
ArgusLi
pushed a commit
to Bit-Quill/arrow
that referenced
this issue
May 15, 2023
…ult (apache#35114) ### Rationale for this change Seems this https://github.com/apache/arrow/blob/913c34506000fd77444ea7b7f24dcb6402003d6b/cpp/src/arrow/acero/swiss_join_internal.h#L571 is an unnecessary copy when outputting join result batch. According to Sasha K.: > I _think_ the compiler will automatically emit the move because it sees we’re copying from an object that’ll never be used again [1], but adding the std::move would be good just to remove any ambiguity. ### What changes are included in this PR? Adding `std::move()` to disambiguate/eliminate the unnecessary copy. ### Are these changes tested? No. This change is possibly undetectable, and the existing tests should cover it well. ### Are there any user-facing changes? No. * Closes: apache#35124 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Weston Pace <weston.pace@gmail.com>
rtpsw
pushed a commit
to rtpsw/arrow
that referenced
this issue
May 16, 2023
…ult (apache#35114) ### Rationale for this change Seems this https://github.com/apache/arrow/blob/913c34506000fd77444ea7b7f24dcb6402003d6b/cpp/src/arrow/acero/swiss_join_internal.h#L571 is an unnecessary copy when outputting join result batch. According to Sasha K.: > I _think_ the compiler will automatically emit the move because it sees we’re copying from an object that’ll never be used again [1], but adding the std::move would be good just to remove any ambiguity. ### What changes are included in this PR? Adding `std::move()` to disambiguate/eliminate the unnecessary copy. ### Are these changes tested? No. This change is possibly undetectable, and the existing tests should cover it well. ### Are there any user-facing changes? No. * Closes: apache#35124 Authored-by: zanmato1984 <zanmato1984@gmail.com> Signed-off-by: Weston Pace <weston.pace@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
See this line:
arrow/cpp/src/arrow/acero/swiss_join_internal.h
Line 571 in 913c345
According to the email discussion with @save-buffer :
Component(s)
C++
The text was updated successfully, but these errors were encountered: