Skip to content

Commit

Permalink
DPL: make DeviceMetricsHelper more robust
Browse files Browse the repository at this point in the history 
- Use constraints to make sure we do not pass unexpected value types.
- Cleanup usage of exceptions since the constraints now enforce the correct
  types.
- Use string_view rather than std::string for read only variables
  • Loading branch information
@ktf
ktf committed Jan 9, 2025
1 parent 3e37f60 commit a12df3e
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 57 deletions.
141 changes: 87 additions & 54 deletions Framework/Core/include/Framework/DeviceMetricsHelper.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,31 @@
#define O2_FRAMEWORK_DEVICEMETRICSHELPERS_H_

#include "Framework/DeviceMetricsInfo.h"
#include "Framework/RuntimeError.h"
#include <array>
#include <concepts>
#include <cstddef>
#include <cstdint>
#include <cstring>
#include <functional>
#include <string>
#include <string_view>
#include <vector>

namespace o2::framework
{
struct DriverInfo;

// General definition of what can of values can be put in a metric.
// Notice that int8_t is used for enums.
template <typename T>
concept DeviceMetricValue = std::same_as<int, T> || std::same_as<float, T> || std::same_as<uint64_t, T> || std::same_as<int8_t, T>;

// Numeric like metrics values.
template <typename T>
concept DeviceMetricNumericValue = std::same_as<int, T> || std::same_as<float, T> || std::same_as<uint64_t, T>;

// Enum like values
template <typename T>
concept DeviceMetricEnumValue = std::same_as<int8_t, T>;

struct DeviceMetricsHelper {
/// Type of the callback which can be provided to be invoked every time a new
/// metric is found by the system.
Expand All @@ -43,68 +55,91 @@ struct DeviceMetricsHelper {
DeviceMetricsInfo& info,
NewMetricCallback newMetricCallback = nullptr);
/// @return the index in metrics for the information of given metric
static size_t metricIdxByName(const std::string& name,
static size_t metricIdxByName(std::string_view const name,
const DeviceMetricsInfo& info);

/// Typesafe way to get the actual store
template <typename T>
template <std::same_as<int> T>
static auto& getMetricsStore(DeviceMetricsInfo& metrics)
{
if constexpr (std::is_same_v<T, int>) {
return metrics.intMetrics;
} else if constexpr (std::is_same_v<T, float>) {
return metrics.floatMetrics;
} else if constexpr (std::is_same_v<T, uint64_t>) {
return metrics.uint64Metrics;
} else if constexpr (std::is_same_v<T, int8_t>) {
return metrics.enumMetrics;
} else {
throw runtime_error("Unhandled metric type");
};
return metrics.intMetrics;
}

template <std::same_as<float> T>
static auto& getMetricsStore(DeviceMetricsInfo& metrics)
{
return metrics.floatMetrics;
}

template <std::same_as<uint64_t> T>
static auto& getMetricsStore(DeviceMetricsInfo& metrics)
{
return metrics.uint64Metrics;
}

template <std::same_as<int8_t> T>
static auto& getMetricsStore(DeviceMetricsInfo& metrics)
{
return metrics.enumMetrics;
}

/// Typesafe way to get the actual store
template <typename T>
template <std::same_as<int> T>
static auto& getTimestampsStore(DeviceMetricsInfo& metrics)
{
if constexpr (std::is_same_v<T, int>) {
return metrics.intTimestamps;
} else if constexpr (std::is_same_v<T, float>) {
return metrics.floatTimestamps;
} else if constexpr (std::is_same_v<T, uint64_t>) {
return metrics.uint64Timestamps;
} else if constexpr (std::is_same_v<T, int8_t>) {
return metrics.enumTimestamps;
} else {
throw runtime_error("Unhandled metric type");
};
return metrics.intTimestamps;
}

template <typename T>
static auto getMetricType()
template <std::same_as<float> T>
static auto& getTimestampsStore(DeviceMetricsInfo& metrics)
{
if constexpr (std::is_same_v<T, int>) {
return MetricType::Int;
} else if constexpr (std::is_same_v<T, float>) {
return MetricType::Float;
} else if constexpr (std::is_same_v<T, uint64_t>) {
return MetricType::Uint64;
} else if constexpr (std::is_same_v<T, int8_t>) {
return MetricType::Enum;
} else {
throw runtime_error("Unhandled metric type");
};
return metrics.floatTimestamps;
}

template <std::same_as<uint64_t> T>
static auto& getTimestampsStore(DeviceMetricsInfo& metrics)
{
return metrics.uint64Timestamps;
}

template <std::same_as<int8_t> T>
static auto& getTimestampsStore(DeviceMetricsInfo& metrics)
{
return metrics.enumTimestamps;
}

template <std::same_as<int> T>
static auto getMetricType() -> MetricType
{
return MetricType::Int;
}

static auto updateNumericInfo(DeviceMetricsInfo& metrics, size_t metricIndex, float value, size_t timestamp) {
metrics.minDomain[metricIndex] = std::min(metrics.minDomain[metricIndex], timestamp);
metrics.maxDomain[metricIndex] = std::max(metrics.maxDomain[metricIndex], timestamp);
metrics.max[metricIndex] = std::max(metrics.max[metricIndex], (float)value);
metrics.min[metricIndex] = std::min(metrics.min[metricIndex], (float)value);
metrics.changed.at(metricIndex) = true;
template <std::same_as<float> T>
static auto getMetricType() -> MetricType
{
return MetricType::Float;
}

template <std::same_as<uint64_t> T>
static auto getMetricType() -> MetricType
{
return MetricType::Uint64;
}

template <std::same_as<int8_t> T>
static auto getMetricType() -> MetricType
{
return MetricType::Enum;
}

static auto updateNumericInfo(DeviceMetricsInfo& metrics, size_t metricIndex, float value, size_t timestamp)
{
metrics.minDomain[metricIndex] = std::min(metrics.minDomain[metricIndex], timestamp);
metrics.maxDomain[metricIndex] = std::max(metrics.maxDomain[metricIndex], timestamp);
metrics.max[metricIndex] = std::max(metrics.max[metricIndex], (float)value);
metrics.min[metricIndex] = std::min(metrics.min[metricIndex], (float)value);
metrics.changed.at(metricIndex) = true;
}

template <typename T>
template <DeviceMetricNumericValue T>
static auto getNumericMetricCursor(size_t metricIndex)
{
return [metricIndex](DeviceMetricsInfo& metrics, T value, size_t timestamp) {
Expand All @@ -123,13 +158,12 @@ struct DeviceMetricsHelper {
static size_t bookMetricInfo(DeviceMetricsInfo& metrics, char const* name, MetricType type);

/// @return helper to insert a given value in the metrics
template <typename T>
template <DeviceMetricNumericValue T>
static size_t
bookNumericMetric(DeviceMetricsInfo& metrics,
char const* name,
NewMetricCallback newMetricsCallback = nullptr)
{
static_assert(std::is_same_v<T, int> || std::is_same_v<T, uint64_t> || std::is_same_v<T, float>, "Unsupported metric type");
size_t metricIndex = bookMetricInfo(metrics, name, getMetricType<T>());
auto& metricInfo = metrics.metrics[metricIndex];
if (newMetricsCallback != nullptr) {
Expand All @@ -139,13 +173,12 @@ struct DeviceMetricsHelper {
}

/// @return helper to insert a given value in the metrics
template <typename T>
template <DeviceMetricNumericValue T>
static std::function<void(DeviceMetricsInfo&, T value, size_t timestamp)>
createNumericMetric(DeviceMetricsInfo& metrics,
char const* name,
NewMetricCallback newMetricsCallback = nullptr)
{
static_assert(std::is_same_v<T, int> || std::is_same_v<T, uint64_t> || std::is_same_v<T, float>, "Unsupported metric type");
size_t metricIndex = bookNumericMetric<T>(metrics, name, newMetricsCallback);
return getNumericMetricCursor<T>(metricIndex);
}
Expand Down
6 changes: 3 additions & 3 deletions Framework/Core/src/DeviceMetricsHelper.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -538,14 +538,14 @@ bool DeviceMetricsHelper::processMetric(ParsedMetricMatch& match,
return true;
}

size_t DeviceMetricsHelper::metricIdxByName(const std::string& name, const DeviceMetricsInfo& info)
size_t DeviceMetricsHelper::metricIdxByName(std::string_view const name, const DeviceMetricsInfo& info)
{
size_t i = 0;
while (i < info.metricLabels.size()) {
auto& metricName = info.metricLabels[i];
std::string_view metricName(info.metricLabels[i].label, info.metricLabels[i].size);
// We check the size first and then the last character because that's
// likely to be different for multi-index metrics
if (metricName.size == name.size() && metricName.label[metricName.size - 1] == name[metricName.size - 1] && memcmp(metricName.label, name.c_str(), metricName.size) == 0) {
if (metricName.size() == name.size() && metricName[metricName.size() - 1] == name[name.size() - 1] && metricName == name) {
return i;
}
++i;
Expand Down

0 comments on commit a12df3e

Please sign in to comment.