bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, fdeazeve, DavidSpickett.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The majority of UnixSignals strings are static in the sense that they do
not change. The overwhelming majority of these strings are string
literals. Using ConstString to manage their lifetime does not make
sense. The only exception to this is one of the subclasses of
UnixSignals, for which I have created a StringSet local to that file
which will guarantee the lifetimes of these StringRefs.
As for the other benefits of ConstString, string uniqueness is not a
concern (as many of them are already string literals) and comparing
signal names and aliases should not be a hot path.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D159011
Files:
lldb/include/lldb/Target/UnixSignals.h
lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
lldb/source/Target/UnixSignals.cpp
Index: lldb/source/Target/UnixSignals.cpp
===================================================================
--- lldb/source/Target/UnixSignals.cpp
+++ lldb/source/Target/UnixSignals.cpp
@@ -18,17 +18,13 @@
using namespace lldb_private;
using namespace llvm;
-UnixSignals::Signal::Signal(const char *name, bool default_suppress,
+UnixSignals::Signal::Signal(llvm::StringRef name, bool default_suppress,
bool default_stop, bool default_notify,
- const char *description, const char *alias)
- : m_name(name), m_alias(alias), m_description(),
+ llvm::StringRef description, llvm::StringRef alias)
+ : m_name(name), m_alias(alias), m_description(description),
m_suppress(default_suppress), m_stop(default_stop),
- m_notify(default_notify),
- m_default_suppress(default_suppress), m_default_stop(default_stop),
- m_default_notify(default_notify) {
- if (description)
- m_description.assign(description);
-}
+ m_notify(default_notify), m_default_suppress(default_suppress),
+ m_default_stop(default_stop), m_default_notify(default_notify) {}
lldb::UnixSignalsSP UnixSignals::Create(const ArchSpec &arch) {
const auto &triple = arch.GetTriple();
@@ -104,9 +100,10 @@
// clang-format on
}
-void UnixSignals::AddSignal(int signo, const char *name, bool default_suppress,
- bool default_stop, bool default_notify,
- const char *description, const char *alias) {
+void UnixSignals::AddSignal(int signo, llvm::StringRef name,
+ bool default_suppress, bool default_stop,
+ bool default_notify, llvm::StringRef description,
+ llvm::StringRef alias) {
Signal new_signal(name, default_suppress, default_stop, default_notify,
description, alias);
m_signals.insert(std::make_pair(signo, new_signal));
@@ -135,7 +132,7 @@
const auto pos = m_signals.find(signo);
if (pos == m_signals.end())
return {};
- return pos->second.m_name.GetStringRef();
+ return pos->second.m_name;
}
std::string
@@ -147,7 +144,7 @@
collection::const_iterator pos = m_signals.find(signo);
if (pos != m_signals.end()) {
- str = pos->second.m_name.GetCString();
+ str = pos->second.m_name.str();
if (code) {
std::map<int32_t, SignalCode>::const_iterator cpos =
@@ -199,14 +196,13 @@
}
int32_t UnixSignals::GetSignalNumberFromName(const char *name) const {
- ConstString const_name(name);
+ llvm::StringRef name_ref(name);
collection::const_iterator pos, end = m_signals.end();
for (pos = m_signals.begin(); pos != end; pos++) {
- if ((const_name == pos->second.m_name) ||
- (const_name == pos->second.m_alias) ||
- (const_name == GetShortName(pos->second.m_name)) ||
- (const_name == GetShortName(pos->second.m_alias)))
+ if ((name_ref == pos->second.m_name) || (name_ref == pos->second.m_alias) ||
+ (name_ref == GetShortName(pos->second.m_name)) ||
+ (name_ref == GetShortName(pos->second.m_alias)))
return pos->first;
}
@@ -373,11 +369,10 @@
json::Value UnixSignals::GetHitCountStatistics() const {
json::Array json_signals;
- for (const auto &pair: m_signals) {
+ for (const auto &pair : m_signals) {
if (pair.second.m_hit_count > 0)
- json_signals.emplace_back(json::Object{
- { pair.second.m_name.GetCString(), pair.second.m_hit_count }
- });
+ json_signals.emplace_back(
+ json::Object{{pair.second.m_name, pair.second.m_hit_count}});
}
return std::move(json_signals);
}
Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
===================================================================
--- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
+++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
@@ -28,6 +28,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/StreamString.h"
#include "lldb/Utility/UriParser.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FormatAdapters.h"
#include "Plugins/Process/Utility/GDBRemoteSignals.h"
@@ -41,6 +42,10 @@
LLDB_PLUGIN_DEFINE_ADV(PlatformRemoteGDBServer, PlatformGDB)
static bool g_initialized = false;
+// UnixSignals does not store the signal names or descriptions itself.
+// It holds onto StringRefs. Becaue we may get signal information dynamically
+// from the remote, these strings need persistent storage client-side.
+static llvm::StringSet<> g_signal_string_storage;
void PlatformRemoteGDBServer::Initialize() {
Platform::Initialize();
@@ -749,8 +754,15 @@
if (object_sp && object_sp->IsValid())
description = std::string(object_sp->GetStringValue());
- remote_signals_sp->AddSignal(signo, name.str().c_str(), suppress, stop,
- notify, description.c_str());
+ const llvm::StringRef name_backed =
+ g_signal_string_storage.insert(name).first->getKeyData();
+ llvm::StringRef description_backed;
+ if (!description.empty())
+ description_backed =
+ g_signal_string_storage.insert(description).first->getKeyData();
+
+ remote_signals_sp->AddSignal(signo, name_backed, suppress, stop, notify,
+ description_backed);
return true;
});
Index: lldb/include/lldb/Target/UnixSignals.h
===================================================================
--- lldb/include/lldb/Target/UnixSignals.h
+++ lldb/include/lldb/Target/UnixSignals.h
@@ -14,7 +14,6 @@
#include <string>
#include <vector>
-#include "lldb/Utility/ConstString.h"
#include "lldb/lldb-private.h"
#include "llvm/Support/JSON.h"
@@ -101,9 +100,10 @@
// your subclass of UnixSignals or in your Process Plugin's GetUnixSignals
// method before you return the UnixSignal object.
- void AddSignal(int signo, const char *name, bool default_suppress,
+ void AddSignal(int signo, llvm::StringRef name, bool default_suppress,
bool default_stop, bool default_notify,
- const char *description, const char *alias = nullptr);
+ llvm::StringRef description,
+ llvm::StringRef alias = llvm::StringRef());
enum SignalCodePrintOption { None, Address, Bounds };
@@ -147,17 +147,20 @@
const SignalCodePrintOption m_print_option;
};
+ // The StringRefs in Signal are either backed by string literals or reside in
+ // persistent storage (e.g. a StringSet).
struct Signal {
- ConstString m_name;
- ConstString m_alias;
- std::string m_description;
+ llvm::StringRef m_name;
+ llvm::StringRef m_alias;
+ llvm::StringRef m_description;
std::map<int32_t, SignalCode> m_codes;
uint32_t m_hit_count = 0;
bool m_suppress : 1, m_stop : 1, m_notify : 1;
bool m_default_suppress : 1, m_default_stop : 1, m_default_notify : 1;
- Signal(const char *name, bool default_suppress, bool default_stop,
- bool default_notify, const char *description, const char *alias);
+ Signal(llvm::StringRef name, bool default_suppress, bool default_stop,
+ bool default_notify, llvm::StringRef description,
+ llvm::StringRef alias);
~Signal() = default;
void Reset(bool reset_stop, bool reset_notify, bool reset_suppress);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
