fix: stop fabricating per-core GPU utilisation - #13
Merged
Conversation
The per-core GPU bars and the per-core GPU timeline view were not measuring anything. GpuMonitor::get_gpu_info() took the single system-wide residency figure and multiplied it by a fixed ramp (1.0 + i * 0.02) to manufacture one value per core. Every bar was the same number wearing a different hat. macOS publishes no per-core GPU breakdown; the gpu_power sampler reports aggregate hardware active residency and nothing else. Removing the fabrication cascades: - GpuInfo and GpuCoreInfo are gone. GpuMonitor now exposes utilization() -> f32, a direct read of the atomic the background thread writes. The history buffer and refresh() were ceremony around that one load. - detect_gpu_cores() is deleted. Its only consumer was sizing the fake per-core buffers, and its hardcoded table was itself wrong for binned SKUs (it mapped every M4 Max to 40 GPU cores; the 32-core variant exists). - DataUpdate::Gpu carries one value instead of a Vec. - The 'gpu G0:12% G1:12% ...' header line is removed. It repeated one number once per core, and the KPI header already shows that number once. - TimelineView::Gpu now plots the system-wide trace on its own scale rather than N identical fabricated traces. Adds a test for extract_percentage(), the one piece of real parsing logic left in gpu.rs, covering the frequency-residency suffix and a leading core index. Also drops test_gpu_detection_expectations and test_cores_to_show_calculation. Both asserted arithmetic on locally-declared literals and called no production code, and both described the per-core GPU layout this commit removes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #12, which removed the fabricated per-process GPU column. This removes the same disease one layer down: the per-core GPU display.
The problem
GpuMonitor::get_gpu_info()took the single system-wide residency figure and manufactured one value per core from it:Twenty bars in the header, twenty traces in the
Tab→ GPU timeline, all derived from one number times a fixed ramp. The comment admitted it ("Apple Silicon doesn't expose per-core GPU stats, so we estimate") while the UI presented the output as measurement.The change
macOS publishes no per-core GPU breakdown — the
gpu_powersampler reports aggregate hardware active residency and nothing else. Deleting the fabrication cascades through the plumbing:GpuInfoandGpuCoreInfoare gone.GpuMonitornow exposesutilization() -> f32, a direct read of the atomic the background thread already writes. The history buffer,refresh(), andget_info()were ceremony around that one load.detect_gpu_cores()is deleted. Its only consumer was sizing the fake per-core buffers, and the hardcoded table was itself inaccurate for binned SKUs — it mapped every M4 Max to 40 GPU cores, but the 32-core variant ships too.DataUpdate::Gpucarries one value instead of aVec<f32>.gpu G0:12% G1:12% …header line is removed, freeing a layout row. It printed one number once per core, and the KPI header already shows that number once.TimelineView::Gpuplots the system-wide trace on its own scale instead of N identical fabricated traces. TheTabcycle keeps all four modes.Net: 81 insertions, 333 deletions.
src/gpu.rsgoes from 411 to 277 lines.Tests
Adds a test for
extract_percentage()— the one piece of real parsing logic left ingpu.rs— covering the frequency-residency suffix (12.23% (444 MHz: 12% …)) and a leading core index that must not be mistaken for the value.Removes
test_gpu_detection_expectationsandtest_cores_to_show_calculation. Both asserted arithmetic on literals declared in the test body, called no production code, and described the per-core GPU layout this PR deletes.Worth flagging separately: four of the remaining tests have the same problem.
test_dot_pattern_generation,test_dot_string_format, andtest_core_name_formattingexercisegenerate_dot_patternandformat_core_line, both of which are defined insidemod tests;test_floating_panel_dimensionsrecomputes(width / 3).max(35)inline and asserts the result. None of them touch production code. Left alone here since deciding between deleting them and rewriting them against real functions is a separate call.Verification
All five CI steps pass locally on macOS (
rustc 1.96.1):cargo build,cargo test(7 passed),cargo fmt -- --check,cargo clippy --all-targets -- -D warnings, andcargo docwithRUSTDOCFLAGS=-D warnings.🤖 Generated with Claude Code