You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2022/02/14 08:55:33 UTC
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18229
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
[thread] Small refactor to improve ThreadMgr performance
Move threads_started_metric_ and threads_running_metric_ out of lock and
use atomic to improve performance.
Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
---
M src/kudu/util/thread.cc
1 file changed, 18 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/18229/1
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18229
to look at the new patch set (#2).
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
[thread] Small refactor to improve ThreadMgr performance
Move threads_started_metric_ and threads_running_metric_ out of lock and
use atomic to improve performance.
Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
---
M src/kudu/util/thread.cc
1 file changed, 25 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/18229/2
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18229 )
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
[thread] Small refactor to improve ThreadMgr performance
Move threads_started_metric_ and threads_running_metric_ out of lock and
use atomic to improve performance.
Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Reviewed-on: http://gerrit.cloudera.org:8080/18229
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Yingchun Lai <ac...@gmail.com>
---
M src/kudu/util/thread.cc
1 file changed, 25 insertions(+), 29 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Yingchun Lai: Verified
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18229 )
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG@10
PS1, Line 10: to improve performance
> Not yet, but I think this improvement is insignificant indeed.
I guess it helps with spawning threads when there is concurrent activity on the embedded WebUI for the threads, so there is some value in this anyways.
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:52:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18229 )
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
Patch Set 1: Code-Review+1
(3 comments)
Overall looks good to me, just a few nits.
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG@10
PS1, Line 10: to improve performance
BTW, what's the performance gain after this patch applied? Did you have a chance to measure the difference?
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc@256
PS1, Line 256: and thread metrics
nit: remove this since the metrics are no longer protected by 'lock_'
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc@340
PS1, Line 340: uint64_t ThreadMgr::ReadThreadsStarted() const {
: return threads_started_metric_;
: }
:
: uint64_t ThreadMgr::ReadThreadsRunning() const {
: return threads_running_metric_;
: }
nit: maybe, move these methods into the body the 'ThreadMgr' class definition?
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 14 Feb 2022 18:47:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18229 )
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18229/1//COMMIT_MSG@10
PS1, Line 10: to improve performance
> BTW, what's the performance gain after this patch applied? Did you have a
Not yet, but I think this improvement is insignificant indeed.
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc
File src/kudu/util/thread.cc:
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc@256
PS1, Line 256: ThreadCategory> Thr
> nit: remove this since the metrics are no longer protected by 'lock_'
Done
http://gerrit.cloudera.org:8080/#/c/18229/1/src/kudu/util/thread.cc@340
PS1, Line 340: }
: return Status::OK();
: }
:
: void ThreadMgr::AddThread(const pthread_t& pthread_id, const string& name,
: const string& category, int64_t tid) {
:
> nit: maybe, move these methods into the body the 'ThreadMgr' class definiti
Done
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 15 Feb 2022 02:45:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18229 )
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 16 Feb 2022 02:57:35 +0000
Gerrit-HasComments: No
[kudu-CR] [thread] Small refactor to improve ThreadMgr performance
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has removed a vote on this change.
Change subject: [thread] Small refactor to improve ThreadMgr performance
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18229
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I219c509c5818b618e864c534bbf40cc4f9a7dc13
Gerrit-Change-Number: 18229
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>