[Lldb-commits] [lldb] a5a2a5a - [lldb][NFCI] Remove use of ConstString in StructuredData
Alex Langford via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 14 10:53:52 PDT 2023
Author: Alex Langford
Date: 2023-09-14T10:53:39-07:00
New Revision: a5a2a5a3eca06998d9f71186db2ce78ae2716022
URL: https://github.com/llvm/llvm-project/commit/a5a2a5a3eca06998d9f71186db2ce78ae2716022
DIFF: https://github.com/llvm/llvm-project/commit/a5a2a5a3eca06998d9f71186db2ce78ae2716022.diff
LOG: [lldb][NFCI] Remove use of ConstString in StructuredData
The remaining use of ConstString in StructuredData is the Dictionary
class. Internally it's backed by a `std::map<ConstString, ObjectSP>`.
I propose that we replace it with a `llvm::StringMap<ObjectSP>`.
Many StructuredData::Dictionary objects are ephemeral and only exist for
a short amount of time. Many of these Dictionaries are only produced
once and are never used again. That leaves us with a lot of string data
in the ConstString StringPool that is sitting there never to be used
again. Even if the same string is used many times for keys of different
Dictionary objects, that is something we can measure and adjust for
instead of assuming that every key may be reused at some point in the
future.
Quick comparisons of key data is likely not a concern with Dictionary,
but the use of `llvm::StringMap` means that lookups should be fast with
its hashing strategy.
Switching to a llvm::StringMap meant that the iteration order may be
different. To account for this when serializing/dumping the dictionary,
I added some code to sort the output by key before emitting anything.
Differential Revision: https://reviews.llvm.org/D159313
Added:
Modified:
lldb/include/lldb/Utility/StructuredData.h
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Target/Target.cpp
lldb/source/Utility/StructuredData.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Utility/StructuredData.h b/lldb/include/lldb/Utility/StructuredData.h
index 4133c0e036a22a1..279238f9af76e01 100644
--- a/lldb/include/lldb/Utility/StructuredData.h
+++ b/lldb/include/lldb/Utility/StructuredData.h
@@ -9,10 +9,10 @@
#ifndef LLDB_UTILITY_STRUCTUREDDATA_H
#define LLDB_UTILITY_STRUCTUREDDATA_H
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
-#include "lldb/Utility/ConstString.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Stream.h"
#include "lldb/lldb-enumerations.h"
@@ -423,34 +423,25 @@ class StructuredData {
size_t GetSize() const { return m_dict.size(); }
- void ForEach(std::function<bool(ConstString key, Object *object)> const
+ void ForEach(std::function<bool(llvm::StringRef key, Object *object)> const
&callback) const {
for (const auto &pair : m_dict) {
- if (!callback(pair.first, pair.second.get()))
+ if (!callback(pair.first(), pair.second.get()))
break;
}
}
ArraySP GetKeys() const {
auto array_sp = std::make_shared<Array>();
- collection::const_iterator iter;
- for (iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
- auto key_object_sp = std::make_shared<String>();
- key_object_sp->SetValue(iter->first.AsCString());
+ for (auto iter = m_dict.begin(); iter != m_dict.end(); ++iter) {
+ auto key_object_sp = std::make_shared<String>(iter->first());
array_sp->Push(key_object_sp);
}
return array_sp;
}
ObjectSP GetValueForKey(llvm::StringRef key) const {
- ObjectSP value_sp;
- if (!key.empty()) {
- ConstString key_cs(key);
- collection::const_iterator iter = m_dict.find(key_cs);
- if (iter != m_dict.end())
- value_sp = iter->second;
- }
- return value_sp;
+ return m_dict.lookup(key);
}
bool GetValueForKeyAsBoolean(llvm::StringRef key, bool &result) const {
@@ -539,15 +530,10 @@ class StructuredData {
return false;
}
- bool HasKey(llvm::StringRef key) const {
- ConstString key_cs(key);
- collection::const_iterator search = m_dict.find(key_cs);
- return search != m_dict.end();
- }
+ bool HasKey(llvm::StringRef key) const { return m_dict.contains(key); }
void AddItem(llvm::StringRef key, ObjectSP value_sp) {
- ConstString key_cs(key);
- m_dict[key_cs] = std::move(value_sp);
+ m_dict.insert_or_assign(key, std::move(value_sp));
}
template <typename T> void AddIntegerItem(llvm::StringRef key, T value) {
@@ -577,8 +563,7 @@ class StructuredData {
void GetDescription(lldb_private::Stream &s) const override;
protected:
- typedef std::map<ConstString, ObjectSP> collection;
- collection m_dict;
+ llvm::StringMap<ObjectSP> m_dict;
};
class Null : public Object {
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index ed617fe79fb476c..765902c7cf29b87 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -989,7 +989,7 @@ PlatformDarwin::ExtractAppSpecificInfo(Process &process) {
StructuredData::DictionarySP dict_sp =
std::make_shared<StructuredData::Dictionary>();
- auto flatten_asi_dict = [&dict_sp](ConstString key,
+ auto flatten_asi_dict = [&dict_sp](llvm::StringRef key,
StructuredData::Object *val) -> bool {
if (!val)
return false;
@@ -998,7 +998,7 @@ PlatformDarwin::ExtractAppSpecificInfo(Process &process) {
if (!arr || !arr->GetSize())
return false;
- dict_sp->AddItem(key.AsCString(), arr->GetItemAtIndex(0));
+ dict_sp->AddItem(key, arr->GetItemAtIndex(0));
return true;
};
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 8ca2072b987dc72..2fc446b1dd7ab97 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1926,24 +1926,23 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
lldb::ThreadSP
ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) {
- static ConstString g_key_tid("tid");
- static ConstString g_key_name("name");
- static ConstString g_key_reason("reason");
- static ConstString g_key_metype("metype");
- static ConstString g_key_medata("medata");
- static ConstString g_key_qaddr("qaddr");
- static ConstString g_key_dispatch_queue_t("dispatch_queue_t");
- static ConstString g_key_associated_with_dispatch_queue(
+ static constexpr llvm::StringLiteral g_key_tid("tid");
+ static constexpr llvm::StringLiteral g_key_name("name");
+ static constexpr llvm::StringLiteral g_key_reason("reason");
+ static constexpr llvm::StringLiteral g_key_metype("metype");
+ static constexpr llvm::StringLiteral g_key_medata("medata");
+ static constexpr llvm::StringLiteral g_key_qaddr("qaddr");
+ static constexpr llvm::StringLiteral g_key_dispatch_queue_t(
+ "dispatch_queue_t");
+ static constexpr llvm::StringLiteral g_key_associated_with_dispatch_queue(
"associated_with_dispatch_queue");
- static ConstString g_key_queue_name("qname");
- static ConstString g_key_queue_kind("qkind");
- static ConstString g_key_queue_serial_number("qserialnum");
- static ConstString g_key_registers("registers");
- static ConstString g_key_memory("memory");
- static ConstString g_key_address("address");
- static ConstString g_key_bytes("bytes");
- static ConstString g_key_description("description");
- static ConstString g_key_signal("signal");
+ static constexpr llvm::StringLiteral g_key_queue_name("qname");
+ static constexpr llvm::StringLiteral g_key_queue_kind("qkind");
+ static constexpr llvm::StringLiteral g_key_queue_serial_number("qserialnum");
+ static constexpr llvm::StringLiteral g_key_registers("registers");
+ static constexpr llvm::StringLiteral g_key_memory("memory");
+ static constexpr llvm::StringLiteral g_key_description("description");
+ static constexpr llvm::StringLiteral g_key_signal("signal");
// Stop with signal and thread info
lldb::tid_t tid = LLDB_INVALID_THREAD_ID;
@@ -1971,7 +1970,7 @@ ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) {
&thread_dispatch_qaddr, &queue_vars_valid,
&associated_with_dispatch_queue, &dispatch_queue_t,
&queue_name, &queue_kind, &queue_serial_number](
- ConstString key,
+ llvm::StringRef key,
StructuredData::Object *object) -> bool {
if (key == g_key_tid) {
// thread in big endian hex
@@ -2029,10 +2028,10 @@ ProcessGDBRemote::SetThreadStopInfo(StructuredData::Dictionary *thread_dict) {
if (registers_dict) {
registers_dict->ForEach(
- [&expedited_register_map](ConstString key,
+ [&expedited_register_map](llvm::StringRef key,
StructuredData::Object *object) -> bool {
uint32_t reg;
- if (llvm::to_integer(key.AsCString(), reg))
+ if (llvm::to_integer(key, reg))
expedited_register_map[reg] =
std::string(object->GetStringValue());
return true; // Keep iterating through all array items
diff --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
index 7a1568f0b26c1e5..6008c582454a7ba 100644
--- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -224,7 +224,7 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
[&module_sp, new_style_source_remapping_dictionary,
original_DBGSourcePath_value,
do_truncate_remapping_names](
- ConstString key,
+ llvm::StringRef key,
StructuredData::Object *object) -> bool {
if (object && object->GetAsString()) {
@@ -237,7 +237,7 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
DBGSourcePath = original_DBGSourcePath_value;
}
module_sp->GetSourceMappingList().Append(
- key.GetStringRef(), DBGSourcePath, true);
+ key, DBGSourcePath, true);
// With version 2 of DBGSourcePathRemapping, we
// can chop off the last two filename parts
// from the source remapping and get a more
@@ -245,7 +245,7 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
// Add this as another option in addition to
// the full source path remap.
if (do_truncate_remapping_names) {
- FileSpec build_path(key.AsCString());
+ FileSpec build_path(key);
FileSpec source_path(DBGSourcePath.c_str());
build_path.RemoveLastPathComponent();
build_path.RemoveLastPathComponent();
diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 7429b9e80f26acc..35f5ef324fde667 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3859,11 +3859,10 @@ void Target::StopHookScripted::GetSubclassDescription(
s.Indent("Args:\n");
s.SetIndentLevel(s.GetIndentLevel() + 4);
- auto print_one_element = [&s](ConstString key,
+ auto print_one_element = [&s](llvm::StringRef key,
StructuredData::Object *object) {
s.Indent();
- s.Printf("%s : %s\n", key.GetCString(),
- object->GetStringValue().str().c_str());
+ s.Format("{0} : {1}\n", key, object->GetStringValue());
return true;
};
diff --git a/lldb/source/Utility/StructuredData.cpp b/lldb/source/Utility/StructuredData.cpp
index c870a0eb82b27c4..7686d052c599c67 100644
--- a/lldb/source/Utility/StructuredData.cpp
+++ b/lldb/source/Utility/StructuredData.cpp
@@ -162,8 +162,18 @@ void StructuredData::String::Serialize(json::OStream &s) const {
void StructuredData::Dictionary::Serialize(json::OStream &s) const {
s.objectBegin();
- for (const auto &pair : m_dict) {
- s.attributeBegin(pair.first.GetStringRef());
+
+ // To ensure the output format is always stable, we sort the dictionary by key
+ // first.
+ using Entry = std::pair<llvm::StringRef, ObjectSP>;
+ std::vector<Entry> sorted_entries;
+ for (const auto &pair : m_dict)
+ sorted_entries.push_back({pair.first(), pair.second});
+
+ llvm::sort(sorted_entries);
+
+ for (const auto &pair : sorted_entries) {
+ s.attributeBegin(pair.first);
pair.second->Serialize(s);
s.attributeEnd();
}
@@ -228,9 +238,20 @@ void StructuredData::Array::GetDescription(lldb_private::Stream &s) const {
void StructuredData::Dictionary::GetDescription(lldb_private::Stream &s) const {
size_t indentation_level = s.GetIndentLevel();
- for (auto iter = m_dict.begin(); iter != m_dict.end(); iter++) {
+
+ // To ensure the output format is always stable, we sort the dictionary by key
+ // first.
+ using Entry = std::pair<llvm::StringRef, ObjectSP>;
+ std::vector<Entry> sorted_entries;
+ for (const auto &pair : m_dict)
+ sorted_entries.push_back({pair.first(), pair.second});
+
+ llvm::sort(sorted_entries);
+
+ for (auto iter = sorted_entries.begin(); iter != sorted_entries.end();
+ iter++) {
// Sanitize.
- if (iter->first.IsNull() || iter->first.IsEmpty() || !iter->second)
+ if (iter->first.empty() || !iter->second)
continue;
// Reset original indentation level.
@@ -238,7 +259,7 @@ void StructuredData::Dictionary::GetDescription(lldb_private::Stream &s) const {
s.Indent();
// Print key.
- s.Printf("%s:", iter->first.AsCString());
+ s.Format("{0}:", iter->first);
// Return to new line and increase indentation if value is record type.
// Otherwise add spacing.
@@ -252,7 +273,7 @@ void StructuredData::Dictionary::GetDescription(lldb_private::Stream &s) const {
// Print value and new line if now last pair.
iter->second->GetDescription(s);
- if (std::next(iter) != m_dict.end())
+ if (std::next(iter) != sorted_entries.end())
s.EOL();
// Reset indentation level if it was incremented previously.
More information about the lldb-commits
mailing list