You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/09/21 10:03:38 UTC
[GitHub] [arrow] pitrou opened a new pull request, #14188: ARROW-17792: [C++] Use lambda capture move construction
pitrou opened a new pull request, #14188:
URL: https://github.com/apache/arrow/pull/14188
Simplify some code using functors for the purpose of capturing a moved value.
I did not modify all such places, only where readability would seem to improve.
--
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 #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #14188:
URL: https://github.com/apache/arrow/pull/14188#issuecomment-1255546357
Benchmark runs are scheduled for baseline = b0d74bd0ee964d30d715fe6123c87a901bad369e and contender = 842d48238909af38ace2a24e4ce2c2f0d7a372d9. 842d48238909af38ace2a24e4ce2c2f0d7a372d9 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/d9fd42490fd941c792fa9f1700059cfe...b3d7d810ba9945189ca29d843f12231f/)
[Failed :arrow_down:0.99% :arrow_up:0.14%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/84a569d179db4b94a4eded6990db1797...1c6e940b285740a08fb80876f2b4fff6/)
[Failed :arrow_down:0.27% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/2cdab6f6d6e346cbb9e887e293df68b7...11e6eee6837942fcae7d530af8b79598/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/f527db4859c746eb8b25afedd9ddb21c...19f109d6812c457bb136104764f652e7/)
Buildkite builds:
[Finished] [`842d4823` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1545)
[Failed] [`842d4823` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1562)
[Failed] [`842d4823` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1545)
[Finished] [`842d4823` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1559)
[Finished] [`b0d74bd0` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1544)
[Failed] [`b0d74bd0` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1561)
[Failed] [`b0d74bd0` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1544)
[Finished] [`b0d74bd0` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1558)
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] github-actions[bot] commented on pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #14188:
URL: https://github.com/apache/arrow/pull/14188#issuecomment-1253496945
https://issues.apache.org/jira/browse/ARROW-17792
--
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] kou commented on pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
kou commented on PR #14188:
URL: https://github.com/apache/arrow/pull/14188#issuecomment-1255527889
> @kou Have you already seen this error? https://github.com/apache/arrow/actions/runs/3096909253/jobs/5012993282#step:10:202
>
> ```
> [58/106] Generating symbol file arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
> FAILED: arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
> "C:/msys64/ucrt64/bin/meson" "--internal" "symbolextractor" "D:/a/arrow/arrow/build/c_glib" arrow-flight-glib/libarrow-flight-glib-1000.dll "arrow-flight-glib/libarrow-flight-glib.dll.a" arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
> ```
Sorry. I missed this.
I have seen the error but it was disappeared without any changes... So I don't know why this is happened yet.
--
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] pitrou merged pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
pitrou merged PR #14188:
URL: https://github.com/apache/arrow/pull/14188
--
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] pitrou commented on pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
pitrou commented on PR #14188:
URL: https://github.com/apache/arrow/pull/14188#issuecomment-1253571150
@kou Have you already seen this error? https://github.com/apache/arrow/actions/runs/3096909253/jobs/5012993282#step:10:202
```
[58/106] Generating symbol file arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
FAILED: arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
"C:/msys64/ucrt64/bin/meson" "--internal" "symbolextractor" "D:/a/arrow/arrow/build/c_glib" arrow-flight-glib/libarrow-flight-glib-1000.dll "arrow-flight-glib/libarrow-flight-glib.dll.a" arrow-flight-glib/libarrow-flight-glib-1000.dll.p/libarrow-flight-glib-1000.dll.symbols
```
--
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] westonpace commented on a diff in pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
westonpace commented on code in PR #14188:
URL: https://github.com/apache/arrow/pull/14188#discussion_r977911397
##########
cpp/src/arrow/dataset/dataset_writer.cc:
##########
@@ -174,19 +174,9 @@ class DatasetWriterFileQueue {
}
void ScheduleBatch(std::shared_ptr<RecordBatch> batch) {
- // TODO(ARROW-17110) This task is only here to std::move the batch
- // It can be a lambda once we adopt C++14
- struct WriteTask : public util::AsyncTaskScheduler::Task {
- WriteTask(DatasetWriterFileQueue* self, std::shared_ptr<RecordBatch> batch)
- : self(self), batch(std::move(batch)) {}
- Result<Future<>> operator()(util::AsyncTaskScheduler*) {
- return self->WriteNext(std::move(batch));
- }
- DatasetWriterFileQueue* self;
- std::shared_ptr<RecordBatch> batch;
- };
- scheduler_->AddTask(
- ::arrow::internal::make_unique<WriteTask>(this, std::move(batch)));
+ scheduler_->AddSimpleTask([self = this, batch = std::move(batch)]() {
Review Comment:
Do we want to specify all arguments in this way or just the ones that we need to capture specially? E.g. Is `[self = this, batch = std::move(batch)]` preferred over `[this, batch = std::move(batch)]`?
--
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] pitrou commented on a diff in pull request #14188: ARROW-17792: [C++] Use lambda capture move construction
Posted by GitBox <gi...@apache.org>.
pitrou commented on code in PR #14188:
URL: https://github.com/apache/arrow/pull/14188#discussion_r977913171
##########
cpp/src/arrow/dataset/dataset_writer.cc:
##########
@@ -174,19 +174,9 @@ class DatasetWriterFileQueue {
}
void ScheduleBatch(std::shared_ptr<RecordBatch> batch) {
- // TODO(ARROW-17110) This task is only here to std::move the batch
- // It can be a lambda once we adopt C++14
- struct WriteTask : public util::AsyncTaskScheduler::Task {
- WriteTask(DatasetWriterFileQueue* self, std::shared_ptr<RecordBatch> batch)
- : self(self), batch(std::move(batch)) {}
- Result<Future<>> operator()(util::AsyncTaskScheduler*) {
- return self->WriteNext(std::move(batch));
- }
- DatasetWriterFileQueue* self;
- std::shared_ptr<RecordBatch> batch;
- };
- scheduler_->AddTask(
- ::arrow::internal::make_unique<WriteTask>(this, std::move(batch)));
+ scheduler_->AddSimpleTask([self = this, batch = std::move(batch)]() {
Review Comment:
That's a good question. I think either way is ok.
--
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