Skip to content

Commit 0d4a2e5

Browse files
soerengrunewaldsoerengrunewald
committed
fileutils: Refactor read_as again for better usability
[Why] While switching over the CPU readout, it became clear that error propagation wasn't not very nice, since we relied on comparison with the default value. [How] Since we have C++17 we can use std::optional to make the error propagation nicer. Signed-off-by: Soeren Grunewald <soeren.grunewald@gmx.net>
1 parent e87109f commit 0d4a2e5

3 files changed

Lines changed: 44 additions & 42 deletions

File tree

‎src/amdgpu.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,10 @@ void AMDGPU::metrics_polling_thread() {
318318

319319
void AMDGPU::get_sysfs_metrics() {
320320
if (sysfs_nodes.busy)
321-
metrics.load = read_as<int>(sysfs_nodes.busy);
321+
metrics.load = read_as<int>(sysfs_nodes.busy).value_or(0);
322322

323323
if (sysfs_nodes.memory_clock)
324-
metrics.MemClock = read_as<int64_t>(sysfs_nodes.memory_clock) / 1000000;
324+
metrics.MemClock = read_as<int64_t>(sysfs_nodes.memory_clock).value_or(0) / 1000000;
325325

326326
// TODO: on some gpus this will use the power1_input instead
327327
// this value is instantaneous and should be averaged over time
@@ -334,7 +334,7 @@ void AMDGPU::get_sysfs_metrics() {
334334
} else
335335
#endif
336336
if (sysfs_nodes.power_usage)
337-
metrics.powerUsage = read_as<int64_t>(sysfs_nodes.power_usage) / 1000000;
337+
metrics.powerUsage = read_as<int64_t>(sysfs_nodes.power_usage).value_or(0) / 1000000;
338338

339339
#ifndef TEST_ONLY
340340
if (!get_params()->enabled[OVERLAY_PARAM_ENABLED_gpu_power_limit]) {
@@ -344,38 +344,38 @@ void AMDGPU::get_sysfs_metrics() {
344344
} else
345345
#endif
346346
if (sysfs_nodes.power_limit)
347-
metrics.powerLimit = read_as<int64_t>(sysfs_nodes.power_limit) / 1000000;
347+
metrics.powerLimit = read_as<int64_t>(sysfs_nodes.power_limit).value_or(0) / 1000000;
348348

349349
if (sysfs_nodes.fan) {
350-
metrics.fan_speed = read_as<int64_t>(sysfs_nodes.fan);
350+
metrics.fan_speed = read_as<int64_t>(sysfs_nodes.fan).value_or(0);
351351
metrics.fan_rpm = true;
352352
}
353353

354354
if (sysfs_nodes.vram_total)
355-
metrics.memoryTotal = float(read_as<int64_t>(sysfs_nodes.vram_total)) / (1024 * 1024 * 1024);
355+
metrics.memoryTotal = float(read_as<int64_t>(sysfs_nodes.vram_total).value_or(0)) / (1024 * 1024 * 1024);
356356

357357
if (sysfs_nodes.vram_used)
358-
metrics.sys_vram_used = float(read_as<int64_t>(sysfs_nodes.vram_used)) / (1024 * 1024 * 1024);
358+
metrics.sys_vram_used = float(read_as<int64_t>(sysfs_nodes.vram_used).value_or(0)) / (1024 * 1024 * 1024);
359359

360360
// On some GPUs SMU can sometimes return the wrong temperature.
361361
// As HWMON is way more visible than the SMU metrics, let's always trust it as it is the most likely to work
362362
if (sysfs_nodes.core_clock)
363-
metrics.CoreClock = read_as<int64_t>(sysfs_nodes.core_clock) / 1000000;
363+
metrics.CoreClock = read_as<int64_t>(sysfs_nodes.core_clock).value_or(0) / 1000000;
364364

365365
if (sysfs_nodes.temp)
366-
metrics.temp = read_as<int>(sysfs_nodes.temp) / 1000;
366+
metrics.temp = read_as<int>(sysfs_nodes.temp).value_or(0) / 1000;
367367

368368
if (sysfs_nodes.junction_temp)
369-
metrics.junction_temp = read_as<int>(sysfs_nodes.junction_temp) / 1000;
369+
metrics.junction_temp = read_as<int>(sysfs_nodes.junction_temp).value_or(0) / 1000;
370370

371371
if (sysfs_nodes.memory_temp)
372-
metrics.memory_temp = read_as<int>(sysfs_nodes.memory_temp) / 1000;
372+
metrics.memory_temp = read_as<int>(sysfs_nodes.memory_temp).value_or(0) / 1000;
373373

374374
if (sysfs_nodes.gtt_used)
375-
metrics.gtt_used = float(read_as<int64_t>(sysfs_nodes.gtt_used)) / (1024 * 1024 * 1024);
375+
metrics.gtt_used = float(read_as<int64_t>(sysfs_nodes.gtt_used).value_or(0)) / (1024 * 1024 * 1024);
376376

377377
if (sysfs_nodes.gpu_voltage_soc)
378-
metrics.voltage = read_as<int64_t>(sysfs_nodes.gpu_voltage_soc);
378+
metrics.voltage = read_as<int64_t>(sysfs_nodes.gpu_voltage_soc).value_or(0);
379379
}
380380

381381
AMDGPU::AMDGPU(std::string pci_dev, uint32_t device_id, uint32_t vendor_id) {

‎src/cpu.cpp‎

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ bool CPUStats::ReadcpuTempFile(int& temp) {
282282
if (!m_cpuTempFile)
283283
return false;
284284

285-
temp = read_as<int>(m_cpuTempFile, 0) / 1000;
285+
temp = read_as<int>(m_cpuTempFile).value_or(0) / 1000;
286286
return temp == 0;
287287
}
288288

@@ -308,31 +308,32 @@ static bool get_cpu_power_k10temp(CPUPowerData* cpuPowerData, float& power) {
308308

309309
if(powerData_k10temp->corePowerFile || powerData_k10temp->socPowerFile)
310310
{
311-
auto const corePower = read_as<int>(powerData_k10temp->corePowerFile, -1);
312-
auto const socPower = read_as<int>(powerData_k10temp->socPowerFile, -1);
313-
if (corePower != -1 and socPower != -1) {
314-
power = (corePower + socPower) / 1000000;
311+
auto const corePower = read_as<int>(powerData_k10temp->corePowerFile);
312+
auto const socPower = read_as<int>(powerData_k10temp->socPowerFile);
313+
if (corePower and socPower) {
314+
power = (corePower.value() + socPower.value()) / 1000000;
315315
return true;
316316
}
317317
}
318318

319319
if (!powerData_k10temp->coreVoltageFile || !powerData_k10temp->coreCurrentFile || !powerData_k10temp->socVoltageFile || !powerData_k10temp->socCurrentFile)
320320
return false;
321321

322-
auto const coreVoltage = read_as<int>(powerData_k10temp->coreVoltageFile, -1);
323-
if (coreVoltage == -1)
322+
auto const coreVoltage = read_as<int>(powerData_k10temp->coreVoltageFile);
323+
if (not coreVoltage)
324324
return false;
325-
auto const coreCurrent = read_as<int>(powerData_k10temp->coreCurrentFile, -1);
326-
if (coreCurrent == -1)
325+
auto const coreCurrent = read_as<int>(powerData_k10temp->coreCurrentFile);
326+
if (not coreCurrent)
327327
return false;
328-
auto const socVoltage = read_as<int>(powerData_k10temp->socVoltageFile, -1);
329-
if (socVoltage == -1)
328+
auto const socVoltage = read_as<int>(powerData_k10temp->socVoltageFile);
329+
if (not socVoltage)
330330
return false;
331-
auto const socCurrent = read_as<int>(powerData_k10temp->socCurrentFile, -1);
332-
if (socCurrent== -1)
331+
auto const socCurrent = read_as<int>(powerData_k10temp->socCurrentFile);
332+
if (not socCurrent)
333333
return false;
334334

335-
power = (coreVoltage * coreCurrent + socVoltage * socCurrent) / 1000000;
335+
power = (coreVoltage.value() * coreCurrent.value() +
336+
socVoltage.value() * socCurrent.value()) / 1000000;
336337

337338
return true;
338339
}
@@ -343,14 +344,14 @@ static bool get_cpu_power_zenpower(CPUPowerData* cpuPowerData, float& power) {
343344
if (!powerData_zenpower->corePowerFile || !powerData_zenpower->socPowerFile)
344345
return false;
345346

346-
auto const corePower = read_as<int>(powerData_zenpower->corePowerFile, -1);
347-
if (corePower == -1)
347+
auto const corePower = read_as<int>(powerData_zenpower->corePowerFile);
348+
if (not corePower)
348349
return false;
349-
auto const socPower = read_as<int>(powerData_zenpower->socPowerFile, -1);
350-
if (socPower == -1)
350+
auto const socPower = read_as<int>(powerData_zenpower->socPowerFile);
351+
if (not socPower)
351352
return false;
352353

353-
power = (corePower + socPower) / 1000000;
354+
power = (corePower.value() + socPower.value()) / 1000000;
354355

355356
return true;
356357
}
@@ -361,19 +362,19 @@ static bool get_cpu_power_zenergy(CPUPowerData* cpuPowerData, float& power) {
361362
return false;
362363

363364
auto const energyCounterValue = read_as<uint64_t>(powerData_zenergy->energyCounterFile);
364-
if (energyCounterValue == 0)
365+
if (not energyCounterValue)
365366
return false;
366367

367368
Clock::time_point now = Clock::now();
368369
Clock::duration timeDiff = now - powerData_zenergy->lastCounterValueTime;
369370
int64_t timeDiffMicro = std::chrono::duration_cast<std::chrono::microseconds>(timeDiff).count();
370-
uint64_t energyCounterDiff = energyCounterValue - powerData_zenergy->lastCounterValue;
371+
uint64_t energyCounterDiff = energyCounterValue.value() - powerData_zenergy->lastCounterValue;
371372

372373

373374
if (powerData_zenergy->lastCounterValue > 0 && energyCounterValue > powerData_zenergy->lastCounterValue)
374375
power = (float) energyCounterDiff / (float) timeDiffMicro;
375376

376-
powerData_zenergy->lastCounterValue = energyCounterValue;
377+
powerData_zenergy->lastCounterValue = energyCounterValue.value();
377378
powerData_zenergy->lastCounterValueTime = now;
378379

379380
return true;
@@ -386,18 +387,18 @@ static bool get_cpu_power_rapl(CPUPowerData* cpuPowerData, float& power) {
386387
return false;
387388

388389
auto const energyCounterValue = read_as<uint64_t>(powerData_rapl->energyCounterFile);
389-
if (energyCounterValue == 0)
390+
if (not energyCounterValue)
390391
return false;
391392

392393
Clock::time_point now = Clock::now();
393394
Clock::duration timeDiff = now - powerData_rapl->lastCounterValueTime;
394395
int64_t timeDiffMicro = std::chrono::duration_cast<std::chrono::microseconds>(timeDiff).count();
395-
uint64_t energyCounterDiff = energyCounterValue - powerData_rapl->lastCounterValue;
396+
uint64_t energyCounterDiff = energyCounterValue.value() - powerData_rapl->lastCounterValue;
396397

397398
if (powerData_rapl->lastCounterValue > 0 && energyCounterValue > powerData_rapl->lastCounterValue)
398399
power = energyCounterDiff / timeDiffMicro;
399400

400-
powerData_rapl->lastCounterValue = energyCounterValue;
401+
powerData_rapl->lastCounterValue = energyCounterValue.value();
401402
powerData_rapl->lastCounterValueTime = now;
402403

403404
return true;
@@ -420,10 +421,10 @@ static bool get_cpu_power_xgene(CPUPowerData* cpuPowerData, float& power) {
420421
return false;
421422

422423
auto const powerValue = read_as<uint64_t>(powerData_xgene->powerFile);
423-
if (powerValue == 0)
424+
if (not powerValue)
424425
return false;
425426

426-
power = (float) powerValue / 1000000.0f;
427+
power = (float) powerValue.value() / 1000000.0f;
427428

428429
return true;
429430
}

‎src/file_utils.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include <cinttypes>
66
#include <filesystem.h>
7+
#include <optional>
78
#include <string>
89
#include <vector>
910

@@ -44,12 +45,12 @@ namespace detail {
4445

4546
/** Read a single value from sysfs file */
4647
template<typename ValueType>
47-
ValueType read_as(FILE* f, ValueType d = 0) {
48+
std::optional<ValueType> read_as(FILE* f) {
4849
rewind(f);
4950
fflush(f);
5051
ValueType v;
5152
if (fscanf(f, detail::type_to_fmt<ValueType>(), &v) != 1)
52-
return d;
53+
return {};
5354
return v;
5455
}
5556

0 commit comments

Comments
 (0)