Hello Dan Burkert, Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/8002
to review the following change.
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
---
M src/kudu/gutil/ref_counted.h
1 file changed, 11 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/02/8002/1
--
To view, visit http://gerrit.cloudera.org:8080/8002
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b6b6753ab16221e065900eba5a7c178975d38a6
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
