From c274b6e5830ea88d3f55d6dc1d2b99e38cf6595e Mon Sep 17 00:00:00 2001 From: Jason Molenda Date: Thu, 26 May 2022 00:30:12 -0700 Subject: [PATCH] Defer source path remap tilde expansion until source file use When reading source path remappings out of a dSYM, lldb currently does tilde expansion -- expanding the tilde-username and checking that the destination pathname exists, for each dSYM with the path remappings. This cost happens during lldb's initial process launch / load, an especially perf-sensitive time. Inside Apple, we have dSYMs with source path remappings pointing to NFS directories where these extra stats for every dSYM can be very expensive if the network is slow. This patch instead keeps the source path mapping in the original tilde-username terms and does the tilde expansion when we need to read a specific source file from one of the modules. We'll be stat'ing all of those inodes to load the source file anyway, so the fact that we do the tilde expansion on every source file we load, it doesn't cost us significantly. Differential Revision: https://reviews.llvm.org/D126435 rdar://77091379 --- lldb/source/Core/SourceManager.cpp | 17 ++++++++++++++--- .../SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp | 12 ------------ 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp index 8599c0a6a2284..d4e1791aa3e46 100644 --- a/lldb/source/Core/SourceManager.cpp +++ b/lldb/source/Core/SourceManager.cpp @@ -49,6 +49,13 @@ using namespace lldb_private; static inline bool is_newline_char(char ch) { return ch == '\n' || ch == '\r'; } +static void resolve_tilde(FileSpec &file_spec) { + if (!FileSystem::Instance().Exists(file_spec) && + file_spec.GetDirectory().GetCString()[0] == '~') { + FileSystem::Instance().Resolve(file_spec); + } +} + // SourceManager constructor SourceManager::SourceManager(const TargetSP &target_sp) : m_last_line(0), m_last_count(0), m_default_set(false), @@ -66,10 +73,13 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) { if (!file_spec) return nullptr; + FileSpec resolved_fspec = file_spec; + resolve_tilde(resolved_fspec); + DebuggerSP debugger_sp(m_debugger_wp.lock()); FileSP file_sp; if (debugger_sp && debugger_sp->GetUseSourceCache()) - file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec); + file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec); TargetSP target_sp(m_target_wp.lock()); @@ -87,9 +97,9 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) { // If file_sp is no good or it points to a non-existent file, reset it. if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) { if (target_sp) - file_sp = std::make_shared(file_spec, target_sp.get()); + file_sp = std::make_shared(resolved_fspec, target_sp.get()); else - file_sp = std::make_shared(file_spec, debugger_sp); + file_sp = std::make_shared(resolved_fspec, debugger_sp); if (debugger_sp && debugger_sp->GetUseSourceCache()) debugger_sp->GetSourceFileCache().AddSourceFile(file_sp); @@ -441,6 +451,7 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec, } } } + resolve_tilde(m_file_spec); // Try remapping if m_file_spec does not correspond to an existing file. if (!FileSystem::Instance().Exists(m_file_spec)) { // Check target specific source remappings (i.e., the diff --git a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp index b99ade50d857b..ea1aa7e058710 100644 --- a/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp +++ b/lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp @@ -237,13 +237,6 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp, !original_DBGSourcePath_value.empty()) { DBGSourcePath = original_DBGSourcePath_value; } - if (DBGSourcePath[0] == '~') { - FileSpec resolved_source_path( - DBGSourcePath.c_str()); - FileSystem::Instance().Resolve( - resolved_source_path); - DBGSourcePath = resolved_source_path.GetPath(); - } module_sp->GetSourceMappingList().Append( key.GetStringRef(), DBGSourcePath, true); // With version 2 of DBGSourcePathRemapping, we @@ -275,11 +268,6 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp, DBGBuildSourcePath); plist.GetValueAsString("DBGSourcePath", DBGSourcePath); if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) { - if (DBGSourcePath[0] == '~') { - FileSpec resolved_source_path(DBGSourcePath.c_str()); - FileSystem::Instance().Resolve(resolved_source_path); - DBGSourcePath = resolved_source_path.GetPath(); - } module_sp->GetSourceMappingList().Append( DBGBuildSourcePath, DBGSourcePath, true); }