You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2017/03/19 23:39:29 UTC
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Hello David Ribeiro Alves, Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/6431
to review the following change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
key_encoder: avoid unordered_map for looking up encoders
I saw EncoderResolver::GetKeyEncoder() taking up ~3% of CPU in some
workloads, mostly caused by a 'div' instruction in unordered_map. Since
we know that the type enums are low integers, we can just use a vector
for lookup as well.
This also switches from shared_ptr to unique_ptr for holding the
resolvers: the shared_ptr is a holdover from pre-C++11 where putting
single-ownership smart pointers into a container was impossible.
Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
---
M src/kudu/common/key_encoder.cc
1 file changed, 15 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/6431/1
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello David Ribeiro Alves, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6431
to look at the new patch set (#3).
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
key_encoder: avoid unordered_map for looking up encoders
I saw EncoderResolver::GetKeyEncoder() taking up ~3% of CPU in some
workloads, mostly caused by a 'div' instruction in unordered_map. Since
we know that the type enums are low integers, we can just use a vector
for lookup as well.
This also switches from shared_ptr to unique_ptr for holding the
resolvers: the shared_ptr is a holdover from pre-C++11 where putting
single-ownership smart pointers into a container was impossible.
Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/common/key_encoder.cc
2 files changed, 17 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/6431/3
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/6431
to look at the new patch set (#2).
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
key_encoder: avoid unordered_map for looking up encoders
I saw EncoderResolver::GetKeyEncoder() taking up ~3% of CPU in some
workloads, mostly caused by a 'div' instruction in unordered_map. Since
we know that the type enums are low integers, we can just use a vector
for lookup as well.
This also switches from shared_ptr to unique_ptr for holding the
resolvers: the shared_ptr is a holdover from pre-C++11 where putting
single-ownership smart pointers into a container was impossible.
Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
---
M src/kudu/cfile/cfile-test.cc
M src/kudu/common/key_encoder.cc
2 files changed, 17 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/31/6431/2
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6431/2/src/kudu/common/key_encoder.cc
File src/kudu/common/key_encoder.cc:
PS2, Line 65: Type
> Can we get the type name here? Granted, we're not expecting to fail here, i
Done
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6431/2/src/kudu/common/key_encoder.cc
File src/kudu/common/key_encoder.cc:
PS2, Line 65: Type
Can we get the type name here? Granted, we're not expecting to fail here, it could be useful if it does happen.
May be more trouble than it's worth, so ultimately your call.
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] key encoder: avoid unordered map for looking up encoders
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: key_encoder: avoid unordered_map for looking up encoders
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/6431
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a9bf3a28e451562c3347fe9650db3874d62a368
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: No