[kudu-CR] Fix move constructors to be noexcept
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9382 ) Change subject: Fix move constructors to be noexcept .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/9382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Gerrit-Change-Number: 9382 Gerrit-PatchSet: 2 Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 23 Feb 2018 05:36:25 + Gerrit-HasComments: No
[kudu-CR] Fix move constructors to be noexcept
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9382 ) Change subject: Fix move constructors to be noexcept .. Fix move constructors to be noexcept I noticed while looking at a profile of a tablet server starting up that I saw a lot of CPU used in stacks like: kudu::subtle::RefCountedThreadSafeBase::AddRef() const void std::vector:: _M_emplace_back_aux Digging into the source of std::vector, I saw that the emplace_back_aux function is used when emplacing requires resizing the underlying array. Since scoped_refptr has a move constructor I was surprised to see it using AddRef() here during the array expansion. In fact it turns out that std::vector<> will only use the move constructor when it is marked 'noexcept', which ours was not. I verified this with a simple test program as well[1]. If the move constructor is marked 'noexcept', it is called. Otherwise, it is not. This patch corrects scoped_refptr as well as a few other spots where we had move constructors without noexcept. [1] https://gist.github.com/toddlipcon/c9a807c6a67037ccd1b63caa93f74c67 Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Reviewed-on: http://gerrit.cloudera.org:8080/9382 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/cfile/block_cache.h M src/kudu/cfile/block_handle.h M src/kudu/gutil/ref_counted.h M src/kudu/tablet/lock_manager.cc M src/kudu/tablet/lock_manager.h M src/kudu/util/cow_object.h M src/kudu/util/status.h 7 files changed, 17 insertions(+), 17 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/9382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Gerrit-Change-Number: 9382 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix move constructors to be noexcept
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9382 to look at the new patch set (#2). Change subject: Fix move constructors to be noexcept .. Fix move constructors to be noexcept I noticed while looking at a profile of a tablet server starting up that I saw a lot of CPU used in stacks like: kudu::subtle::RefCountedThreadSafeBase::AddRef() const void std::vector:: _M_emplace_back_aux Digging into the source of std::vector, I saw that the emplace_back_aux function is used when emplacing requires resizing the underlying array. Since scoped_refptr has a move constructor I was surprised to see it using AddRef() here during the array expansion. In fact it turns out that std::vector<> will only use the move constructor when it is marked 'noexcept', which ours was not. I verified this with a simple test program as well[1]. If the move constructor is marked 'noexcept', it is called. Otherwise, it is not. This patch corrects scoped_refptr as well as a few other spots where we had move constructors without noexcept. [1] https://gist.github.com/toddlipcon/c9a807c6a67037ccd1b63caa93f74c67 Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 --- M src/kudu/cfile/block_cache.h M src/kudu/cfile/block_handle.h M src/kudu/gutil/ref_counted.h M src/kudu/tablet/lock_manager.cc M src/kudu/tablet/lock_manager.h M src/kudu/util/cow_object.h M src/kudu/util/status.h 7 files changed, 17 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/9382/2 -- To view, visit http://gerrit.cloudera.org:8080/9382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Gerrit-Change-Number: 9382 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Fix move constructors to be noexcept
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9382 ) Change subject: Fix move constructors to be noexcept .. Patch Set 1: nope, I just didn't grep well enough to find those. Will fix -- To view, visit http://gerrit.cloudera.org:8080/9382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Gerrit-Change-Number: 9382 Gerrit-PatchSet: 1 Gerrit-Owner: Todd LipconGerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 22 Feb 2018 08:15:06 + Gerrit-HasComments: No
[kudu-CR] Fix move constructors to be noexcept
Hello Alexey Serbin, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9382 to review the following change. Change subject: Fix move constructors to be noexcept .. Fix move constructors to be noexcept I noticed while looking at a profile of a tablet server starting up that I saw a lot of CPU used in stacks like: kudu::subtle::RefCountedThreadSafeBase::AddRef() const void std::vector:: _M_emplace_back_aux Digging into the source of std::vector, I saw that the emplace_back_aux function is used when emplacing requires resizing the underlying array. Since scoped_refptr has a move constructor I was surprised to see it using AddRef() here during the array expansion. In fact it turns out that std::vector<> will only use the move constructor when it is marked 'noexcept', which ours was not. I verified this with a simple test program as well[1]. If the move constructor is marked 'noexcept', it is called. Otherwise, it is not. This patch corrects scoped_refptr as well as a few other spots where we had move constructors without noexcept. [1] https://gist.github.com/toddlipcon/c9a807c6a67037ccd1b63caa93f74c67 Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 --- M src/kudu/cfile/block_handle.h M src/kudu/gutil/ref_counted.h M src/kudu/tablet/lock_manager.cc M src/kudu/tablet/lock_manager.h M src/kudu/util/status.h 5 files changed, 12 insertions(+), 12 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/9382/1 -- To view, visit http://gerrit.cloudera.org:8080/9382 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idd7487766be3261e3506ff9613a5d733013018b0 Gerrit-Change-Number: 9382 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Alexey Serbin