[lldb][NFCI] Remove use of ConstString from OptionValueProperties

bulbazord · bulbazord · commit 643ba926c1f6 · 2023-06-08T15:19:27.000-07:00
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
diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -125,7 +125,7 @@ class OptionValue {
 
   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
@@ -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 @@ class OptionValueProperties
 
   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 @@ class OptionValueProperties
   // 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 @@ class OptionValueProperties
   // "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 @@ class OptionValueProperties
   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<void()> callback);
@@ -176,11 +174,9 @@ class OptionValueProperties
     return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr);
   }
 
-  typedef UniqueCStringMap<size_t> NameToIndex;
-
-  ConstString m_name;
+  std::string m_name;
   std::vector<Property> m_properties;
-  NameToIndex m_name_to_index;
+  llvm::StringMap<size_t> m_name_to_index;
 };
 
 } // namespace lldb_private
diff --git 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();
