[kudu-CR] WIP: cfile set: avoid Status allocation for row not found

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: WIP: cfile_set: avoid Status allocation for row not found
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6804/1/src/kudu/tablet/cfile_set.cc
File src/kudu/tablet/cfile_set.cc:

Line 221: } else if (!s.ok()) {
> warning: don't use else after return [readability-else-after-return]
Done


http://gerrit.cloudera.org:8080/#/c/6804/1/src/kudu/tablet/cfile_set.h
File src/kudu/tablet/cfile_set.h:

Line 111:   Status NewKeyIterator(CFileIterator **iter) const;
> warning: function 'kudu::tablet::CFileSet::NewKeyIterator' has a definition
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/6804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I3056aaaf0eec6b7deebcd1f206cb551b75996927
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] WIP: cfile set: avoid Status allocation for row not found

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves,

I'd like you to do a code review.  Please visit

http://gerrit.cloudera.org:8080/6804

to review the following change.

Change subject: WIP: cfile_set: avoid Status allocation for row not found
..

WIP: cfile_set: avoid Status allocation for row not found

We expect Bloom Filter lookups to typically return "row not found". The
code currently indicates this using a Status::NotFound(). However,
constructing such a status involves an alloc/free pair, which is
relatively expensive to do millions of times per second for this common
case.

In a profile of tcph_real_world, the tserver spends a good chunk of its
CPU in these code paths, especially now that our alloc/free contain
heavier instrumentation for memory accounting.

This changes to using a boost::optional out-parameter for the
common case instead.

WIP: need to add benchmark results

Change-Id: I3056aaaf0eec6b7deebcd1f206cb551b75996927
---
M src/kudu/tablet/cfile_set.cc
M src/kudu/tablet/cfile_set.h
M src/kudu/tablet/diskrowset.cc
3 files changed, 30 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/04/6804/1
-- 
To view, visit http://gerrit.cloudera.org:8080/6804
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3056aaaf0eec6b7deebcd1f206cb551b75996927
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves