[Impala-ASF-CR] IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings
Jim Apple has posted comments on this change. Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings .. Patch Set 7: Code-Review+2 -- 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: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings
Lars Volker has posted comments on this change. Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/4528/5/be/src/util/cpu-info.cc File be/src/util/cpu-info.cc: Line 178: } > Either way is OK with me, though if you mention it one place, you might wan Added comments, as I suspect this will bite people otherwise. -- 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: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings
Lars Volker has posted comments on this change. Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings .. Patch Set 5: (3 comments) Thanks for the review, please see PS5. http://gerrit.cloudera.org:8080/#/c/4528/3/be/src/util/cpu-info.cc File be/src/util/cpu-info.cc: PS3, Line 86: bool > Nobody is checking this. Indeed, fixed it. PS3, Line 90: ( > The ifstream destructor does this automatically, doesn't it? It does, removed it here and elsewhere in the file. PS3, Line 172: : void CpuInfo::VerifyTurboDisabled() { : WarnIfFileNotEqual("/sys/devices/system/cpu/intel_pstate/no_turbo", "1", : "WARNING: CPU turbo is enabled. This setting can change the clock " : "frequency of CPU cores during the benchmark run, which can lead to " : "inaccurate results. You can disable CPU turbo by writing a 1 to" : "/sys/devices/system/cpu/intel_pstate/no_turbo"); : } > Even this whole thing can be a helper function now, Done -- 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 VolkerGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings
Lars Volker has posted comments on this change. Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings .. Patch Set 3: (6 comments) Thanks for the review. Please see PS3. http://gerrit.cloudera.org:8080/#/c/4528/1//COMMIT_MSG Commit Message: PS1, Line 9: Change-Id: I5e879cb35cf736f6112c1caed829722a38849794 > Doesn't this solve that issue? You're right, it does. http://gerrit.cloudera.org:8080/#/c/4528/1/be/src/util/cpu-info.cc File be/src/util/cpu-info.cc: PS1, Line 149: > This is the default, right? It is, removed it here and elsewhere in the file. PS1, Line 151: LOG(ERROR) << "CPU does not support the Supplemental SSE3 (SSSE3) instruction set. " :<< "This setup is generally unsupported and Impala might be unstable."; : } : } : : void CpuInfo::V > This is in the other method, too; can you factor it out? Done Line 158: string governor_file = > Can you use a C==-style == here? Done Line 172: string turbo_file = "/sys/devices/system/cpu/intel_pstate/no_turbo"; > here also, you could compare to "1", right? Yes, done. Line 173: string turbo_off; > Explain why this matters - it can ramp-up and down performance dynamically Done -- 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: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Lars Volker Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings
Lars Volker has uploaded a new patch set (#3). Change subject: IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings .. IMPALA-4193: Warn when benchmarks run with sub-optimal CPU settings Change-Id: I5e879cb35cf736f6112c1caed829722a38849794 --- M be/src/util/benchmark.cc M be/src/util/cpu-info.cc M be/src/util/cpu-info.h 3 files changed, 47 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/4528/3 -- To view, visit http://gerrit.cloudera.org:8080/4528 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5e879cb35cf736f6112c1caed829722a38849794 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars VolkerGerrit-Reviewer: Jim Apple