From 309b370223e0e6db5197ec84417f18250868b23f Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Thu, 1 Feb 2024 23:19:37 -0800 Subject: [PATCH] [glass] Storage: Store Value by value This is possible because std::map has stable value pointers. --- glass/src/lib/native/cpp/Storage.cpp | 158 ++++++++----------- glass/src/lib/native/include/glass/Storage.h | 42 ++++- 2 files changed, 98 insertions(+), 102 deletions(-) diff --git a/glass/src/lib/native/cpp/Storage.cpp b/glass/src/lib/native/cpp/Storage.cpp index d0009937c4..14d113fa29 100644 --- a/glass/src/lib/native/cpp/Storage.cpp +++ b/glass/src/lib/native/cpp/Storage.cpp @@ -198,15 +198,7 @@ Storage::Value* Storage::FindValue(std::string_view key) { if (it == m_values.end()) { return nullptr; } - return it->second.get(); -} - -Storage::Value& Storage::GetValue(std::string_view key) { - auto& val = m_values[key]; - if (!val) { - val = std::make_unique(); - } - return *val; + return &it->second; } #define DEFUN(CapsName, LowerName, CType, CParamType, ArrCType) \ @@ -216,7 +208,7 @@ Storage::Value& Storage::GetValue(std::string_view key) { if (it == m_values.end()) { \ return CType{defaultVal}; \ } \ - Value& value = *it->second; \ + Value& value = it->second; \ if (value.type != Value::k##CapsName) { \ if (!Convert##CapsName(&value)) { \ value.Reset(Value::k##CapsName); \ @@ -229,66 +221,59 @@ Storage::Value& Storage::GetValue(std::string_view key) { } \ \ void Storage::Set##CapsName(std::string_view key, CParamType val) { \ - auto& valuePtr = m_values[key]; \ - if (!valuePtr) { \ - valuePtr = std::make_unique(Value::k##CapsName); \ - } else { \ - valuePtr->Reset(Value::k##CapsName); \ + auto [it, isNew] = m_values.try_emplace(key, Value::k##CapsName); \ + if (!isNew) { \ + it->second.Reset(Value::k##CapsName); \ } \ - valuePtr->LowerName##Val = val; \ - valuePtr->LowerName##Default = {}; \ + it->second.LowerName##Val = val; \ + it->second.LowerName##Default = {}; \ } \ \ CType& Storage::Get##CapsName(std::string_view key, CParamType defaultVal) { \ - auto& valuePtr = m_values[key]; \ - bool setValue = false; \ - if (!valuePtr) { \ - valuePtr = std::make_unique(Value::k##CapsName); \ - setValue = true; \ - } else if (valuePtr->type != Value::k##CapsName) { \ - if (!Convert##CapsName(valuePtr.get())) { \ - valuePtr->Reset(Value::k##CapsName); \ + auto [it, setValue] = m_values.try_emplace(key, Value::k##CapsName); \ + if (!setValue && it->second.type != Value::k##CapsName) { \ + if (!Convert##CapsName(&it->second)) { \ + it->second.Reset(Value::k##CapsName); \ setValue = true; \ } \ } \ + Value& value = it->second; \ if (setValue) { \ - valuePtr->LowerName##Val = defaultVal; \ + value.LowerName##Val = defaultVal; \ } \ - if (!valuePtr->hasDefault) { \ - valuePtr->LowerName##Default = defaultVal; \ - valuePtr->hasDefault = true; \ + if (!value.hasDefault) { \ + value.LowerName##Default = defaultVal; \ + value.hasDefault = true; \ } \ - return valuePtr->LowerName##Val; \ + return value.LowerName##Val; \ } \ \ std::vector& Storage::Get##CapsName##Array( \ std::string_view key, std::span defaultVal) { \ - auto& valuePtr = m_values[key]; \ - bool setValue = false; \ - if (!valuePtr) { \ - valuePtr = std::make_unique(Value::k##CapsName##Array); \ - setValue = true; \ - } else if (valuePtr->type != Value::k##CapsName##Array) { \ - if (!Convert##CapsName##Array(valuePtr.get())) { \ - valuePtr->Reset(Value::k##CapsName##Array); \ + auto [it, setValue] = \ + m_values.try_emplace(key, Value::k##CapsName##Array); \ + if (!setValue && it->second.type != Value::k##CapsName##Array) { \ + if (!Convert##CapsName##Array(&it->second)) { \ + it->second.Reset(Value::k##CapsName##Array); \ setValue = true; \ } \ } \ + Value& value = it->second; \ if (setValue) { \ - valuePtr->LowerName##Array = \ + value.LowerName##Array = \ new std::vector{defaultVal.begin(), defaultVal.end()}; \ } \ - if (!valuePtr->hasDefault) { \ + if (!value.hasDefault) { \ if (defaultVal.empty()) { \ - valuePtr->LowerName##ArrayDefault = nullptr; \ + value.LowerName##ArrayDefault = nullptr; \ } else { \ - valuePtr->LowerName##ArrayDefault = \ + value.LowerName##ArrayDefault = \ new std::vector{defaultVal.begin(), defaultVal.end()}; \ } \ - valuePtr->hasDefault = true; \ + value.hasDefault = true; \ } \ - assert(valuePtr->LowerName##Array); \ - return *valuePtr->LowerName##Array; \ + assert(value.LowerName##Array); \ + return *value.LowerName##Array; \ } DEFUN(Int, int, int, int, int) @@ -303,45 +288,35 @@ Storage& Storage::GetChild(std::string_view label_id) { if (id.empty()) { id = label; } - auto& childPtr = m_values[id]; - if (!childPtr) { - childPtr = std::make_unique(); + Value& childValue = GetValue(id); + if (childValue.type != Value::kChild) { + childValue.Reset(Value::kChild); + childValue.child = new Storage; } - if (childPtr->type != Value::kChild) { - childPtr->Reset(Value::kChild); - childPtr->child = new Storage; - } - return *childPtr->child; + return *childValue.child; } std::vector>& Storage::GetChildArray( std::string_view key) { - auto& valuePtr = m_values[key]; - if (!valuePtr) { - valuePtr = std::make_unique(Value::kChildArray); - valuePtr->childArray = new std::vector>(); - } else if (valuePtr->type != Value::kChildArray) { - valuePtr->Reset(Value::kChildArray); - valuePtr->childArray = new std::vector>(); + auto [it, isNew] = m_values.try_emplace(key, Value::kChildArray); + if (!isNew && it->second.type != Value::kChildArray) { + it->second.Reset(Value::kChildArray); + it->second.childArray = new std::vector>(); } - return *valuePtr->childArray; + return *it->second.childArray; } -std::unique_ptr Storage::Erase(std::string_view key) { +void Storage::Erase(std::string_view key) { auto it = m_values.find(key); if (it != m_values.end()) { - auto rv = std::move(it->second); m_values.erase(it); - return rv; } - return nullptr; } void Storage::EraseChildren() { - std::erase_if(m_values, [](const auto& kv) { - return kv.second->type == Value::kChild; - }); + std::erase_if(m_values, + [](const auto& kv) { return kv.second.type == Value::kChild; }); } static bool JsonArrayToStorage(Storage::Value* valuePtr, const wpi::json& jarr, @@ -473,52 +448,47 @@ bool Storage::FromJson(const wpi::json& json, const char* filename) { return false; } for (auto&& jkv : json.items()) { - auto& valuePtr = m_values[jkv.key()]; - bool created = false; - if (!valuePtr) { - valuePtr = std::make_unique(); - created = true; - } + auto [it, created] = m_values.try_emplace(jkv.key()); auto& jvalue = jkv.value(); switch (jvalue.type()) { case wpi::json::value_t::boolean: - valuePtr->Reset(Value::kBool); - valuePtr->boolVal = jvalue.get(); + it->second.Reset(Value::kBool); + it->second.boolVal = jvalue.get(); break; case wpi::json::value_t::number_float: - valuePtr->Reset(Value::kDouble); - valuePtr->doubleVal = jvalue.get(); + it->second.Reset(Value::kDouble); + it->second.doubleVal = jvalue.get(); break; case wpi::json::value_t::number_integer: - valuePtr->Reset(Value::kInt64); - valuePtr->int64Val = jvalue.get(); + it->second.Reset(Value::kInt64); + it->second.int64Val = jvalue.get(); break; case wpi::json::value_t::number_unsigned: - valuePtr->Reset(Value::kInt64); - valuePtr->int64Val = jvalue.get(); + it->second.Reset(Value::kInt64); + it->second.int64Val = jvalue.get(); break; case wpi::json::value_t::string: - valuePtr->Reset(Value::kString); - valuePtr->stringVal = jvalue.get_ref(); + it->second.Reset(Value::kString); + it->second.stringVal = jvalue.get_ref(); break; case wpi::json::value_t::object: - if (valuePtr->type != Value::kChild) { - valuePtr->Reset(Value::kChild); - valuePtr->child = new Storage; + if (it->second.type != Value::kChild) { + it->second.Reset(Value::kChild); + it->second.child = new Storage; } - valuePtr->child->FromJson(jvalue, filename); // recurse + it->second.child->FromJson(jvalue, filename); // recurse break; case wpi::json::value_t::array: - if (!JsonArrayToStorage(valuePtr.get(), jvalue, filename)) { + if (!JsonArrayToStorage(&it->second, jvalue, filename)) { if (created) { - m_values.erase(jkv.key()); + m_values.erase(it); } } break; default: ImGui::LogText("null value in %s, ignoring", filename); if (created) { - m_values.erase(jkv.key()); + m_values.erase(it); } break; } @@ -557,7 +527,7 @@ wpi::json Storage::ToJson() const { wpi::json j = wpi::json::object(); for (auto&& kv : m_values) { wpi::json jelem; - auto& value = *kv.second; + Value& value = kv.second; switch (value.type) { #define CASE(CapsName, LowerName) \ case Value::k##CapsName: \ @@ -615,7 +585,7 @@ void Storage::Clear() { void Storage::ClearValues() { for (auto&& kv : m_values) { - auto& value = *kv.second; + Value& value = kv.second; switch (value.type) { case Value::kInt: value.intVal = value.intDefault; @@ -701,7 +671,7 @@ void Storage::Apply() { void Storage::ApplyChildren() { for (auto&& kv : m_values) { - auto& value = *kv.second; + Value& value = kv.second; switch (value.type) { case Value::kChild: value.child->Apply(); diff --git a/glass/src/lib/native/include/glass/Storage.h b/glass/src/lib/native/include/glass/Storage.h index 765bbfe8a4..518fa592c0 100644 --- a/glass/src/lib/native/include/glass/Storage.h +++ b/glass/src/lib/native/include/glass/Storage.h @@ -62,6 +62,22 @@ class Storage { explicit Value(Type type) : type{type} {} Value(const Value&) = delete; Value& operator=(const Value&) = delete; + Value(Value&& rhs) + : type{rhs.type}, + stringVal{std::move(rhs.stringVal)}, + stringDefault{std::move(rhs.stringDefault)}, + hasDefault{rhs.hasDefault} { + rhs.type = kNone; + } + Value& operator=(Value&& rhs) { + Reset(kNone); + type = rhs.type; + stringVal = std::move(rhs.stringVal); + stringDefault = std::move(rhs.stringDefault); + hasDefault = rhs.hasDefault; + rhs.type = kNone; + return *this; + } ~Value() { Reset(kNone); } Type type = kNone; @@ -103,7 +119,7 @@ class Storage { void Reset(Type newType); }; - using ValueMap = wpi::StringMap>; + using ValueMap = wpi::StringMap; template using ChildIterator = detail::ChildIterator; @@ -148,7 +164,9 @@ class Storage { std::vector>& GetChildArray(std::string_view key); Value* FindValue(std::string_view key); - Value& GetValue(std::string_view key); + Value& GetValue(std::string_view key) { + return m_values.try_emplace(key, Value::kNone).first->second; + } Storage& GetChild(std::string_view label_id); void SetData(std::shared_ptr&& data) { m_data = std::move(data); } @@ -158,15 +176,23 @@ class Storage { return static_cast(m_data.get()); } + template + T& GetOrNewData(Args&&... args) { + if (!m_data) { + m_data = std::make_shared(std::forward(args)...); + } + return *static_cast(m_data.get()); + } + Storage() = default; Storage(const Storage&) = delete; Storage& operator=(const Storage&) = delete; - void Insert(std::string_view key, std::unique_ptr value) { - m_values[key] = std::move(value); + void Insert(std::string_view key, Value&& value) { + m_values.try_emplace(std::string{key}, std::move(value)); } - std::unique_ptr Erase(std::string_view key); + void Erase(std::string_view key); void EraseAll() { m_values.clear(); } @@ -249,7 +275,7 @@ class ChildIterator { public: ChildIterator(IteratorType it, IteratorType end) noexcept : anchor(it), end(end) { - while (anchor != end && anchor->second->type != Storage::Value::kChild) { + while (anchor != end && anchor->second.type != Storage::Value::kChild) { ++anchor; } } @@ -260,7 +286,7 @@ class ChildIterator { /// increment operator (needed for range-based for) ChildIterator& operator++() { ++anchor; - while (anchor != end && anchor->second->type != Storage::Value::kChild) { + while (anchor != end && anchor->second.type != Storage::Value::kChild) { ++anchor; } return *this; @@ -275,7 +301,7 @@ class ChildIterator { std::string_view key() const { return anchor->first; } /// return value of the iterator - Storage& value() const { return *anchor->second->child; } + Storage& value() const { return *anchor->second.child; } }; } // namespace detail