jasonmolenda created this revision.
jasonmolenda added reviewers: JDevlieghere, jingham.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
Internal to Apple, we have dSYMs that include source path remappings from the
build-system paths to the long-term storage, and the latter point to NFS
mounted directories, using a tilde username component. Currently, lldb expands
those tilde paths to absolute paths when it is parsing the dSYMs, but this
requires a stat of the full NFS filepath to the source directory -- and over a
slow network, that delay can be significant. These initial realpath & stat's
would all happen at initial file load / process launch, which is already a huge
perf bottleneck. It's quite easy to move them out of there, and over a slow
network connection, can yield a large time savings.
This patch keeps the source path remapping names in terms of tilde-username
until we are actually opening one of the source files, at which point we
realpath and test for the path existence. We'll be opening the source file
anyway in a second, so walking the inodes would be done for that - there's no
additional perf penalty from doing the expansion-and-check here.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126435
Files:
lldb/source/Core/SourceManager.cpp
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===================================================================
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -237,13 +237,6 @@
!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 @@
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);
}
Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -66,10 +66,16 @@
if (!file_spec)
return nullptr;
+ FileSpec resolved_fspec = file_spec;
+ if (!FileSystem::Instance().Exists(file_spec) &&
+ file_spec.GetDirectory().GetCString()[0] == '~') {
+ FileSystem::Instance().Resolve(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 +93,9 @@
// 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>(file_spec, target_sp.get());
+ file_sp = std::make_shared<File>(resolved_fspec, target_sp.get());
else
- file_sp = std::make_shared<File>(file_spec, debugger_sp);
+ file_sp = std::make_shared<File>(resolved_fspec, debugger_sp);
if (debugger_sp && debugger_sp->GetUseSourceCache())
debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
@@ -394,6 +400,7 @@
m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
m_debugger_wp(target ? target->GetDebugger().shared_from_this()
: DebuggerSP()) {
+// m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
CommonInitializer(file_spec, target);
}
@@ -441,6 +448,11 @@
}
}
}
+ // Try tilde expansion if the file does not exist
+ if (!FileSystem::Instance().Exists(m_file_spec) &&
+ m_file_spec.GetDirectory().GetCString()[0] == '~') {
+ FileSystem::Instance().Resolve(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
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===================================================================
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -237,13 +237,6 @@
!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 @@
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);
}
Index: lldb/source/Core/SourceManager.cpp
===================================================================
--- lldb/source/Core/SourceManager.cpp
+++ lldb/source/Core/SourceManager.cpp
@@ -66,10 +66,16 @@
if (!file_spec)
return nullptr;
+ FileSpec resolved_fspec = file_spec;
+ if (!FileSystem::Instance().Exists(file_spec) &&
+ file_spec.GetDirectory().GetCString()[0] == '~') {
+ FileSystem::Instance().Resolve(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 +93,9 @@
// 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>(file_spec, target_sp.get());
+ file_sp = std::make_shared<File>(resolved_fspec, target_sp.get());
else
- file_sp = std::make_shared<File>(file_spec, debugger_sp);
+ file_sp = std::make_shared<File>(resolved_fspec, debugger_sp);
if (debugger_sp && debugger_sp->GetUseSourceCache())
debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
@@ -394,6 +400,7 @@
m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
m_debugger_wp(target ? target->GetDebugger().shared_from_this()
: DebuggerSP()) {
+// m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec);
CommonInitializer(file_spec, target);
}
@@ -441,6 +448,11 @@
}
}
}
+ // Try tilde expansion if the file does not exist
+ if (!FileSystem::Instance().Exists(m_file_spec) &&
+ m_file_spec.GetDirectory().GetCString()[0] == '~') {
+ FileSystem::Instance().Resolve(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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
