Todd Lipcon has submitted this change and it was merged. Change subject: ref_counted: fix move constructors to actually move ......................................................................
ref_counted: fix move constructors to actually move Ever since the move constructors for scoped_refptr were first implemented, they didn't properly move. That is to say, they caused an unnecessary AddRef/Release pair. The issue is that they were passing through an rvalue-reference parameter without properly using std::move(). See [1] for an explanation of this subtlety. When combined with some other in-flight patches, this resulted in a substantial improvement in LBM startup time, since a hashmap containing ref-counted elements could relocate them without unnecessary reference count increments and decrements. It's likely it will also improve performance elsewhere throughout Kudu. This patch also pulls in an extra move constructor from the Chromium code base which they claim helps Clang generate better warnings. I didn't verify that, but figured I'd copy it while I was looking at the code. [1] https://stackoverflow.com/questions/14486367/why-do-you-use-stdmove-when-you-have-in-c11 Change-Id: I6b6b6753ab16221e065900eba5a7c178975d38a6 Reviewed-on: http://gerrit.cloudera.org:8080/8002 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo <[email protected]> Reviewed-by: Dan Burkert <[email protected]> --- M src/kudu/gutil/ref_counted.h 1 file changed, 12 insertions(+), 2 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8002 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I6b6b6753ab16221e065900eba5a7c178975d38a6 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]>
