Jim Apple has posted comments on this change. Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings ......................................................................
Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/4528/5/be/src/util/cpu-info.cc File be/src/util/cpu-info.cc: Line 86: bool ReadLine(const string& filename, string* out) { now this is only called one place, so you can just inline it there. Line 90: trim(*out); Why trim? Line 94: // Helper function to warn if a given file does not contain an expected string. If the Use anonymous namespace, since this is not called elsewhere https://google.github.io/styleguide/cppguide.html#Namespaces PS5, Line 94: n expected string " as its first line" Line 100: if (line != expected) LOG(ERROR) << warning_text; "Expected " << expected << ", actual " << line << endl << warning_text Line 165: string governor_file = const, here and below -- To view, visit http://gerrit.cloudera.org:8080/4528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5e879cb35cf736f6112c1caed829722a38849794 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-HasComments: Yes
