fix: report RAPL DRAM as RAM energy instead of adding it to CPU energy - #1344
davidberenstein1957 wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1344 +/- ##
=======================================
Coverage 91.70% 91.70%
=======================================
Files 49 49
Lines 5157 5159 +2
=======================================
+ Hits 4729 4731 +2
Misses 428 428 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ad8dc5d to
911929e
Compare
f5a7183 to
cb38949
Compare
Aggregate the dram domain into the reported processor energy alongside package/psys, with tests covering aggregation and the non-power-domain fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ad837f7 to
0a24bdb
Compare
Verdict: 🔧 Request changesThe bug is real. DRAM domains keep the raw name Must fix:
Worth documenting:
|
…_include_dram in fallback Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Made the changes in c2cc54b: merged master, DRAM mirror exclusion now uses a |
Draft: needs a run on a real Linux machine with readable RAPL (bare metal, ideally a two-socket Xeon) before merge. Everything below is tested against fake sysfs trees only.
Description
On Linux,
rapl_include_dram=Trueread the DRAM RAPL domains but their energy never reached any total, so the flag did nothing. This PR reports that energy as RAM instead of adding it to CPU energy:rapl_include_dram=Trueand a DRAM domain present,ram_energy/ram_powercome from the measured RAPL DRAM counter, andcpu_energyno longer includes DRAM. Memory is counted once.intel-rapl:0/intel-rapl:0:N, the usual server layout), and deduplicated by zone id, so a two-socket machine reports both sockets.tracking_mode="process"(DRAM is machine-wide), and when psys is used (psys usually includes DRAM already).Related Issue
Fixes #1305. Addresses #1268 for Linux RAPL. Windows EMI still adds DRAM to CPU energy; that stays a follow-up on #1268.
Motivation and Context
The first version of this PR added DRAM to
cpu_energy, which counted memory twice becauseram_energyis still estimated separately. The maintainer decision was to route measured DRAM into the RAM component instead, as #1268 proposes.How Has This Been Tested?
uv run task test-package: 672 passed.test_rapl_cpu_energy_excludes_dram(flag on and off),test_rapl_reads_nested_dram_zones_once_per_socket(nested layout plus flat symlinks, 1 and 2 sockets),test_rapl_psys_does_not_read_dram,test_ram_reports_measured_dram_energy,test_ram_falls_back_to_estimate_without_live_dram,test_ram_live_dram_counter_reports_a_zero_delta_as_measured,test_ram_dead_dram_counter_is_dropped_once,test_ram_negative_dram_delta_falls_back_to_estimate,test_ram_uses_rapl_dram_when_available(including process mode).Still to check on hardware:
intel-rapl) and MMIO (intel-rapl-mmio); if so, it would be counted twice in RAM.Screenshots (if appropriate):
N/A
Types of changes
Note: a measurement change for users with
rapl_include_dram=Trueon Linux:cpu_energydrops by the DRAM share andram_energybecomes the measured value. DRAM log lines are now named "DRAM Energy Delta_N".AI Usage Disclosure
Checklist:
Commits