From 643ba926c1f618401c86dc37e659df795db2e1a0 Mon Sep 17 00:00:00 2001 From: Alex Langford Date: Mon, 5 Jun 2023 16:50:24 -0700 Subject: [PATCH] [lldb][NFCI] Remove use of ConstString from OptionValueProperties In the interest of keeping the ConstString StringPool small, this patch aims to remove the use of ConstString from OptionValueProperties. We can maintain quick lookups by using an llvm::StringMap to find the correct index by name. Differential Revision: https://reviews.llvm.org/D152210 --- lldb/include/lldb/Interpreter/OptionValue.h | 2 +- .../lldb/Interpreter/OptionValueProperties.h | 24 ++++----- lldb/source/Interpreter/OptionValueProperties.cpp | 59 ++++++++++++---------- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h index cd587ed..b437153 100644 --- a/lldb/include/lldb/Interpreter/OptionValue.h +++ b/lldb/include/lldb/Interpreter/OptionValue.h @@ -125,7 +125,7 @@ public: virtual bool IsAggregateValue() const { return false; } - virtual ConstString GetName() const { return ConstString(); } + virtual llvm::StringRef GetName() const { return llvm::StringRef(); } virtual bool DumpQualifiedName(Stream &strm) const; diff --git a/lldb/include/lldb/Interpreter/OptionValueProperties.h b/lldb/include/lldb/Interpreter/OptionValueProperties.h index 3f40290..6655c47 100644 --- a/lldb/include/lldb/Interpreter/OptionValueProperties.h +++ b/lldb/include/lldb/Interpreter/OptionValueProperties.h @@ -15,7 +15,6 @@ #include "lldb/Core/UniqueCStringMap.h" #include "lldb/Interpreter/OptionValue.h" #include "lldb/Interpreter/Property.h" -#include "lldb/Utility/ConstString.h" namespace lldb_private { class Properties; @@ -26,7 +25,7 @@ class OptionValueProperties public: OptionValueProperties() = default; - OptionValueProperties(ConstString name); + OptionValueProperties(llvm::StringRef name); ~OptionValueProperties() override = default; @@ -49,7 +48,7 @@ public: llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override; - ConstString GetName() const override { return m_name; } + llvm::StringRef GetName() const override { return m_name; } virtual Status DumpPropertyValue(const ExecutionContext *exe_ctx, Stream &strm, llvm::StringRef property_path, @@ -68,13 +67,13 @@ public: // Get the index of a property given its exact name in this property // collection, "name" can't be a path to a property path that refers to a // property within a property - virtual size_t GetPropertyIndex(ConstString name) const; + virtual size_t GetPropertyIndex(llvm::StringRef name) const; // Get a property by exact name exists in this property collection, name can // not be a path to a property path that refers to a property within a // property virtual const Property * - GetProperty(ConstString name, + GetProperty(llvm::StringRef name, const ExecutionContext *exe_ctx = nullptr) const; virtual const Property * @@ -87,14 +86,13 @@ public: // "target.process.extra-startup-command" virtual const Property * GetPropertyAtPath(const ExecutionContext *exe_ctx, - llvm::StringRef property_path) const; virtual lldb::OptionValueSP GetPropertyValueAtIndex(size_t idx, const ExecutionContext *exe_ctx) const; virtual lldb::OptionValueSP GetValueForKey(const ExecutionContext *exe_ctx, - ConstString key) const; + llvm::StringRef key) const; lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx, llvm::StringRef name, @@ -131,11 +129,11 @@ public: OptionValueFileSpecList *GetPropertyAtIndexAsOptionValueFileSpecList( size_t idx, const ExecutionContext *exe_ctx = nullptr) const; - void AppendProperty(ConstString name, llvm::StringRef desc, bool is_global, - const lldb::OptionValueSP &value_sp); + void AppendProperty(llvm::StringRef name, llvm::StringRef desc, + bool is_global, const lldb::OptionValueSP &value_sp); lldb::OptionValuePropertiesSP GetSubProperty(const ExecutionContext *exe_ctx, - ConstString name); + llvm::StringRef name); void SetValueChangedCallback(size_t property_idx, std::function callback); @@ -176,11 +174,9 @@ protected: return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr); } - typedef UniqueCStringMap NameToIndex; - - ConstString m_name; + std::string m_name; std::vector m_properties; - NameToIndex m_name_to_index; + llvm::StringMap m_name_to_index; }; } // namespace lldb_private diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp index c2b5ceb..62cf408 100644 --- a/lldb/source/Interpreter/OptionValueProperties.cpp +++ b/lldb/source/Interpreter/OptionValueProperties.cpp @@ -20,18 +20,17 @@ using namespace lldb; using namespace lldb_private; -OptionValueProperties::OptionValueProperties(ConstString name) : m_name(name) {} +OptionValueProperties::OptionValueProperties(llvm::StringRef name) + : m_name(name.str()) {} void OptionValueProperties::Initialize(const PropertyDefinitions &defs) { for (const auto &definition : defs) { Property property(definition); assert(property.IsValid()); - m_name_to_index.Append(ConstString(property.GetName()), - m_properties.size()); + m_name_to_index.insert({property.GetName(), m_properties.size()}); property.GetValue()->SetParent(shared_from_this()); m_properties.push_back(property); } - m_name_to_index.Sort(); } void OptionValueProperties::SetValueChangedCallback( @@ -41,24 +40,25 @@ void OptionValueProperties::SetValueChangedCallback( property->SetValueChangedCallback(std::move(callback)); } -void OptionValueProperties::AppendProperty(ConstString name, +void OptionValueProperties::AppendProperty(llvm::StringRef name, llvm::StringRef desc, bool is_global, const OptionValueSP &value_sp) { - Property property(name.GetStringRef(), desc, is_global, value_sp); - m_name_to_index.Append(name, m_properties.size()); + Property property(name, desc, is_global, value_sp); + m_name_to_index.insert({name, m_properties.size()}); m_properties.push_back(property); value_sp->SetParent(shared_from_this()); - m_name_to_index.Sort(); } lldb::OptionValueSP OptionValueProperties::GetValueForKey(const ExecutionContext *exe_ctx, - ConstString key) const { - lldb::OptionValueSP value_sp; - size_t idx = m_name_to_index.Find(key, SIZE_MAX); - if (idx < m_properties.size()) - value_sp = GetPropertyAtIndex(idx, exe_ctx)->GetValue(); - return value_sp; + llvm::StringRef key) const { + auto iter = m_name_to_index.find(key); + if (iter == m_name_to_index.end()) + return OptionValueSP(); + const size_t idx = iter->second; + if (idx >= m_properties.size()) + return OptionValueSP(); + return GetPropertyAtIndex(idx, exe_ctx)->GetValue(); } lldb::OptionValueSP @@ -69,13 +69,13 @@ OptionValueProperties::GetSubValue(const ExecutionContext *exe_ctx, return OptionValueSP(); llvm::StringRef sub_name; - ConstString key; + llvm::StringRef key; size_t key_len = name.find_first_of(".[{"); if (key_len != llvm::StringRef::npos) { - key.SetString(name.take_front(key_len)); + key = name.take_front(key_len); sub_name = name.drop_front(key_len); } else - key.SetString(name); + key = name; value_sp = GetValueForKey(exe_ctx, key); if (sub_name.empty() || !value_sp) @@ -138,14 +138,20 @@ Status OptionValueProperties::SetSubValue(const ExecutionContext *exe_ctx, return error; } -size_t OptionValueProperties::GetPropertyIndex(ConstString name) const { - return m_name_to_index.Find(name, SIZE_MAX); +size_t OptionValueProperties::GetPropertyIndex(llvm::StringRef name) const { + auto iter = m_name_to_index.find(name); + if (iter == m_name_to_index.end()) + return SIZE_MAX; + return iter->second; } const Property * -OptionValueProperties::GetProperty(ConstString name, +OptionValueProperties::GetProperty(llvm::StringRef name, const ExecutionContext *exe_ctx) const { - return GetPropertyAtIndex(m_name_to_index.Find(name, SIZE_MAX), exe_ctx); + auto iter = m_name_to_index.find(name); + if (iter == m_name_to_index.end()) + return nullptr; + return GetPropertyAtIndex(iter->second, exe_ctx); } lldb::OptionValueSP OptionValueProperties::GetPropertyValueAtIndex( @@ -399,18 +405,19 @@ OptionValueProperties::DeepCopy(const OptionValueSP &new_parent) const { const Property * OptionValueProperties::GetPropertyAtPath(const ExecutionContext *exe_ctx, llvm::StringRef name) const { - const Property *property = nullptr; if (name.empty()) return nullptr; + + const Property *property = nullptr; llvm::StringRef sub_name; - ConstString key; + llvm::StringRef key; size_t key_len = name.find_first_of(".[{"); if (key_len != llvm::StringRef::npos) { - key.SetString(name.take_front(key_len)); + key = name.take_front(key_len); sub_name = name.drop_front(key_len); } else - key.SetString(name); + key = name; property = GetProperty(key, exe_ctx); if (sub_name.empty() || !property) @@ -473,7 +480,7 @@ void OptionValueProperties::Apropos( lldb::OptionValuePropertiesSP OptionValueProperties::GetSubProperty(const ExecutionContext *exe_ctx, - ConstString name) { + llvm::StringRef name) { lldb::OptionValueSP option_value_sp(GetValueForKey(exe_ctx, name)); if (option_value_sp) { OptionValueProperties *ov_properties = option_value_sp->GetAsProperties(); -- 2.7.4