Todd Lipcon has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/9093 )
Change subject: consensus: avoid extra calls to protobuf SpaceUsed in the log
cache
......................................................................
consensus: avoid extra calls to protobuf SpaceUsed in the log cache
The SpaceUsed function appears to be somewhat expensive since it walks
the entire protobuf structure. In complex protobufs (like those with
larger schemas) this can cause a lot of cache misses, etc. In fact,
SpaceUsed shows up as one of the top 5 CPU consumers in a YCSB workload
I'm looking at.
This patch adds the cached value of SpaceUsed to the log cache so that
it only needs to be computed upon insertion and not during removal.
Additionally it moves some more work outside the lock to reduce
contention.
Performance counter stats for 'build/latest/bin/full_stack-insert-scan-test
--gtest_filter=*MRSOnlyStressTest* --inserts_per_client=200000
--concurrent_inserts=10 --rows_per_batch=1 --skip_scans':
321562.582290 task-clock (msec) # 4.896 CPUs utilized
11,833,623 context-switches # 0.037 M/sec
3,675,101 cpu-migrations # 0.011 M/sec
118,072 page-faults # 0.367 K/sec
1,035,621,247,373 cycles # 3.221 GHz
659,776,225,172 instructions # 0.64 insn per cycle
124,415,953,758 branches # 386.911 M/sec
1,520,148,589 branch-misses # 1.22% of all branches
65.679925745 seconds time elapsed
Performance counter stats for 'build/latest/bin/full_stack-insert-scan-test
--gtest_filter=*MRSOnlyStressTest* --inserts_per_client=200000
--concurrent_inserts=10 --rows_per_batch=1 --skip_scans':
305878.625093 task-clock (msec) # 5.108 CPUs utilized
12,860,037 context-switches # 0.042 M/sec
3,877,232 cpu-migrations # 0.013 M/sec
114,011 page-faults # 0.373 K/sec
981,876,239,200 cycles # 3.210 GHz
599,697,732,411 instructions # 0.61 insn per cycle
112,309,222,234 branches # 367.169 M/sec
1,427,381,915 branch-misses # 1.27% of all branches
59.886073541 seconds time elapsed
Change-Id: I0439995fb1369a3333b7d5858518d01277b53076
Reviewed-on: http://gerrit.cloudera.org:8080/9093
Reviewed-by: Dan Burkert <[email protected]>
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves <[email protected]>
---
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/log_cache.h
M src/kudu/gutil/map-util.h
M src/kudu/util/map-util-test.cc
4 files changed, 73 insertions(+), 32 deletions(-)
Approvals:
Dan Burkert: Looks good to me, but someone else must approve
Kudu Jenkins: Verified
David Ribeiro Alves: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9093
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0439995fb1369a3333b7d5858518d01277b53076
Gerrit-Change-Number: 9093
Gerrit-PatchSet: 5
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>