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"}, _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits