Skip to content

Remote filesystem perf improvements #5062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 5, 2022
Merged

Remote filesystem perf improvements #5062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1cac5d2
[NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols…
jasonmolenda May 16, 2022
84f3118
Defer source path remap tilde expansion until source file use
jasonmolenda May 26, 2022
2869c4b
Check that a FileSpec has a Directory component before using
jasonmolenda May 26, 2022
ba6126a
One further tweak for realpathing filepath to match dyld
jasonmolenda May 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions lldb/source/Core/SourceManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,14 @@ 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() &&
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),
Expand All @@ -66,10 +74,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());

Expand All @@ -87,9 +98,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>(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);
Expand Down Expand Up @@ -507,6 +518,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
Expand Down
18 changes: 6 additions & 12 deletions lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
ObjectFile::FindPlugin(module_sp, &dsym_fspec, 0,
FileSystem::Instance().GetByteSize(dsym_fspec),
dsym_file_data_sp, dsym_file_data_offset);
// Important to save the dSYM FileSpec so we don't call
// Symbols::LocateExecutableSymbolFile a second time while trying to
// add the symbol ObjectFile to this Module.
if (dsym_objfile_sp && !module_sp->GetSymbolFileFileSpec()) {
module_sp->SetSymbolFileFileSpec(dsym_fspec);
}
if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
// We need a XML parser if we hope to parse a plist...
if (XMLDocument::XMLEnabled()) {
Expand Down Expand Up @@ -236,13 +242,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, ConstString(DBGSourcePath), true);
// With version 2 of DBGSourcePathRemapping, we
Expand Down Expand Up @@ -275,11 +274,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(
ConstString(DBGBuildSourcePath),
ConstString(DBGSourcePath), true);
Expand Down
120 changes: 72 additions & 48 deletions lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
#include "Host/macosx/cfcpp/CFCData.h"
#include "Host/macosx/cfcpp/CFCReleaser.h"
#include "Host/macosx/cfcpp/CFCString.h"
#include "lldb/Core/Module.h"
#include "lldb/Core/ModuleList.h"
#include "lldb/Core/ModuleSpec.h"
#include "lldb/Host/Host.h"
#include "lldb/Host/HostInfo.h"
#include "lldb/Symbol/ObjectFile.h"
#include "lldb/Utility/ArchSpec.h"
#include "lldb/Utility/DataBuffer.h"
Expand Down Expand Up @@ -108,12 +110,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
if (dsym_url.get()) {
if (::CFURLGetFileSystemRepresentation(
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
if (log) {
LLDB_LOGF(log,
"DebugSymbols framework returned dSYM path of %s for "
"UUID %s -- looking for the dSYM",
path, uuid->GetAsString().c_str());
}
LLDB_LOGF(log,
"DebugSymbols framework returned dSYM path of %s for "
"UUID %s -- looking for the dSYM",
path, uuid->GetAsString().c_str());
FileSpec dsym_filespec(path);
if (path[0] == '~')
FileSystem::Instance().Resolve(dsym_filespec);
Expand Down Expand Up @@ -147,16 +147,54 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
uuid_dict = static_cast<CFDictionaryRef>(
::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
}
if (uuid_dict) {

// Check to see if we have the file on the local filesystem.
if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
ModuleSpec exe_spec;
exe_spec.GetFileSpec() = module_spec.GetFileSpec();
exe_spec.GetUUID() = module_spec.GetUUID();
ModuleSP module_sp;
module_sp.reset(new Module(exe_spec));
if (module_sp && module_sp->GetObjectFile() &&
module_sp->MatchesModuleSpec(exe_spec)) {
success = true;
return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
module_spec.GetFileSpec().GetPath().c_str(),
uuid->GetAsString().c_str());
++items_found;
}
}

// Check if the requested image is in our shared cache.
if (!success) {
SharedCacheImageInfo image_info = HostInfo::GetSharedCacheImageInfo(
module_spec.GetFileSpec().GetPath());

// If we found it and it has the correct UUID, let's proceed with
// creating a module from the memory contents.
if (image_info.uuid && (!module_spec.GetUUID() ||
module_spec.GetUUID() == image_info.uuid)) {
success = true;
return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
LLDB_LOGF(log,
"using binary from shared cache for filepath %s for "
"UUID %s",
module_spec.GetFileSpec().GetPath().c_str(),
uuid->GetAsString().c_str());
++items_found;
}
}

// Use the DBGSymbolRichExecutable filepath if present
if (!success && uuid_dict) {
CFStringRef exec_cf_path =
static_cast<CFStringRef>(::CFDictionaryGetValue(
uuid_dict, CFSTR("DBGSymbolRichExecutable")));
if (exec_cf_path && ::CFStringGetFileSystemRepresentation(
exec_cf_path, path, sizeof(path))) {
if (log) {
LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
path, uuid->GetAsString().c_str());
}
LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
path, uuid->GetAsString().c_str());
++items_found;
FileSpec exec_filespec(path);
if (path[0] == '~')
Expand All @@ -168,20 +206,17 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
}
}

