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 2016/08/09 20:56:33 UTC
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Hello Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/3875
to review the following change.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
cfile-test: some test micro-optimization to avoid timeouts
It turns out that SCOPED_TRACE is relatively slow. Removing some SCOPED_TRACE
is hot parts of the test speed some of the test cases up by almost 2x in ASAN
builds on my box. Given that these tests are fairly deterministic given a seed,
and have been stable for a long time, leaving the SCOPED_TRACEs in has limited
value.
I only removed those that are in hot paths rather than removing all SCOPED_TRACEs.
I also optimized the "large strings" test to avoid using a large amount of
padding via the StringPrintf argument. The new implementation seems to run
almost twice as fast on my machine. Comparing the
'CacheTypes/TestCFileBothCacheTypes.TestReadWriteLargeStrings/0' in fast-test
ASAN mode before and after:
Before:
real 0m39.710s
user 0m28.808s
sys 0m3.828s
After:
real 0m21.274s
user 0m9.824s
sys 0m3.832s
Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
---
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
2 files changed, 37 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/3875/1
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/2767/
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/2765/
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
cfile-test: some test micro-optimization to avoid timeouts
It turns out that SCOPED_TRACE is relatively slow. Removing some SCOPED_TRACE
in hot parts of the test speeds some of the test cases up by almost 2x in ASAN
builds on my box. Given that these tests are fairly deterministic given a seed,
and have been stable for a long time, leaving the SCOPED_TRACEs in has limited
value.
I only removed those that are in hot paths rather than removing all SCOPED_TRACEs.
I also optimized the "large strings" test to avoid using a large amount of
padding via the StringPrintf argument. The new implementation seems to run
almost twice as fast on my machine. Comparing the
'CacheTypes/TestCFileBothCacheTypes.TestReadWriteLargeStrings/0' in fast-test
ASAN mode before and after:
Before:
real 0m39.710s
user 0m28.808s
sys 0m3.828s
After:
real 0m21.274s
user 0m9.824s
sys 0m3.832s
Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Reviewed-on: http://gerrit.cloudera.org:8080/3875
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
2 files changed, 37 insertions(+), 22 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
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/3875
to look at the new patch set (#2).
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
cfile-test: some test micro-optimization to avoid timeouts
It turns out that SCOPED_TRACE is relatively slow. Removing some SCOPED_TRACE
in hot parts of the test speeds some of the test cases up by almost 2x in ASAN
builds on my box. Given that these tests are fairly deterministic given a seed,
and have been stable for a long time, leaving the SCOPED_TRACEs in has limited
value.
I only removed those that are in hot paths rather than removing all SCOPED_TRACEs.
I also optimized the "large strings" test to avoid using a large amount of
padding via the StringPrintf argument. The new implementation seems to run
almost twice as fast on my machine. Comparing the
'CacheTypes/TestCFileBothCacheTypes.TestReadWriteLargeStrings/0' in fast-test
ASAN mode before and after:
Before:
real 0m39.710s
user 0m28.808s
sys 0m3.828s
After:
real 0m21.274s
user 0m9.824s
sys 0m3.832s
Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
---
M src/kudu/cfile/cfile-test-base.h
M src/kudu/cfile/cfile-test.cc
2 files changed, 37 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/75/3875/2
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] cfile-test: some test micro-optimization to avoid timeouts
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: cfile-test: some test micro-optimization to avoid timeouts
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3875/1//COMMIT_MSG
Commit Message:
PS1, Line 10: is hot parts of the test
Nit: "is" makes the sentence read in a strange way.
--
To view, visit http://gerrit.cloudera.org:8080/3875
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I951f4ef47c5275ea9743b1be0ff74374498192b1
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes