version-handler: Refactor maps to simplify accesses
This reduces the number of map lookups needed for read() and close()
operations and the number of stored strings by referencing the pinned
blobId.
Change-Id: I8c6af5749b8cc8415eedeba484bf4a39a98f0286
Signed-off-by: William A. Kennington III <wak@google.com>
diff --git a/bmc/version-handler/version_handler.cpp b/bmc/version-handler/version_handler.cpp
index 6cb8368..76de76c 100644
--- a/bmc/version-handler/version_handler.cpp
+++ b/bmc/version-handler/version_handler.cpp
@@ -12,12 +12,14 @@
{
for (auto& config : configs)
{
- BlobInfo info = {std::move(config.blobId), std::move(config.actions),
- std::move(config.handler)};
- if (!blobInfoMap.try_emplace(info.blobId, std::move(info)).second)
+ auto info = std::make_unique<BlobInfo>();
+ info->blobId = std::move(config.blobId);
+ info->actions = std::move(config.actions);
+ info->handler = std::move(config.handler);
+ if (!blobInfoMap.try_emplace(info->blobId, std::move(info)).second)
{
fprintf(stderr, "Ignoring duplicate config for %s\n",
- info.blobId.c_str());
+ info->blobId.c_str());
}
}
}
@@ -32,7 +34,7 @@
std::vector<std::string> ret;
for (const auto& [key, _] : blobInfoMap)
{
- ret.push_back(key);
+ ret.emplace_back(key);
}
return ret;
}
@@ -57,12 +59,6 @@
bool VersionBlobHandler::open(uint16_t session, uint16_t flags,
const std::string& path)
{
- if (sessionToBlob.insert({session, path}).second == false)
- {
- fprintf(stderr, "open %s fail: session number %d assigned to %s\n",
- path.c_str(), session, sessionToBlob.at(session).c_str());
- return false;
- }
/* only reads are supported, check if blob is handled and make sure
* the blob isn't already opened
*/
@@ -70,47 +66,42 @@
{
fprintf(stderr, "open %s fail: unsupported flags(0x%04X.)\n",
path.c_str(), flags);
- cleanup(session);
return false;
}
- try
+ auto& v = *blobInfoMap.at(path);
+ auto [it, emplaced] = sessionToBlob.try_emplace(session, &v);
+ if (!emplaced)
{
- auto& v = blobInfoMap.at(path);
- if (v.blobState == blobs::StateFlags::open_read)
- {
- cleanup(session);
- fprintf(stderr, "open %s fail: blob already opened for read\n",
- path.c_str());
- return false;
- }
- if (v.actions->onOpen->trigger() == false)
- {
- fprintf(stderr, "open %s fail: onOpen trigger failed\n",
- path.c_str());
- cleanup(session);
- return false;
- }
- v.blobState = blobs::StateFlags::open_read;
- return true;
+ fprintf(stderr, "open %s fail: session number %d assigned to %s\n",
+ path.c_str(), session, it->second->blobId.c_str());
+ return false;
}
- catch (const std::out_of_range& e)
+
+ if (v.blobState == blobs::StateFlags::open_read)
{
- fprintf(stderr, "open %s fail, exception:%s\n", path.c_str(), e.what());
+ fprintf(stderr, "open %s fail: blob already opened for read\n",
+ path.c_str());
cleanup(session);
return false;
}
+ if (v.actions->onOpen->trigger() == false)
+ {
+ fprintf(stderr, "open %s fail: onOpen trigger failed\n", path.c_str());
+ cleanup(session);
+ return false;
+ }
+ v.blobState = blobs::StateFlags::open_read;
+ return true;
}
std::vector<uint8_t> VersionBlobHandler::read(uint16_t session, uint32_t offset,
uint32_t requestedSize)
{
- std::string* blobName;
BlobInfo* pack;
try
{
- blobName = &sessionToBlob.at(session);
- pack = &blobInfoMap.at(*blobName);
+ pack = sessionToBlob.at(session);
}
catch (const std::out_of_range& e)
{
@@ -127,14 +118,14 @@
if (!pack->handler->open("don't care", std::ios::in))
{
fprintf(stderr, "read failed: file open unsuccessful blob=%s\n",
- blobName->c_str());
+ pack->blobId.c_str());
return {};
}
auto d = pack->handler->read(offset, requestedSize);
if (!d)
{
fprintf(stderr, "read failed: unable to read file for blob %s\n",
- blobName->c_str());
+ pack->blobId.c_str());
pack->handler->close();
return {};
}
@@ -161,8 +152,7 @@
{
try
{
- const auto& blobName = sessionToBlob.at(session);
- auto& pack = blobInfoMap.at(blobName);
+ auto& pack = *sessionToBlob.at(session);
if (pack.actions->onOpen->status() == ActionStatus::running)
{
pack.actions->onOpen->abort();
diff --git a/bmc/version-handler/version_handler.hpp b/bmc/version-handler/version_handler.hpp
index ad380b8..891977d 100644
--- a/bmc/version-handler/version_handler.hpp
+++ b/bmc/version-handler/version_handler.hpp
@@ -10,6 +10,7 @@
#include <map>
#include <memory>
#include <string>
+#include <string_view>
#include <unordered_map>
#include <vector>
@@ -68,13 +69,13 @@
private:
struct BlobInfo
{
- std::string blobId;
+ Pinned<std::string> blobId;
std::unique_ptr<ActionPack> actions;
std::unique_ptr<ImageHandlerInterface> handler;
blobs::StateFlags blobState = static_cast<blobs::StateFlags>(0);
};
- std::unordered_map<std::string, BlobInfo> blobInfoMap;
- std::unordered_map<uint16_t, std::string> sessionToBlob;
+ std::unordered_map<std::string_view, std::unique_ptr<BlobInfo>> blobInfoMap;
+ std::unordered_map<uint16_t, BlobInfo*> sessionToBlob;
};
} // namespace ipmi_flash
diff --git a/util.hpp b/util.hpp
index e5197dd..26067b5 100644
--- a/util.hpp
+++ b/util.hpp
@@ -13,4 +13,25 @@
inline constexpr char ubiTarballBlobId[] = "/flash/tarball";
inline constexpr char cleanupBlobId[] = "/flash/cleanup";
+/** @brief Lightweight class wrapper that removes move operations from a class
+ * in order to guarantee the contents stay pinned to a specific location
+ * in memory.
+ */
+template <typename T>
+struct Pinned : public T
+{
+ template <typename... Args>
+ Pinned(Args&&... args) : T(std::forward<Args>(args)...)
+ {}
+ template <typename Arg>
+ Pinned& operator=(const Arg& o)
+ {
+ *static_cast<T*>(this) = o;
+ return *this;
+ }
+
+ Pinned(Pinned&&) = delete;
+ Pinned& operator=(Pinned&&) = delete;
+};
+
} // namespace ipmi_flash