NFC delay tilde expansion on source path remappings until we are opening a source file

[jasonmolenda]

Summary

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.

[JDevlieghere]

Very nice

lldb/source/Core/SourceManager.cpp

76–80

I would create a little helper function (resolve_tilde(FileSpec& s) or something) to avoid duplicating this code on line 541?

407

I guess this shouldn't be here?

[jasonmolenda]

Address Jonas' feedback.

[JDevlieghere]

LGTM

[jasonmolenda]

Temporarily reverted for an x86_64 debian bot failure. Probably my comparison of the first character of the FileSpec path with '~' in resolve_tilde() is crashing with some code path that hands us an empty/uninitialized FileSpec. Will try again tomorrow after looking more closely.

lldb/source/Core/SourceManager.cpp

 class ValueObject;
 }
 
 using namespace lldb;
 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),
       m_target_wp(target_sp),
       m_debugger_wp(target_sp->GetDebugger().shared_from_this()) {}
 
 SourceManager::SourceManager(const DebuggerSP &debugger_sp)
     : m_last_line(0), m_last_count(0), m_default_set(false), m_target_wp(),
       m_debugger_wp(debugger_sp) {}
 
 // Destructor
 SourceManager::~SourceManager() = default;
 
 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());
 
   // It the target source path map has been updated, get this file again so we
   // can successfully remap the source file
   if (target_sp && file_sp &&
       file_sp->GetSourceMapModificationID() !=
           target_sp->GetSourcePathMap().GetModificationID())
     file_sp.reset();
 
   // Update the file contents if needed if we found a file
   if (file_sp)
     file_sp->UpdateIfNeeded();
 
   // 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);
   }
   return file_sp;
 }
 
 static bool should_highlight_source(DebuggerSP debugger_sp) {
SourceManager::File::File(const FileSpec &file_spec,
   CommonInitializer(file_spec, nullptr);
 }
 
 SourceManager::File::File(const FileSpec &file_spec, Target *target)
     : m_file_spec_orig(file_spec), m_file_spec(file_spec),
       m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)),
       m_debugger_wp(target ? target->GetDebugger().shared_from_this()
                            : DebuggerSP()) {
   CommonInitializer(file_spec, target);
 }
 
 void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
                                             Target *target) {
   if (m_mod_time == llvm::sys::TimePoint<>()) {
     if (target) {
       m_source_map_mod_id = target->GetSourcePathMap().GetModificationID();
 
if (target) {
             SymbolContext sc;
             sc_list.GetContextAtIndex(0, sc);
             if (sc.comp_unit)
               m_file_spec = sc.comp_unit->GetPrimaryFile();
             m_mod_time = FileSystem::Instance().GetModificationTime(m_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
         // target.source-map setting), then fall back to the module
         // specific remapping (i.e., the .dSYM remapping dictionary).
         auto remapped = target->GetSourcePathMap().FindFile(m_file_spec);
         if (!remapped) {
           FileSpec new_spec;

lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp

if (dsym_fspec) {
                               // key is DBGBuildSourcePath
                               // object is DBGSourcePath
                               std::string DBGSourcePath =
                                   std::string(object->GetStringValue());
                               if (!new_style_source_remapping_dictionary &&
                                   !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
                               // can chop off the last two filename parts
                               // from the source remapping and get a more
                               // general source remapping that still works.
                               // Add this as another option in addition to
                               // the full source path remap.
if (dsym_fspec) {
 
                     // If we have a DBGBuildSourcePath + DBGSourcePath pair,
                     // append those to the source path remappings.
 
                     plist.GetValueAsString("DBGBuildSourcePath",
                                            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);
                     }
                   }
                 }
               }
             }
           }