Improve sensor tree handling in sdrutils.hpp
This change adds a bimap of sensor number to path to improve
lookup of both types.
Also, with the current implementation each time a sensor number is
requested, the sensor tree must be retrieved from dbus and
parsed. This is fairly expensive and mostly unnecessary
because the sensor tree doesn't change often.
To help improve performance, this change also caches the sensor
tree and number mapping so requests can be returned quickly.
The sensor tree cache is cleared whenever a sensor interface
is added or removed. Regenerating the sensor tree cache
causes the sensor number map cache to also be regenerated so
both contain the latest sensor information.
Tested: Manually added SEL events to verify that they are
mapped to the correct sensors.
'ipmitool sel list' command with 105 SEL entries improved from
8.25 seconds to 1.50 seconds.
Change-Id: I0366e41985c980cbfa28f66a2a979f30ab98141a
Signed-off-by: Jason M. Bills <jason.m.bills@linux.intel.com>
diff --git a/include/sdrutils.hpp b/include/sdrutils.hpp
index bb75ce4..89b64ac 100644
--- a/include/sdrutils.hpp
+++ b/include/sdrutils.hpp
@@ -15,9 +15,11 @@
*/
#include <boost/algorithm/string.hpp>
+#include <boost/bimap.hpp>
#include <boost/container/flat_map.hpp>
#include <cstring>
#include <phosphor-logging/log.hpp>
+#include <sdbusplus/bus/match.hpp>
#pragma once
@@ -34,8 +36,13 @@
boost::container::flat_map<std::string, std::vector<std::string>>,
CmpStrVersion>;
-inline static bool getSensorSubtree(SensorSubTree& subtree)
+using SensorNumMap = boost::bimap<int, std::string>;
+
+namespace details
{
+inline static bool getSensorSubtree(std::shared_ptr<SensorSubTree>& subtree)
+{
+ static std::shared_ptr<SensorSubTree> sensorTreePtr;
sd_bus* bus = NULL;
int ret = sd_bus_default_system(&bus);
if (ret < 0)
@@ -47,6 +54,27 @@
return false;
}
sdbusplus::bus::bus dbus(bus);
+ static sdbusplus::bus::match::match sensorAdded(
+ dbus,
+ "type='signal',member='InterfacesAdded',arg0path='/xyz/openbmc_project/"
+ "sensors/'",
+ [](sdbusplus::message::message& m) { sensorTreePtr.reset(); });
+
+ static sdbusplus::bus::match::match sensorRemoved(
+ dbus,
+ "type='signal',member='InterfacesRemoved',arg0path='/xyz/"
+ "openbmc_project/sensors/'",
+ [](sdbusplus::message::message& m) { sensorTreePtr.reset(); });
+
+ bool sensorTreeUpdated = false;
+ if (sensorTreePtr)
+ {
+ subtree = sensorTreePtr;
+ return sensorTreeUpdated;
+ }
+
+ sensorTreePtr = std::make_shared<SensorSubTree>();
+
auto mapperCall =
dbus.new_method_call("xyz.openbmc_project.ObjectMapper",
"/xyz/openbmc_project/object_mapper",
@@ -61,14 +89,60 @@
try
{
auto mapperReply = dbus.call(mapperCall);
- subtree.clear();
- mapperReply.read(subtree);
+ mapperReply.read(*sensorTreePtr);
}
catch (sdbusplus::exception_t& e)
{
phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
+ return sensorTreeUpdated;
+ }
+ subtree = sensorTreePtr;
+ sensorTreeUpdated = true;
+ return sensorTreeUpdated;
+}
+
+inline static bool getSensorNumMap(std::shared_ptr<SensorNumMap>& sensorNumMap)
+{
+ static std::shared_ptr<SensorNumMap> sensorNumMapPtr;
+ bool sensorNumMapUpated = false;
+
+ std::shared_ptr<SensorSubTree> sensorTree;
+ bool sensorTreeUpdated = details::getSensorSubtree(sensorTree);
+ if (!sensorTree)
+ {
+ return sensorNumMapUpated;
+ }
+
+ if (!sensorTreeUpdated && sensorNumMapPtr)
+ {
+ sensorNumMap = sensorNumMapPtr;
+ return sensorNumMapUpated;
+ }
+
+ sensorNumMapPtr = std::make_shared<SensorNumMap>();
+
+ uint8_t sensorNum = 1;
+ for (const auto& sensor : *sensorTree)
+ {
+ sensorNumMapPtr->insert(
+ SensorNumMap::value_type(sensorNum++, sensor.first));
+ }
+ sensorNumMap = sensorNumMapPtr;
+ sensorNumMapUpated = true;
+ return sensorNumMapUpated;
+}
+} // namespace details
+
+inline static bool getSensorSubtree(SensorSubTree& subtree)
+{
+ std::shared_ptr<SensorSubTree> sensorTree;
+ details::getSensorSubtree(sensorTree);
+ if (!sensorTree)
+ {
return false;
}
+
+ subtree = *sensorTree;
return true;
}
@@ -132,20 +206,22 @@
inline static uint8_t getSensorNumberFromPath(const std::string& path)
{
- SensorSubTree sensorTree;
- if (!getSensorSubtree(sensorTree))
- return 0xFF;
- uint8_t sensorNum = 0xFF;
-
- for (const auto& sensor : sensorTree)
+ std::shared_ptr<SensorNumMap> sensorNumMapPtr;
+ details::getSensorNumMap(sensorNumMapPtr);
+ if (!sensorNumMapPtr)
{
- sensorNum++;
- if (sensor.first == path)
- {
- break;
- }
+ return 0xFF;
}
- return sensorNum;
+
+ try
+ {
+ return sensorNumMapPtr->right.at(path);
+ }
+ catch (std::out_of_range& e)
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
+ return 0xFF;
+ }
}
inline static uint8_t getSensorEventTypeFromPath(const std::string& path)
@@ -156,25 +232,20 @@
inline static std::string getPathFromSensorNumber(uint8_t sensorNum)
{
- SensorSubTree sensorTree;
- std::string path;
- if (!getSensorSubtree(sensorTree))
- return path;
-
- if (sensorTree.size() < sensorNum)
+ std::shared_ptr<SensorNumMap> sensorNumMapPtr;
+ details::getSensorNumMap(sensorNumMapPtr);
+ if (!sensorNumMapPtr)
{
- return path;
+ return std::string();
}
- uint8_t sensorIndex = sensorNum;
- for (const auto& sensor : sensorTree)
+ try
{
- if (sensorIndex-- == 0)
- {
- path = sensor.first;
- break;
- }
+ return sensorNumMapPtr->left.at(sensorNum);
}
-
- return path;
+ catch (std::out_of_range& e)
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(e.what());
+ return std::string();
+ }
}