Optimize dbus communication for sensor readings This commit introduces a new utility function `getCurrentSteadyClockTimestamp` to retrieve the current steady clock timestamp in milliseconds. Additionally, it updates various sensor classes to utilize this new function for timestamping sensor readings, ensuring consistent and accurate time tracking across the system. Fixes JIRA https://jirasw.nvidia.com/browse/DGXOPENBMC-17812 Signed-off-by: Paweł Iwaneczko <piwaneczko@nvidia.com>
diff --git a/common/utils.cpp b/common/utils.cpp index ba18493..edda83b 100644 --- a/common/utils.cpp +++ b/common/utils.cpp
@@ -251,6 +251,13 @@ return std::nullopt; } +uint64_t getCurrentSteadyClockTimestamp() +{ + return std::chrono::duration_cast<std::chrono::milliseconds>( + std::chrono::steady_clock::now().time_since_epoch()) + .count(); +} + eid_t getEidFromUUID( const std::multimap<uuid_t, std::tuple<eid_t, MctpMedium, MctpBinding>>& eidTable,
diff --git a/common/utils.hpp b/common/utils.hpp index ab629fd..0033841 100644 --- a/common/utils.hpp +++ b/common/utils.hpp
@@ -437,6 +437,12 @@ */ std::string getCurrentSystemTime(); +/** @brief Get the current system time in milliseconds + * + * @return - uint64_t equivalent of the system time + */ +uint64_t getCurrentSteadyClockTimestamp(); + /** @brief Get UUID from the eid * * @return - uuid_t uuid for corresponding eid
diff --git a/nsmd/nsmCommon/sharedMemCommon.cpp b/nsmd/nsmCommon/sharedMemCommon.cpp index 6b92721..28d8297 100644 --- a/nsmd/nsmCommon/sharedMemCommon.cpp +++ b/nsmd/nsmCommon/sharedMemCommon.cpp
@@ -17,9 +17,10 @@ #include "config.h" +#include "utils.hpp" + #include <tal.hpp> -#include <chrono> namespace nsm_shmem_utils { void updateSharedMemoryOnSuccess( @@ -30,10 +31,7 @@ [[maybe_unused]] nv::sensor_aggregation::DbusVariantType propValue) { #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); tal::TelemetryAggregator::updateTelemetry(inventoryObjPath, ifaceName, propName, smbusData, timestamp, 0, propValue);
diff --git a/nsmd/nsmGPM/nsmGpmOem.cpp b/nsmd/nsmGPM/nsmGpmOem.cpp index 6608282..dc6141c 100644 --- a/nsmd/nsmGPM/nsmGpmOem.cpp +++ b/nsmd/nsmGPM/nsmGpmOem.cpp
@@ -41,13 +41,14 @@ void updateMetric([[maybe_unused]] const std::string& name, const double val) override { - (intf->*updateFunc)(val); + if (previousValue != val) + { + (intf->*updateFunc)(val); + previousValue = val; + } #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); DbusVariantType valueVariant{val}; @@ -76,13 +77,14 @@ void updateMetric([[maybe_unused]] const std::string& name, const double val) override { - (intf->*updateFunc)(val); + if (previousValue != val) + { + (intf->*updateFunc)(val); + previousValue = val; + } #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); DbusVariantType valueVariant{val}; @@ -109,13 +111,14 @@ void DRAMUsageMetricUpdator::updateMetric( [[maybe_unused]] const std::string& name, const double val) { - intf->utilization(val); + if (previousValue != val) + { + intf->utilization(val); + previousValue = val; + } #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); DbusVariantType valueVariant{val}; @@ -163,13 +166,14 @@ {} void updateMetric(const std::vector<double>& metrics) override { - (*gpmIntf.*updateFunc)(metrics); + if (previousMetrics != metrics) + { + (*gpmIntf.*updateFunc)(metrics); + previousMetrics = metrics; + } #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); DbusVariantType valueVariant{metrics}; @@ -201,15 +205,22 @@ void updateMetric(const std::vector<double>& metrics) override { const size_t length = std::min(metrics.size(), updatorInfos.size()); + if (previousMetrics.size() != length) + { + previousMetrics.resize(length, + std::numeric_limits<double>::quiet_NaN()); + } + for (size_t i{}; i < length; ++i) { - (updatorInfos[i].interface.get()->*updateFunc)(metrics[i]); + if (previousMetrics[i] != metrics[i]) + { + (updatorInfos[i].interface.get()->*updateFunc)(metrics[i]); + previousMetrics[i] = metrics[i]; + } #ifdef NVIDIA_SHMEM - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); DbusVariantType valueVariant{metrics[i]};
diff --git a/nsmd/nsmGPM/nsmGpmOem.hpp b/nsmd/nsmGPM/nsmGpmOem.hpp index ddf4d3e..92f1898 100644 --- a/nsmd/nsmGPM/nsmGpmOem.hpp +++ b/nsmd/nsmGPM/nsmGpmOem.hpp
@@ -41,6 +41,9 @@ class MetricUpdator { + protected: + double previousValue = std::numeric_limits<double>::quiet_NaN(); + public: virtual ~MetricUpdator() = default; @@ -49,6 +52,9 @@ class MetricPerInstanceUpdator { + protected: + std::vector<double> previousMetrics; + public: virtual ~MetricPerInstanceUpdator() = default; virtual void updateMetric(const std::vector<double>& metrics) = 0;
diff --git a/nsmd/nsmNumericSensor/nsmEnergy.cpp b/nsmd/nsmNumericSensor/nsmEnergy.cpp index b99464f..f4b8887 100644 --- a/nsmd/nsmNumericSensor/nsmEnergy.cpp +++ b/nsmd/nsmNumericSensor/nsmEnergy.cpp
@@ -80,18 +80,17 @@ auto rc = decode_get_current_energy_count_resp( responseMsg, responseLen, &cc, &reasonCode, &readingInMilliJoules); - double readingInJoules = (double)readingInMilliJoules / 1000.0; LG2_ERROR_FLT( "decode_get_current_energy_count_resp failure | reasonCode: {REASONCODE}, cc: {CC}, rc: {RC}", "REASONCODE", reasonCode, "CC", cc, "RC", rc); - if (rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS) - { - sensorValue->updateReading(readingInJoules); - } - else - { - sensorValue->updateReading(std::numeric_limits<double>::quiet_NaN()); - } + + // unit of energy is millijoules in NSM Command Response and selected unit + // in SensorValue PDI is Joules. Hence it is converted to Joules. + auto reading = rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS + ? readingInMilliJoules / 1000.0 + : std::numeric_limits<double>::quiet_NaN(); + sensorValue->updateReading(reading, + utils::getCurrentSteadyClockTimestamp()); return cc ? cc : rc; }
diff --git a/nsmd/nsmNumericSensor/nsmNumericSensor.cpp b/nsmd/nsmNumericSensor/nsmNumericSensor.cpp index e2dcc94..b1e2dc7 100644 --- a/nsmd/nsmNumericSensor/nsmNumericSensor.cpp +++ b/nsmd/nsmNumericSensor/nsmNumericSensor.cpp
@@ -106,13 +106,18 @@ } associationDefinitionsIntf.associations(associations_list); - updateReading(std::numeric_limits<double>::quiet_NaN()); + valueIntf.value(previousValue); } -void NsmNumericSensorDbusValue::updateReading(double value, - uint64_t /*timestamp*/) +void NsmNumericSensorDbusValue::updateReading(double value, uint64_t timestamp) { - valueIntf.value(value); + if (timestamp == 0 || + (previousValue != value && (timestamp - lastTimestamp) > 1000)) + { + lastTimestamp = timestamp; + valueIntf.value(value); + previousValue = value; + } } NsmNumericSensorDbusValueTimestamp::NsmNumericSensorDbusValueTimestamp( @@ -133,8 +138,12 @@ void NsmNumericSensorDbusValueTimestamp::updateReading(double value, uint64_t timestamp) { - timestampIntf.elapsed(timestamp); - NsmNumericSensorDbusValue::updateReading(value); + if (lastTimestamp < timestamp) + { + lastTimestamp = timestamp; + timestampIntf.elapsed(timestamp); + } + NsmNumericSensorDbusValue::updateReading(value, timestamp); } NsmNumericSensorDbusPeakValueTimestamp::NsmNumericSensorDbusPeakValueTimestamp( @@ -242,10 +251,7 @@ void NsmNumericSensorShmem::updateReading(double value, uint64_t /*timestamp*/) { - auto timestamp = static_cast<uint64_t>( - std::chrono::duration_cast<std::chrono::milliseconds>( - std::chrono::steady_clock::now().time_since_epoch()) - .count()); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); nv::sensor_aggregation::DbusVariantType valueVariant{value};
diff --git a/nsmd/nsmNumericSensor/nsmNumericSensor.hpp b/nsmd/nsmNumericSensor/nsmNumericSensor.hpp index 09751a6..0e7847e 100644 --- a/nsmd/nsmNumericSensor/nsmNumericSensor.hpp +++ b/nsmd/nsmNumericSensor/nsmNumericSensor.hpp
@@ -91,6 +91,8 @@ void updateReading(double value, uint64_t timestamp = 0) override; private: + uint64_t lastTimestamp = 0; + double previousValue = std::numeric_limits<double>::quiet_NaN(); ValueIntf valueIntf; AssociationDefinitionsInft associationDefinitionsIntf; DecoratorAreaIntf decoratorAreaIntf; @@ -109,9 +111,10 @@ const std::string& physicalContext, const std::string* implementation, const double maxAllowableValue, const std::string* readingBasis, const std::string* description); - void updateReading(double value, uint64_t timestamp = 0) final; + void updateReading(double value, uint64_t timestamp) final; private: + uint64_t lastTimestamp = 0; TimestampIntf timestampIntf; };
diff --git a/nsmd/nsmNumericSensor/nsmPeakPower.cpp b/nsmd/nsmNumericSensor/nsmPeakPower.cpp index ef0c2f0..3105317 100644 --- a/nsmd/nsmNumericSensor/nsmPeakPower.cpp +++ b/nsmd/nsmNumericSensor/nsmPeakPower.cpp
@@ -76,16 +76,12 @@ LG2_ERROR_FLT( "decode_get_max_observed_power_resp failure | reasonCode: {REASONCODE}, cc: {CC}, rc: {RC}", "REASONCODE", reasonCode, "CC", cc, "RC", rc); - if (rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS) - { - // unit of power is milliwatt in NSM Command Response and selected unit - // on DBus is Watts. Hence it is converted to Watts. - sensorValue->updateReading(reading / 1000.0); - } - else - { - sensorValue->updateReading(std::numeric_limits<double>::quiet_NaN()); - } + // unit of power is milliwatt in NSM Command Response and selected unit + // on DBus is Watts. Hence it is converted to Watts. + double value = rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS + ? reading / 1000.0 + : std::numeric_limits<double>::quiet_NaN(); + sensorValue->updateReading(value, utils::getCurrentSteadyClockTimestamp()); return cc ? cc : rc; }
diff --git a/nsmd/nsmNumericSensor/nsmPower.cpp b/nsmd/nsmNumericSensor/nsmPower.cpp index c98f9c1..18642a0 100644 --- a/nsmd/nsmNumericSensor/nsmPower.cpp +++ b/nsmd/nsmNumericSensor/nsmPower.cpp
@@ -91,16 +91,12 @@ LG2_ERROR_FLT( "decode_get_current_power_draw_resp failure | reasonCode: {REASONCODE}, cc: {CC}, rc: {RC}", "REASONCODE", reasonCode, "CC", cc, "RC", rc); - if (rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS) - { - // unit of power is milliwatt in NSM Command Response and selected unit - // in SensorValue PDI is Watts. Hence it is converted to Watts. - sensorValue->updateReading(reading / 1000.0); - } - else - { - sensorValue->updateReading(std::numeric_limits<double>::quiet_NaN()); - } + // unit of power is milliwatt in NSM Command Response and selected unit + // in SensorValue PDI is Watts. Hence it is converted to Watts. + auto value = rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS + ? reading / 1000.0 + : std::numeric_limits<double>::quiet_NaN(); + sensorValue->updateReading(value, utils::getCurrentSteadyClockTimestamp()); return cc ? cc : rc; }
diff --git a/nsmd/nsmNumericSensor/nsmTemp.cpp b/nsmd/nsmNumericSensor/nsmTemp.cpp index 5beacb5..30bd618 100644 --- a/nsmd/nsmNumericSensor/nsmTemp.cpp +++ b/nsmd/nsmNumericSensor/nsmTemp.cpp
@@ -84,14 +84,12 @@ LG2_ERROR_FLT( "decode_get_temperature_reading_resp failure | reasonCode: {REASONCODE}, cc: {CC}, rc: {RC}", "REASONCODE", reasonCode, "CC", cc, "RC", rc); - if (rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS) - { - sensorValue->updateReading(reading); - } - else - { - sensorValue->updateReading(std::numeric_limits<double>::quiet_NaN()); - } + + reading = rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS + ? reading + : std::numeric_limits<double>::quiet_NaN(); + sensorValue->updateReading(reading, + utils::getCurrentSteadyClockTimestamp()); return cc ? cc : rc; }
diff --git a/nsmd/nsmNumericSensor/nsmVoltage.cpp b/nsmd/nsmNumericSensor/nsmVoltage.cpp index 48acd75..de09a74 100644 --- a/nsmd/nsmNumericSensor/nsmVoltage.cpp +++ b/nsmd/nsmNumericSensor/nsmVoltage.cpp
@@ -74,16 +74,12 @@ LG2_ERROR_FLT( "decode_get_voltage_resp failure | reasonCode: {REASONCODE}, cc: {CC}, rc: {RC}", "REASONCODE", reasonCode, "CC", cc, "RC", rc); - if (rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS) - { - // unit of voltage is microvolts in NSM Command Response and selected - // unit in SensorValue PDI is Volts. Hence it is converted to Volts. - sensorValue->updateReading(reading / 1'000'000.0); - } - else - { - sensorValue->updateReading(std::numeric_limits<double>::quiet_NaN()); - } + // unit of voltage is microvolts in NSM Command Response and selected unit + // in SensorValue PDI is Volts. Hence it is converted to Volts. + double value = rc == NSM_SW_SUCCESS && cc == NSM_SUCCESS + ? reading / 1'000'000.0 + : std::numeric_limits<double>::quiet_NaN(); + sensorValue->updateReading(value, utils::getCurrentSteadyClockTimestamp()); return cc ? cc : rc; }
diff --git a/nsmd/nsmNumericSensor/test/nsmNumericSensors_test.cpp b/nsmd/nsmNumericSensor/test/nsmNumericSensors_test.cpp index 3ddf7db..52f3635 100644 --- a/nsmd/nsmNumericSensor/test/nsmNumericSensors_test.cpp +++ b/nsmd/nsmNumericSensor/test/nsmNumericSensors_test.cpp
@@ -109,7 +109,8 @@ reading, msg); EXPECT_EQ(rc, NSM_SW_SUCCESS); - EXPECT_CALL(*value, updateReading(testing::DoubleNear(reading, 0.01), 0)) + EXPECT_CALL(*value, updateReading(testing::DoubleNear(reading, 0.01), + testing::Gt(0))) .Times(1); sensor.handleResponseMsg(msg, msg_size); @@ -222,7 +223,8 @@ reading, msg); EXPECT_EQ(rc, NSM_SW_SUCCESS); - EXPECT_CALL(*value, updateReading(reading / 1000.0, 0)).Times(1); + EXPECT_CALL(*value, updateReading(reading / 1000.0, testing::Gt(0))) + .Times(1); sensor.handleResponseMsg(msg, msg_size); } @@ -313,7 +315,8 @@ reading, msg); EXPECT_EQ(rc, NSM_SW_SUCCESS); - EXPECT_CALL(*value, updateReading(reading / 1000.0, 0)).Times(1); + EXPECT_CALL(*value, updateReading(reading / 1000.0, testing::Gt(0))) + .Times(1); sensor.handleResponseMsg(msg, msg_size); } @@ -412,7 +415,8 @@ reading, msg); EXPECT_EQ(rc, NSM_SW_SUCCESS); - EXPECT_CALL(*value, updateReading(readingInJoules, 0)).Times(1); + EXPECT_CALL(*value, updateReading(readingInJoules, testing::Gt(0))) + .Times(1); sensor.handleResponseMsg(msg, msg_size); } @@ -505,7 +509,8 @@ msg); EXPECT_EQ(rc, NSM_SW_SUCCESS); - EXPECT_CALL(*value, updateReading(reading / 1'000'000.0, 0)).Times(1); + EXPECT_CALL(*value, updateReading(reading / 1'000'000.0, testing::Gt(0))) + .Times(1); sensor.handleResponseMsg(msg, msg_size); }
diff --git a/nsmd/nsmNumericSensor/test/nsmNumeric_test.cpp b/nsmd/nsmNumericSensor/test/nsmNumeric_test.cpp index cec5eb2..2ea620f 100644 --- a/nsmd/nsmNumericSensor/test/nsmNumeric_test.cpp +++ b/nsmd/nsmNumericSensor/test/nsmNumeric_test.cpp
@@ -45,7 +45,6 @@ static const std::string description("dummy_sensor"); static const double val{32432.8970}; -static const uint64_t timestamp{13432548}; TEST(NsmNumericSensorDbusValue, GoodTest) { @@ -65,6 +64,7 @@ bus, sensorName, sensorType, nsm::SensorUnit::DegreesC, associations, physicalContexnt, nullptr, maxAllowableValue, &readingBasis, &description}; + auto timestamp = utils::getCurrentSteadyClockTimestamp(); value.updateReading(val, timestamp); EXPECT_EQ(value.timestampIntf.elapsed(), timestamp); @@ -163,6 +163,7 @@ EXPECT_EQ(aggregator.objects.size(), 2); + auto timestamp = utils::getCurrentSteadyClockTimestamp(); EXPECT_CALL(*elem1Ptr, updateReading(val, timestamp)).Times(1); EXPECT_CALL(*elem2Ptr, updateReading(val, timestamp)).Times(1);
diff --git a/nsmd/sensorManager.cpp b/nsmd/sensorManager.cpp index 9a6d236..fa82704 100644 --- a/nsmd/sensorManager.cpp +++ b/nsmd/sensorManager.cpp
@@ -31,6 +31,8 @@ #include "sensorQueueMap.hpp" #include "utils.hpp" +#include <ranges> + #ifdef LTTNG_TRACING #include "tracepoints/nsmd-tp.h" #endif