Add CPU and kernel checks to performance tests #213
Conversation
Dynamic CPU frequency scaling and Linux kernel differences cause performance numbers to be quite different so include this in the report. This also provides warnings when dynamic CPU frequency governors are in use or when the CPU frequency is different between cores.
There was a problem hiding this comment.
Pull request overview
This PR enhances the GPIO diagnostics module by adding CPU and Linux kernel information to performance test reports. The changes help identify when dynamic CPU frequency scaling or CPU speed differences could affect benchmark results.
Changes:
- Adds new
Circuits.GPIO.Diagnostics.CPUmodule for managing CPU frequency scaling and checking CPU configuration - Updates performance test output to include kernel version, CPU count, CPU speed, and warnings about CPU configuration
- Changes performance metrics display from calls-per-second to microseconds-per-call for better readability
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lib/gpio/diagnostics/cpu.ex | New module providing CPU frequency management and benchmark suitability checks |
| lib/gpio/diagnostics.ex | Integrates CPU information into diagnostic reports and converts performance metrics to microseconds |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| passed | ||
| end | ||
|
|
||
| # Truncate sub-nanosecond for readability |
There was a problem hiding this comment.
The comment "Truncate sub-nanosecond for readability" is misleading. The function converts calls per second to microseconds per call and formats with 3 decimal places, which means it shows values to the nearest 0.001 microseconds (1 nanosecond). A more accurate comment would be "Convert calls per second to microseconds per call with 3 decimal places for readability".
| # Truncate sub-nanosecond for readability | |
| # Convert calls per second to microseconds per call with 3 decimal places for readability |
| def check_benchmark_suitability() do | ||
| cpus = cpu_list() | ||
|
|
||
| scheduler_warnings? = Enum.all?(cpus, &check_cpu_scheduler/1) |
There was a problem hiding this comment.
The logic here is incorrect. Enum.all? will only return true if ALL CPUs have warnings. Since check_cpu_scheduler/1 returns true when a warning is detected, this should use Enum.any? instead to detect if ANY CPU has a warning condition.
| scheduler_warnings? = Enum.all?(cpus, &check_cpu_scheduler/1) | |
| scheduler_warnings? = Enum.any?(cpus, &check_cpu_scheduler/1) |
| terrible, though. | ||
| If you get warnings about the CPU speed, run | ||
| `Circuits.GPIO.Diagnostics.CPU.force_slowest/0` or | ||
| `Circuits.GPIO.Diagnostics.CPU.set_speed/1` to make sure that the CPU doesn't |
There was a problem hiding this comment.
The documentation references Circuits.GPIO.Diagnostics.CPU.set_speed/1 but the actual function is named set_frequency/1. Update the reference to match the actual function name.
| `Circuits.GPIO.Diagnostics.CPU.set_speed/1` to make sure that the CPU doesn't | |
| `Circuits.GPIO.Diagnostics.CPU.set_frequency/1` to make sure that the CPU doesn't |
|
|
||
| defmodule Circuits.GPIO.Diagnostics.CPU do | ||
| @moduledoc """ | ||
| CPU |
There was a problem hiding this comment.
The module documentation is incomplete. It should describe the purpose and functionality of this module, such as "Utilities for managing CPU frequency scaling and checking CPU configuration for benchmarking purposes."
| CPU | |
| Utilities for inspecting and controlling CPU frequency scaling for diagnostics | |
| and benchmarking purposes. | |
| This module provides helpers to: | |
| * List available CPUs using the Linux `/sys/bus/cpu/devices` interface. | |
| * Force all CPUs to their slowest or fastest frequency using CPU scaling | |
| governors (e.g., `"powersave"` and `"performance"`). | |
| * Set all CPUs to a specific target frequency (in MHz) when the `userspace` | |
| scaling governor is available. | |
| These functions are intended for use on Linux systems where CPU frequency | |
| scaling is exposed via sysfs. They are useful for making benchmark runs and | |
| GPIO diagnostics more repeatable by putting the CPU in a known performance | |
| state. |
| [first | _rest] -> | ||
| mean = Enum.sum(speeds) / length(speeds) | ||
|
|
||
| if abs(mean - first) < 0.001 do | ||
| {false, mean} | ||
| else | ||
| io_warn("CPU speeds don't all match: #{inspect(speeds)}") | ||
| {true, mean} | ||
| end |
There was a problem hiding this comment.
The logic for detecting mismatched CPU speeds is flawed. It only compares the mean to the first CPU speed, which can miss cases where speeds vary but the mean happens to match the first element. For example, if CPU speeds are [1500, 1000, 2000], the mean is 1500 which matches the first element, so no warning would be issued even though speeds don't match. Consider checking if all speeds are equal by verifying that the maximum and minimum speeds differ by less than the tolerance, or by checking if all speeds equal the first speed.
Dynamic CPU frequency scaling and Linux kernel differences cause
performance numbers to be quite different so include this in the report.
This also provides warnings when dynamic CPU frequency governors are in
use or when the CPU frequency is different between cores.
Here's what the report looks like now: