You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "felipecrv (via GitHub)" <gi...@apache.org> on 2023/04/10 13:54:53 UTC
[GitHub] [arrow] felipecrv opened a new pull request, #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
felipecrv opened a new pull request, #35002:
URL: https://github.com/apache/arrow/pull/35002
### Rationale for this change
Make it easier to implement correct kernels dealing with run-end encoded data.
### What changes are included in this PR?
Extraction of code to a header so kernels like REE filter kernels (see #35001) can use them.
### Are these changes tested?
Indirectly via the `run_end_encode` and `run_end_decode` tests.
### Are there any user-facing changes?
No, these are internal compute headers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] benibus commented on a diff in pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "benibus (via GitHub)" <gi...@apache.org>.
benibus commented on code in PR #35002:
URL: https://github.com/apache/arrow/pull/35002#discussion_r1165696100
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -402,6 +402,7 @@ list(APPEND
compute/registry.cc
compute/kernels/codegen_internal.cc
compute/kernels/row_encoder.cc
+ compute/kernels/ree_util_internal.cc
Review Comment:
Alright, sounds good. Just wanted to make sure that it was intentional.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35002:
URL: https://github.com/apache/arrow/pull/35002#issuecomment-1510215794
Benchmark runs are scheduled for baseline = 913c34506000fd77444ea7b7f24dcb6402003d6b and contender = ff94702eee6ee419329d32d86c14bf0bf0c244f3. ff94702eee6ee419329d32d86c14bf0bf0c244f3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/3c950ce7405f4470b8a2434ec4cae3b7...42d5864f31694dd9acdee18949d31cc4/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/40715969bd1c46b2a9e8f3f98a731aab...1af69ab61ad145b78d4fe38f07be03dc/)
[Finished :arrow_down:1.53% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b84ebda404c1446fbea9af2fbe9c5b16...acf048d615a44a3e86cc7cbc3873e23c/)
[Failed :arrow_down:0.52% :arrow_up:0.09%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/fbecb251db08468d9c274b0a5a1eca7a...bf31db6dc3bb46cab087f7298a602870/)
Buildkite builds:
[Finished] [`ff94702e` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2710)
[Failed] [`ff94702e` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2744)
[Finished] [`ff94702e` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2708)
[Finished] [`ff94702e` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2735)
[Finished] [`913c3450` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2709)
[Failed] [`913c3450` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2743)
[Finished] [`913c3450` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2707)
[Failed] [`913c3450` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2734)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35002:
URL: https://github.com/apache/arrow/pull/35002#issuecomment-1510218404
['Python', 'R'] benchmarks have high level of regressions.
[ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b84ebda404c1446fbea9af2fbe9c5b16...acf048d615a44a3e86cc7cbc3873e23c/)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] felipecrv commented on a diff in pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on code in PR #35002:
URL: https://github.com/apache/arrow/pull/35002#discussion_r1164730135
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -402,6 +402,7 @@ list(APPEND
compute/registry.cc
compute/kernels/codegen_internal.cc
compute/kernels/row_encoder.cc
+ compute/kernels/ree_util_internal.cc
Review Comment:
`RunEndEncode` is extremely useful when writing unit tests, so I would consider this "core" and `RunEndDecode` will probably play a central role in `compute` as something we can reach out to whenever a function can't handle REE directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] zeroshade merged pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #35002:
URL: https://github.com/apache/arrow/pull/35002
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] felipecrv commented on pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "felipecrv (via GitHub)" <gi...@apache.org>.
felipecrv commented on PR #35002:
URL: https://github.com/apache/arrow/pull/35002#issuecomment-1505694265
@zeroshade @benibus
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] benibus commented on a diff in pull request #35002: MINOR: [C++] Make the utilities used to implement run_end_encode available to other compute kernels
Posted by "benibus (via GitHub)" <gi...@apache.org>.
benibus commented on code in PR #35002:
URL: https://github.com/apache/arrow/pull/35002#discussion_r1164516798
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -402,6 +402,7 @@ list(APPEND
compute/registry.cc
compute/kernels/codegen_internal.cc
compute/kernels/row_encoder.cc
+ compute/kernels/ree_util_internal.cc
Review Comment:
Does this need to be included in the default build or should it be conditionally compiled when `ARROW_COMPUTE=ON` alongside `vector_run_end_encode.cc` in [this block](https://github.com/apache/arrow/blob/3a3d1ccde4023cb769eb8a2c10531ee6dce666dc/cpp/src/arrow/CMakeLists.txt#L428)? If you expect the utils will be used outside of the REE-kernels then it's probably fine where it is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org