You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/09/07 01:27:04 UTC
[kudu-CR] [threadpool] optimize queue overload detection
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16424
Change subject: [threadpool] optimize queue overload detection
......................................................................
[threadpool] optimize queue overload detection
This patch simplifies the queue overload detection and makes it a bit
more robust, according to 'perf stat' running
KUDU_ALLOW_SLOW_TESTS=1 perf stat ./bin/threadpool-test \
--gtest_filter='*ThreadPoolPerformanceTest.ConcurrentAndSerialTasksMix/1'
Before:
147699.960062 task-clock # 44.348 CPUs utilized
371,519 context-switches # 0.003 M/sec
653 cpu-migrations # 0.004 K/sec
3,664 page-faults # 0.025 K/sec
423,794,029,838 cycles # 2.869 GHz
94,656,186,092 instructions # 0.22 insns per cycle
34,018,899,188 branches # 230.324 M/sec
22,374,862 branch-misses # 0.07% of all branches
3.330492839 seconds time elapsed
After:
126357.374737 task-clock # 40.768 CPUs utilized
350,907 context-switches # 0.003 M/sec
3,167 cpu-migrations # 0.025 K/sec
3,478 page-faults # 0.028 K/sec
362,348,476,629 cycles # 2.868 GHz
82,964,368,394 instructions # 0.23 insns per cycle
29,553,336,891 branches # 233.887 M/sec
16,945,558 branch-misses # 0.06% of all branches
3.099419461 seconds time elapsed
Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
---
M src/kudu/util/threadpool.cc
M src/kudu/util/threadpool.h
2 files changed, 24 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/16424/1
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
Patch Set 1:
> I am curious what prompted this change?
I was thinking about the TODO of keeping running minimum of N numbers while on vacation :) It came to me that keeping the exact minimum was not necessary.
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Sep 2020 17:12:44 +0000
Gerrit-HasComments: No
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
Patch Set 1: Code-Review+1
I am curious what prompted this change?
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Sep 2020 14:22:55 +0000
Gerrit-HasComments: No
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
[threadpool] optimize queue overload detection
This patch simplifies the queue overload detection and makes it a bit
more robust, according to 'perf stat' running
KUDU_ALLOW_SLOW_TESTS=1 perf stat ./bin/threadpool-test \
--gtest_filter='*ThreadPoolPerformanceTest.ConcurrentAndSerialTasksMix/1'
Before:
147699.960062 task-clock # 44.348 CPUs utilized
371,519 context-switches # 0.003 M/sec
653 cpu-migrations # 0.004 K/sec
3,664 page-faults # 0.025 K/sec
423,794,029,838 cycles # 2.869 GHz
94,656,186,092 instructions # 0.22 insns per cycle
34,018,899,188 branches # 230.324 M/sec
22,374,862 branch-misses # 0.07% of all branches
3.330492839 seconds time elapsed
After:
126357.374737 task-clock # 40.768 CPUs utilized
350,907 context-switches # 0.003 M/sec
3,167 cpu-migrations # 0.025 K/sec
3,478 page-faults # 0.028 K/sec
362,348,476,629 cycles # 2.868 GHz
82,964,368,394 instructions # 0.23 insns per cycle
29,553,336,891 branches # 233.887 M/sec
16,945,558 branch-misses # 0.06% of all branches
3.099419461 seconds time elapsed
Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Reviewed-on: http://gerrit.cloudera.org:8080/16424
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/util/threadpool.cc
M src/kudu/util/threadpool.h
2 files changed, 24 insertions(+), 15 deletions(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, but someone else must approve
Andrew Wong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h@348
PS1, Line 348: ssize_t
> nit: can this be negative? If not, maybe just use size_t?
In current implementation it should not become negative, no.
However, making changes in the code that tracks this number might lead to such an outcome. I'm not sure what's best: change it to size_t and add DCHECK()/CHECK() before the lines decrementing the counter or keep this member field to be of ssize_t type?
It seems keeping ssize_t makes the code safer in terms of future maintenance, so I'd prefer to keep ssize_t at this point.
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Sep 2020 20:56:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h@348
PS1, Line 348: ssize_t
> In current implementation it should not become negative, no.
Fair point. SGTM.
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Sep 2020 20:58:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] [threadpool] optimize queue overload detection
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16424 )
Change subject: [threadpool] optimize queue overload detection
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h
File src/kudu/util/threadpool.h:
http://gerrit.cloudera.org:8080/#/c/16424/1/src/kudu/util/threadpool.h@348
PS1, Line 348: ssize_t
nit: can this be negative? If not, maybe just use size_t?
--
To view, visit http://gerrit.cloudera.org:8080/16424
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic01ca7419beba92d7067c60ef520811136d67587
Gerrit-Change-Number: 16424
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 08 Sep 2020 19:12:20 +0000
Gerrit-HasComments: Yes