// Look next to the dSYM for the binary file.
if (!success) {
// No dictionary, check near the dSYM bundle for an executable that
// matches...
if (::CFURLGetFileSystemRepresentation(
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
char *dsym_extension_pos = ::strstr(path, ".dSYM");
if (dsym_extension_pos) {
*dsym_extension_pos = '\0';
if (log) {
LLDB_LOGF(log,
"Looking for executable binary next to dSYM "
"bundle with name with name %s",
path);
}
LLDB_LOGF(log,
"Looking for executable binary next to dSYM "
"bundle with name with name %s",
path);
FileSpec file_spec(path);
FileSystem::Instance().Resolve(file_spec);
ModuleSpecList module_specs;
Expand All @@ -208,12 +243,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
{
++items_found;
return_module_spec.GetFileSpec() = bundle_exe_file_spec;
if (log) {
LLDB_LOGF(log,
"Executable binary %s next to dSYM is "
"compatible; using",
path);
}
LLDB_LOGF(log,
"Executable binary %s next to dSYM is "
"compatible; using",
path);
}
}
}
Expand All @@ -238,12 +271,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
{
++items_found;
return_module_spec.GetFileSpec() = file_spec;
if (log) {
LLDB_LOGF(log,
"Executable binary %s next to dSYM is "
"compatible; using",
path);
}
LLDB_LOGF(log,
"Executable binary %s next to dSYM is "
"compatible; using",
path);
}
break;
}
Expand Down Expand Up @@ -321,11 +352,9 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
if (CFCString::FileSystemRepresentation(cf_str, str)) {
module_spec.GetFileSpec().SetFile(str.c_str(), FileSpec::Style::native);
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
if (log) {
LLDB_LOGF(log,
"From dsymForUUID plist: Symbol rich executable is at '%s'",
str.c_str());
}
LLDB_LOGF(log,
"From dsymForUUID plist: Symbol rich executable is at '%s'",
str.c_str());
}
}

Expand All @@ -337,10 +366,7 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
FileSpec::Style::native);
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
success = true;
if (log) {
LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'",
str.c_str());
}
LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'", str.c_str());
}
}

Expand Down Expand Up @@ -643,14 +669,12 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
}
}
} else {
if (log) {
if (!uuid_str.empty())
LLDB_LOGF(log, "Called %s on %s, no matches",
g_dsym_for_uuid_exe_path, uuid_str.c_str());
else if (file_path[0] != '\0')
LLDB_LOGF(log, "Called %s on %s, no matches",
g_dsym_for_uuid_exe_path, file_path);
}
if (!uuid_str.empty())
LLDB_LOGF(log, "Called %s on %s, no matches",
g_dsym_for_uuid_exe_path, uuid_str.c_str());
else if (file_path[0] != '\0')
LLDB_LOGF(log, "Called %s on %s, no matches",
g_dsym_for_uuid_exe_path, file_path);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/test/API/commands/platform/sdk/TestPlatformSDK.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def cleanup():
lldbutil.wait_for_file_on_target(self, token)

# Move the binary into the 'SDK'.
rel_exe_path = os.path.relpath(exe, '/')
rel_exe_path = os.path.relpath(os.path.realpath(exe), '/')
exe_sdk_path = os.path.join(symbols_dir, rel_exe_path)
lldbutil.mkdir_p(os.path.dirname(exe_sdk_path))
shutil.move(exe, exe_sdk_path)
Expand Down