Skip to content

ARROW-15994: [C++] Back out taskify changes - #12720

Closed
sanjibansg wants to merge 3 commits into
apache:masterfrom
sanjibansg:thread_tasks
Closed

ARROW-15994: [C++] Back out taskify changes#12720
sanjibansg wants to merge 3 commits into
apache:masterfrom
sanjibansg:thread_tasks

Conversation

@sanjibansg

Copy link
Copy Markdown
Contributor

This PR modifies the SubmitTask and Finish methods of MapNode in ExecPlan to avoid scheduling extra thread tasks.

Performed the TPC-H Benchmark developed in PR #12537 with and without the changes.

TPC-H Benchmark (With Extra Thread Tasks)
-------------------------------------------------------------------
Benchmark Time CPU Iterations
-------------------------------------------------------------------
BM_Tpch_Q1/ScaleFactor:1 95035633 ns 178700 ns 100
TPC-H Benchmark (Without Extra Thread Tasks)
-------------------------------------------------------------------
Benchmark Time CPU Iterations
-------------------------------------------------------------------
BM_Tpch_Q1/ScaleFactor:1 91511754 ns 182060 ns 100

Also, tested with the Query Tester as proposed in PR #12586

With Thread Tasks (batch size = 4096)
./query_tester tpch-1
Average Duration: 0.106694s (+/- 0s)
Average Output Rows/S: 37.4902rps
Average Output Bytes/S: 4573.81bps
Without Thread Tasks (batch size = 4096)
./query_tester tpch-1
Average Duration: 0.104658s (+/- 0s)
Average Output Rows/S: 38.2198rps
Average Output Bytes/S: 4662.82bps

@github-actions

Copy link
Copy Markdown

@pitrou
pitrou requested a review from westonpaceMarch 28, 2022 13:13

@westonpacewestonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it's a small benefit to performance but a bigger benefit to readability. One small nit and then I think this is good.

Comment threadcpp/src/arrow/compute/exec/exec_plan.cc Outdated
Comment threadcpp/src/arrow/compute/exec/exec_plan.cc Outdated
@ursabot

ursabot commented Mar 30, 2022

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = ac58b8d and contender = 756f2b7. 756f2b7 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️0.46% ⬆️0.59%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.43% ⬆️0.81%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/413|756f2b72 ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/399|756f2b72 test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/399|756f2b72 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/409|756f2b72 ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/412|ac58b8d5 ec2-t3-xlarge-us-east-2>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/398|ac58b8d5 test-mac-arm>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/398|ac58b8d5 ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/408|ac58b8d5 ursa-thinkcentre-m75q>
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

@sanjibansg
sanjibansg deleted the thread_tasks branch March 30, 2022 01:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@sanjibansg@ursabot@westonpace