[kudu-CR] cache: add a benchmark

[Todd Lipcon (Code Review)]

Todd Lipcon has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..

cache: add a benchmark

In some workloads, the LRU cache is now a substantial bottleneck. This
patch adds a simple benchmark we can use to measure improvements in
cache performance under concurrency.

Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Reviewed-on: http://gerrit.cloudera.org:8080/6696
Reviewed-by: Adar Dembo 
Tested-by: Kudu Jenkins
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/cache-bench.cc
2 files changed, 192 insertions(+), 0 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] cache: add a benchmark

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 21:56:25 +
Gerrit-HasComments: No

[kudu-CR] cache: add a benchmark

[Todd Lipcon (Code Review)]

Hello David Ribeiro Alves, Jean-Daniel Cryans, Kudu Jenkins,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: cache: add a benchmark
..

cache: add a benchmark

In some workloads, the LRU cache is now a substantial bottleneck. This
patch adds a simple benchmark we can use to measure improvements in
cache performance under concurrency.

Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
---
M src/kudu/util/CMakeLists.txt
A src/kudu/util/cache-bench.cc
2 files changed, 192 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/6696/2
--
To view, visit http://gerrit.cloudera.org:8080/6696
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] cache: add a benchmark

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/6696 )

Change subject: cache: add a benchmark
..


Patch Set 1:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@95
PS1, Line 95:
> nit: extra line
Done


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@97
PS1, Line 97: TEST_P(CacheBench, RunBench) {
> would it be interesting to also measure cache churn?
do you mean the eviction rate? I think since we are doing one insert for every 
miss, and the elements are all the same size, the miss rate and eviction rate 
are the same, right?


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@98
PS1, Line 98: 16;
> make this a flag?
Done


http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc@99
PS1, Line 99:   const int kSecondsToRun = AllowSlowTests() ? 10 : 1;
> Can you then document more what this test is about?
I added a warmup phase so it should converge faster. I still made it 
configurable since most of our benchmarks are configurable and it can help to 
get better samples for 'perf record', etc



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-Change-Number: 6696
Gerrit-PatchSet: 1
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 25 Apr 2018 20:28:13 +
Gerrit-HasComments: Yes

[kudu-CR] cache: add a benchmark

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: cache: add a benchmark
..


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

Line 95: 
nit: extra line


PS1, Line 97: TEST_P(CacheBench, RunBench) {
would it be interesting to also measure cache churn?


PS1, Line 98: 16;
make this a flag?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cache: add a benchmark

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: cache: add a benchmark
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

Line 99:   const int kSecondsToRun = AllowSlowTests() ? 10 : 1;
> yea, though running longer has more chance of triggering races (making this
Can you then document more what this test is about?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cache: add a benchmark

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: cache: add a benchmark
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

Line 99:   const int kSecondsToRun = AllowSlowTests() ? 10 : 1;
> Is running for a configurable amount of time really making a difference? Yo
yea, though running longer has more chance of triggering races (making this a 
dual-purpose stress test). about to start screwing with the cache to try to 
make it faster and having the stress coverage is nice


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] cache: add a benchmark

[Jean-Daniel Cryans (Code Review)]

Jean-Daniel Cryans has posted comments on this change.

Change subject: cache: add a benchmark
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6696/1/src/kudu/util/cache-bench.cc
File src/kudu/util/cache-bench.cc:

Line 99:   const int kSecondsToRun = AllowSlowTests() ? 10 : 1;
Is running for a configurable amount of time really making a difference? You'd 
think that once the cache is full that the metrics would converge pretty 
quickly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I156d170c20913a17e4f94b40879409f4798023ab
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Jean-Daniel Cryans 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes