-
Notifications
You must be signed in to change notification settings - Fork 30
Re-enable Turbo Boost after running benchmarks #327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,8 +32,8 @@ def os | |
| end | ||
|
|
||
| # Checked system - error or return info if the command fails | ||
| def check_call(command, env: {}, raise_error: true) | ||
| puts(command) | ||
| def check_call(command, env: {}, raise_error: true, quiet: false) | ||
| puts("+ #{command}") unless quiet | ||
|
|
||
| result = {} | ||
|
|
||
|
|
@@ -57,13 +57,21 @@ def have_yjit?(ruby) | |
| ruby_version.downcase.include?("yjit") | ||
| end | ||
|
|
||
| # Disable Turbo Boost while running benchmarks. Maximize the CPU frequency. | ||
| def set_bench_config(turbo:) | ||
| # sudo requires the flag '-S' in order to take input from stdin | ||
| if File.exist?('/sys/devices/system/cpu/intel_pstate') # Intel | ||
| # sudo requires the flag '-S' in order to take input from stdin | ||
| check_call("sudo -S sh -c 'echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo'") unless turbo || intel_no_turbo? | ||
| unless intel_no_turbo? || turbo | ||
| check_call("sudo -S sh -c 'echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo'") | ||
| at_exit { check_call("sudo -S sh -c 'echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo'", quiet: true) } | ||
| end | ||
| # Disabling Turbo Boost reduces the CPU frequency, so this should be run after that. | ||
| check_call("sudo -S sh -c 'echo 100 > /sys/devices/system/cpu/intel_pstate/min_perf_pct'") unless intel_perf_100pct? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean you've been fine with it so far, so it should be fine. I just kept the code simple by not introducing what I don't need. I'm personally not interested in lowering power consumption, but if you're, feel free to file a PR to change it. |
||
| elsif File.exist?('/sys/devices/system/cpu/cpufreq/boost') # AMD | ||
| check_call("sudo -S sh -c 'echo 0 > /sys/devices/system/cpu/cpufreq/boost'") unless turbo || amd_no_boost? | ||
| unless amd_no_boost? || turbo | ||
| check_call("sudo -S sh -c 'echo 0 > /sys/devices/system/cpu/cpufreq/boost'") | ||
| at_exit { check_call("sudo -S sh -c 'echo 1 > /sys/devices/system/cpu/cpufreq/boost'", quiet: true) } | ||
| end | ||
| check_call("sudo -S cpupower frequency-set -g performance") unless performance_governor? | ||
| end | ||
| end | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems better to not make it quiet here to let the user understand why e.g. the sudo password is asked again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fair. Feel free to file a PR for it, but so far the 15-minute sudo timeout has been long enough for me to never encounter the situation when running benchmarks locally.