-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
GH-41547: [C++] Thirdparty: Upgrade xsimd to 13.0.0 #41548
Conversation
Thanks for opening a pull request! If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project. Then could you also rename the pull request title in the following format?
or
In the case of PARQUET issues on JIRA the title also supports:
See also: |
|
@github-actions crossbow submit -g cpp -g wheel |
This comment was marked as outdated.
This comment was marked as outdated.
@github-actions crossbow submit -g cpp -g wheel |
Revision: 66a9a37 Submitted crossbow builds: ursacomputing/crossbow @ actions-a6192919cb |
Have we seen this before 🤔? |
I don't think so, but it's certainly unrelated. |
BTW, in these few days the conan CI checked in, should we also switch to xsimd 13.0.0 in conan? |
I have no idea. @kou What do you think? |
Does it mean that "should we update arrow/ci/conan/all/conanfile.py Line 227 in 657c4fa
If so, we don't need to do it. It should be the same as the upstream: https://github.com/conan-io/conan-center-index/blob/133dcd62540ae212e68197b49a40947df4ff7c80/recipes/arrow/all/conanfile.py#L205 If it means that "should we update https://github.com/conan-io/conan-center-index/blob/133dcd62540ae212e68197b49a40947df4ff7c80/recipes/arrow/all/conanfile.py#L205 ?", it's better that we update. But a conan user/developer may do it eventually. |
All right, how can we make sure 3rd are ready? I may use some feature introduced after 9.0 later, how should I checks that? |
Sorry. What the "3rd" refers? (You want to check xsimd version in our code?) |
Ah I mean all the third-party libraries about xsimd |
@github-actions crossbow submit -g cpp -g wheel |
Revision: 29f8781 Submitted crossbow builds: ursacomputing/crossbow @ actions-81b3fe46a8 |
@pitrou I've check that the two failed CI is remote storage related, and one is acero-hash-join unittest, could we move forward? |
@mapleFU This is ok to me, feel free to merge. |
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.
+1
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 522b097. There were 12 benchmark results indicating a performance regression:
The full Conbench report has more details. It also includes information about 23 possible false positives for unstable benchmarks that are known to sometimes produce them. |
### Rationale for this change Arrow now uses xsimd 9.0.1, currently, some conversion for batch is now support in neon, see: apache#40335 (comment) . So we can upgrading it. The xsimd currently released 13.0.0, see: xtensor-stack/xsimd#1015 For conan, seems community is updating it: conan-io/conan-center-index#23859 . Maybe we can wait for a while ### What changes are included in this PR? Update xsimd to 13.0.0 ### Are these changes tested? Tested by existing test code ### Are there any user-facing changes? no * GitHub Issue: apache#41547 Authored-by: mwish <maplewish117@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Rationale for this change
Arrow now uses xsimd 9.0.1, currently, some conversion for batch is now support in neon, see: #40335 (comment) . So we can upgrading it.
The xsimd currently released 13.0.0, see: xtensor-stack/xsimd#1015
For conan, seems community is updating it: conan-io/conan-center-index#23859 . Maybe we can wait for a while
What changes are included in this PR?
Update xsimd to 13.0.0
Are these changes tested?
Tested by existing test code
Are there any user-facing changes?
no