[Lldb-commits] [lldb] dfd499a - [lldb][NFC] avoid unnecessary roundtrips between different string types
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 1 22:15:12 PDT 2021
Author: Xu Jun
Date: 2021-11-01T22:15:01-07:00
New Revision: dfd499a61c45778b7f01458d50ccc384343f53d5
URL: https://github.com/llvm/llvm-project/commit/dfd499a61c45778b7f01458d50ccc384343f53d5
DIFF: https://github.com/llvm/llvm-project/commit/dfd499a61c45778b7f01458d50ccc384343f53d5.diff
LOG: [lldb][NFC] avoid unnecessary roundtrips between different string types
The amount of roundtrips between StringRefs, ConstStrings and std::strings is
getting a bit out of hand, this patch avoid the unnecessary roundtrips.
Reviewed By: wallace, aprantl
Differential Revision: https://reviews.llvm.org/D112863
Added:
Modified:
lldb/include/lldb/Target/PathMappingList.h
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Module.cpp
lldb/source/Core/ModuleList.cpp
lldb/source/Interpreter/OptionValuePathMappings.cpp
lldb/source/Target/PathMappingList.cpp
lldb/unittests/Target/FindFileTest.cpp
lldb/unittests/Target/PathMappingListTest.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h
index d788d120c47e9..f1cc779ea50fe 100644
--- a/lldb/include/lldb/Target/PathMappingList.h
+++ b/lldb/include/lldb/Target/PathMappingList.h
@@ -32,8 +32,7 @@ class PathMappingList {
const PathMappingList &operator=(const PathMappingList &rhs);
- void Append(ConstString path, ConstString replacement,
- bool notify);
+ void Append(llvm::StringRef path, llvm::StringRef replacement, bool notify);
void Append(const PathMappingList &rhs, bool notify);
@@ -49,17 +48,16 @@ class PathMappingList {
bool GetPathsAtIndex(uint32_t idx, ConstString &path,
ConstString &new_path) const;
- void Insert(ConstString path, ConstString replacement,
+ void Insert(llvm::StringRef path, llvm::StringRef replacement,
uint32_t insert_idx, bool notify);
bool Remove(size_t index, bool notify);
bool Remove(ConstString path, bool notify);
- bool Replace(ConstString path, ConstString replacement,
- bool notify);
+ bool Replace(llvm::StringRef path, llvm::StringRef replacement, bool notify);
- bool Replace(ConstString path, ConstString replacement,
+ bool Replace(llvm::StringRef path, llvm::StringRef replacement,
uint32_t index, bool notify);
bool RemapPath(ConstString path, ConstString &new_path) const;
@@ -104,7 +102,7 @@ class PathMappingList {
/// The newly remapped filespec that is guaranteed to exist.
llvm::Optional<FileSpec> FindFile(const FileSpec &orig_spec) const;
- uint32_t FindIndexForPath(ConstString path) const;
+ uint32_t FindIndexForPath(llvm::StringRef path) const;
uint32_t GetModificationID() const { return m_mod_id; }
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index 9db5b6d03c3fc..98158f457a04f 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -214,8 +214,8 @@ SBStructuredData SBTarget::GetStatistics() {
if (!target_sp)
return LLDB_RECORD_RESULT(data);
std::string json_str =
- llvm::formatv("{0:2}",
- DebuggerStats::ReportStatistics(target_sp->GetDebugger(),
+ llvm::formatv("{0:2}",
+ DebuggerStats::ReportStatistics(target_sp->GetDebugger(),
target_sp.get())).str();
data.m_impl_up->SetObjectSP(StructuredData::ParseJSON(json_str));
return LLDB_RECORD_RESULT(data);
@@ -1586,13 +1586,13 @@ void SBTarget::AppendImageSearchPath(const char *from, const char *to,
if (!target_sp)
return error.SetErrorString("invalid target");
- const ConstString csFrom(from), csTo(to);
- if (!csFrom)
+ llvm::StringRef srFrom = from, srTo = to;
+ if (srFrom.empty())
return error.SetErrorString("<from> path can't be empty");
- if (!csTo)
+ if (srTo.empty())
return error.SetErrorString("<to> path can't be empty");
- target_sp->GetImageSearchPathList().Append(csFrom, csTo, true);
+ target_sp->GetImageSearchPathList().Append(srFrom, srTo, true);
}
lldb::SBModule SBTarget::AddModule(const char *path, const char *triple,
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp b/lldb/source/Commands/CommandObjectTarget.cpp
index e0a88a710fb97..2a42eb22938d7 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1047,8 +1047,7 @@ class CommandObjectTargetModulesSearchPathsAdd : public CommandObjectParsed {
}
bool last_pair = ((argc - i) == 2);
target->GetImageSearchPathList().Append(
- ConstString(from), ConstString(to),
- last_pair); // Notify if this is the last pair
+ from, to, last_pair); // Notify if this is the last pair
result.SetStatus(eReturnStatusSuccessFinishNoResult);
} else {
if (from[0])
@@ -1175,8 +1174,8 @@ class CommandObjectTargetModulesSearchPathsInsert : public CommandObjectParsed {
if (from[0] && to[0]) {
bool last_pair = ((argc - i) == 2);
- target->GetImageSearchPathList().Insert(
- ConstString(from), ConstString(to), insert_idx, last_pair);
+ target->GetImageSearchPathList().Insert(from, to, insert_idx,
+ last_pair);
result.SetStatus(eReturnStatusSuccessFinishNoResult);
} else {
if (from[0])
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index c7538db7dd240..283e18707dbba 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -1614,15 +1614,14 @@ llvm::Optional<std::string> Module::RemapSourceFile(llvm::StringRef path) const
void Module::RegisterXcodeSDK(llvm::StringRef sdk_name, llvm::StringRef sysroot) {
XcodeSDK sdk(sdk_name.str());
- ConstString sdk_path(HostInfo::GetXcodeSDKPath(sdk));
- if (!sdk_path)
+ llvm::StringRef sdk_path(HostInfo::GetXcodeSDKPath(sdk));
+ if (sdk_path.empty())
return;
// If the SDK changed for a previously registered source path, update it.
// This could happend with -fdebug-prefix-map, otherwise it's unlikely.
- ConstString sysroot_cs(sysroot);
- if (!m_source_mappings.Replace(sysroot_cs, sdk_path, true))
+ if (!m_source_mappings.Replace(sysroot, sdk_path, true))
// In the general case, however, append it to the list.
- m_source_mappings.Append(sysroot_cs, sdk_path, false);
+ m_source_mappings.Append(sysroot, sdk_path, false);
}
bool Module::MergeArchitecture(const ArchSpec &arch_spec) {
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 629c075cbc00a..9176c9dbb357b 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -122,8 +122,7 @@ void ModuleListProperties::UpdateSymlinkMappings() {
FileSpec resolved;
Status status = FileSystem::Instance().Readlink(symlink, resolved);
if (status.Success())
- m_symlink_paths.Append(ConstString(symlink.GetPath()),
- ConstString(resolved.GetPath()), notify);
+ m_symlink_paths.Append(symlink.GetPath(), resolved.GetPath(), notify);
}
}
diff --git a/lldb/source/Interpreter/OptionValuePathMappings.cpp b/lldb/source/Interpreter/OptionValuePathMappings.cpp
index e6a366f39061d..543b0e1b8ea8a 100644
--- a/lldb/source/Interpreter/OptionValuePathMappings.cpp
+++ b/lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -62,10 +62,10 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- if (!m_path_mappings.Replace(a, b, idx, m_notify_changes))
- m_path_mappings.Append(a, b, m_notify_changes);
+ if (!m_path_mappings.Replace(orginal_path, replace_path, idx,
+ m_notify_changes))
+ m_path_mappings.Append(orginal_path, replace_path,
+ m_notify_changes);
changed = true;
} else {
std::string previousError =
@@ -102,9 +102,7 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- m_path_mappings.Append(a, b, m_notify_changes);
+ m_path_mappings.Append(orginal_path, replace_path, m_notify_changes);
m_value_was_set = true;
changed = true;
} else {
@@ -139,9 +137,8 @@ Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
const char *orginal_path = args.GetArgumentAtIndex(i);
const char *replace_path = args.GetArgumentAtIndex(i + 1);
if (VerifyPathExists(replace_path)) {
- ConstString a(orginal_path);
- ConstString b(replace_path);
- m_path_mappings.Insert(a, b, idx, m_notify_changes);
+ m_path_mappings.Insert(orginal_path, replace_path, idx,
+ m_notify_changes);
changed = true;
idx++;
} else {
diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
index cd76421cec18a..e49f6213cf27d 100644
--- a/lldb/source/Target/PathMappingList.cpp
+++ b/lldb/source/Target/PathMappingList.cpp
@@ -30,11 +30,11 @@ namespace {
// with the raw path pair, which doesn't work anymore because the paths have
// been normalized when the debug info was loaded. So we need to store
// nomalized path pairs to ensure things match up.
- ConstString NormalizePath(ConstString path) {
- // If we use "path" to construct a FileSpec, it will normalize the path for
- // us. We then grab the string and turn it back into a ConstString.
- return ConstString(FileSpec(path.GetStringRef()).GetPath());
- }
+std::string NormalizePath(llvm::StringRef path) {
+ // If we use "path" to construct a FileSpec, it will normalize the path for
+ // us. We then grab the string.
+ return FileSpec(path).GetPath();
+}
}
// PathMappingList constructor
PathMappingList::PathMappingList() : m_pairs() {}
@@ -59,8 +59,8 @@ const PathMappingList &PathMappingList::operator=(const PathMappingList &rhs) {
PathMappingList::~PathMappingList() = default;
-void PathMappingList::Append(ConstString path,
- ConstString replacement, bool notify) {
+void PathMappingList::Append(llvm::StringRef path, llvm::StringRef replacement,
+ bool notify) {
++m_mod_id;
m_pairs.emplace_back(pair(NormalizePath(path), NormalizePath(replacement)));
if (notify && m_callback)
@@ -78,9 +78,8 @@ void PathMappingList::Append(const PathMappingList &rhs, bool notify) {
}
}
-void PathMappingList::Insert(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+void PathMappingList::Insert(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
++m_mod_id;
iterator insert_iter;
if (index >= m_pairs.size())
@@ -93,9 +92,8 @@ void PathMappingList::Insert(ConstString path,
m_callback(*this, m_callback_baton);
}
-bool PathMappingList::Replace(ConstString path,
- ConstString replacement, uint32_t index,
- bool notify) {
+bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef replacement,
+ uint32_t index, bool notify) {
if (index >= m_pairs.size())
return false;
++m_mod_id;
@@ -221,20 +219,19 @@ llvm::Optional<FileSpec> PathMappingList::FindFile(const FileSpec &orig_spec) co
// We must normalize the orig_spec again using the host's path style,
// otherwise there will be mismatch between the host and remote platform
// if they use
diff erent path styles.
- if (auto remapped = RemapPath(
- NormalizePath(ConstString(orig_spec.GetCString())).GetStringRef(),
- /*only_if_exists=*/true))
+ if (auto remapped = RemapPath(NormalizePath(orig_spec.GetPath()),
+ /*only_if_exists=*/true))
return remapped;
return {};
}
-bool PathMappingList::Replace(ConstString path,
- ConstString new_path, bool notify) {
+bool PathMappingList::Replace(llvm::StringRef path, llvm::StringRef new_path,
+ bool notify) {
uint32_t idx = FindIndexForPath(path);
if (idx < m_pairs.size()) {
++m_mod_id;
- m_pairs[idx].second = new_path;
+ m_pairs[idx].second = ConstString(new_path);
if (notify && m_callback)
m_callback(*this, m_callback_baton);
return true;
@@ -290,8 +287,8 @@ bool PathMappingList::GetPathsAtIndex(uint32_t idx, ConstString &path,
return false;
}
-uint32_t PathMappingList::FindIndexForPath(ConstString orig_path) const {
- const ConstString path = NormalizePath(orig_path);
+uint32_t PathMappingList::FindIndexForPath(llvm::StringRef orig_path) const {
+ const ConstString path = ConstString(NormalizePath(orig_path));
const_iterator pos;
const_iterator begin = m_pairs.begin();
const_iterator end = m_pairs.end();
diff --git a/lldb/unittests/Target/FindFileTest.cpp b/lldb/unittests/Target/FindFileTest.cpp
index 30ac4dfc9f5d1..9d7269780950a 100644
--- a/lldb/unittests/Target/FindFileTest.cpp
+++ b/lldb/unittests/Target/FindFileTest.cpp
@@ -75,8 +75,8 @@ TEST_F(FindFileTest, FindFileTests) {
llvm::FileRemover file_remover(FileName);
PathMappingList map;
- map.Append(ConstString("/old"), ConstString(DirName.str()), false);
- map.Append(ConstString(R"(C:\foo)"), ConstString(DirName.str()), false);
+ map.Append("/old", DirName.str(), false);
+ map.Append(R"(C:\foo)", DirName.str(), false);
Matches matches[] = {
{"/old", llvm::sys::path::Style::posix, DirName.c_str()},
diff --git a/lldb/unittests/Target/PathMappingListTest.cpp b/lldb/unittests/Target/PathMappingListTest.cpp
index 90b6f1134a2b6..31077d83c2c7f 100644
--- a/lldb/unittests/Target/PathMappingListTest.cpp
+++ b/lldb/unittests/Target/PathMappingListTest.cpp
@@ -66,16 +66,16 @@ TEST(PathMappingListTest, RelativeTests) {
#endif
};
PathMappingList map;
- map.Append(ConstString("."), ConstString("/tmp"), false);
+ map.Append(".", "/tmp", false);
TestPathMappings(map, matches, fails);
PathMappingList map2;
- map2.Append(ConstString(""), ConstString("/tmp"), false);
+ map2.Append("", "/tmp", false);
TestPathMappings(map, matches, fails);
}
TEST(PathMappingListTest, AbsoluteTests) {
PathMappingList map;
- map.Append(ConstString("/old"), ConstString("/new"), false);
+ map.Append("/old", "/new", false);
Matches matches[] = {
{"/old", "/new"},
{"/old/", "/new"},
@@ -97,7 +97,7 @@ TEST(PathMappingListTest, AbsoluteTests) {
TEST(PathMappingListTest, RemapRoot) {
PathMappingList map;
- map.Append(ConstString("/"), ConstString("/new"), false);
+ map.Append("/", "/new", false);
Matches matches[] = {
{"/old", "/new/old"},
{"/old/", "/new/old"},
@@ -118,7 +118,7 @@ TEST(PathMappingListTest, RemapRoot) {
#ifndef _WIN32
TEST(PathMappingListTest, CrossPlatformTests) {
PathMappingList map;
- map.Append(ConstString(R"(C:\old)"), ConstString("/new"), false);
+ map.Append(R"(C:\old)", "/new", false);
Matches matches[] = {
{R"(C:\old)", llvm::sys::path::Style::windows, "/new"},
{R"(C:\old\)", llvm::sys::path::Style::windows, "/new"},
More information about the lldb-commits
mailing list