Skip to content

Commit 1a0c6d9

Browse files
jasonmolendajasonmolenda
authored
Remote filesystem perf improvements (#5062)
* [NFC] Don't bother with unstripped binary w/ dSYM, don't DebugSymbols twice This patch addresses two perf issues when we find a dSYM on macOS after calling into the DebugSymbols framework. First, when we have a local (probably stripped) binaary, we find the dSYM and we may be told about the location of the symbol rich binary (probably unstripped) which may be on a remote filesystem. We don't need the unstripped binary, use the local binary we already have. Second, after we've found the path to the dSYM, save that in the Module so we don't call into DebugSymbols a second time later on to rediscover it. If the user has a DBGShellCommands set, we need to exec that process twice, serially, which can add up. Differential Revision: https://reviews.llvm.org/D125616 rdar://84576917 (cherry picked from commit d2f3b60) * 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 (cherry picked from commit c274b6e) (cherry picked from commit 8a46728) * Check that a FileSpec has a Directory component before using A follow on to my patch for https://reviews.llvm.org/D126435 hit by an x86_64 linux bot; I assumed that a FileSpec had a directory component and checked if the first character was a '~'. This was not a valid assumption. (cherry picked from commit bd67468) * One further tweak for realpathing filepath to match dyld I missed one place I need to realpath the build artifact path, to make it match the path we get back from dyld. (cherry picked from commit ea6d0c4)
1 parent 774f7a7 commit 1a0c6d9

File tree

4 files changed

+94
-64
lines changed

4 files changed

+94
-64
lines changed

‎lldb/source/Core/SourceManager.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,14 @@ using namespace lldb_private;
4949

5050
static inline bool is_newline_char(char ch) { return ch == '\n' || ch == '\r'; }
5151

52+
static void resolve_tilde(FileSpec &file_spec) {
53+
if (!FileSystem::Instance().Exists(file_spec) &&
54+
file_spec.GetDirectory() &&
55+
file_spec.GetDirectory().GetCString()[0] == '~') {
56+
FileSystem::Instance().Resolve(file_spec);
57+
}
58+
}
59+
5260
// SourceManager constructor
5361
SourceManager::SourceManager(const TargetSP &target_sp)
5462
: m_last_line(0), m_last_count(0), m_default_set(false),
@@ -66,10 +74,13 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
6674
if (!file_spec)
6775
return nullptr;
6876

77+
FileSpec resolved_fspec = file_spec;
78+
resolve_tilde(resolved_fspec);
79+
6980
DebuggerSP debugger_sp(m_debugger_wp.lock());
7081
FileSP file_sp;
7182
if (debugger_sp && debugger_sp->GetUseSourceCache())
72-
file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec);
83+
file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec);
7384

7485
TargetSP target_sp(m_target_wp.lock());
7586

@@ -87,9 +98,9 @@ SourceManager::FileSP SourceManager::GetFile(const FileSpec &file_spec) {
8798
// If file_sp is no good or it points to a non-existent file, reset it.
8899
if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) {
89100
if (target_sp)
90-
file_sp = std::make_shared<File>(file_spec, target_sp.get());
101+
file_sp = std::make_shared<File>(resolved_fspec, target_sp.get());
91102
else
92-
file_sp = std::make_shared<File>(file_spec, debugger_sp);
103+
file_sp = std::make_shared<File>(resolved_fspec, debugger_sp);
93104

94105
if (debugger_sp && debugger_sp->GetUseSourceCache())
95106
debugger_sp->GetSourceFileCache().AddSourceFile(file_sp);
@@ -507,6 +518,7 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec,
507518
}
508519
}
509520
}
521+
resolve_tilde(m_file_spec);
510522
// Try remapping if m_file_spec does not correspond to an existing file.
511523
if (!FileSystem::Instance().Exists(m_file_spec)) {
512524
// Check target specific source remappings (i.e., the

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

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,12 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
154154
ObjectFile::FindPlugin(module_sp, &dsym_fspec, 0,
155155
FileSystem::Instance().GetByteSize(dsym_fspec),
156156
dsym_file_data_sp, dsym_file_data_offset);
157+
// Important to save the dSYM FileSpec so we don't call
158+
// Symbols::LocateExecutableSymbolFile a second time while trying to
159+
// add the symbol ObjectFile to this Module.
160+
if (dsym_objfile_sp && !module_sp->GetSymbolFileFileSpec()) {
161+
module_sp->SetSymbolFileFileSpec(dsym_fspec);
162+
}
157163
if (UUIDsMatch(module_sp.get(), dsym_objfile_sp.get(), feedback_strm)) {
158164
// We need a XML parser if we hope to parse a plist...
159165
if (XMLDocument::XMLEnabled()) {
@@ -236,13 +242,6 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
236242
!original_DBGSourcePath_value.empty()) {
237243
DBGSourcePath = original_DBGSourcePath_value;
238244
}
239-
if (DBGSourcePath[0] == '~') {
240-
FileSpec resolved_source_path(
241-
DBGSourcePath.c_str());
242-
FileSystem::Instance().Resolve(
243-
resolved_source_path);
244-
DBGSourcePath = resolved_source_path.GetPath();
245-
}
246245
module_sp->GetSourceMappingList().Append(
247246
key, ConstString(DBGSourcePath), true);
248247
// With version 2 of DBGSourcePathRemapping, we
@@ -275,11 +274,6 @@ SymbolVendorMacOSX::CreateInstance(const lldb::ModuleSP &module_sp,
275274
DBGBuildSourcePath);
276275
plist.GetValueAsString("DBGSourcePath", DBGSourcePath);
277276
if (!DBGBuildSourcePath.empty() && !DBGSourcePath.empty()) {
278-
if (DBGSourcePath[0] == '~') {
279-
FileSpec resolved_source_path(DBGSourcePath.c_str());
280-
FileSystem::Instance().Resolve(resolved_source_path);
281-
DBGSourcePath = resolved_source_path.GetPath();
282-
}
283277
module_sp->GetSourceMappingList().Append(
284278
ConstString(DBGBuildSourcePath),
285279
ConstString(DBGSourcePath), true);

‎lldb/source/Symbol/LocateSymbolFileMacOSX.cpp

Lines changed: 72 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,11 @@
1818
#include "Host/macosx/cfcpp/CFCData.h"
1919
#include "Host/macosx/cfcpp/CFCReleaser.h"
2020
#include "Host/macosx/cfcpp/CFCString.h"
21+
#include "lldb/Core/Module.h"
2122
#include "lldb/Core/ModuleList.h"
2223
#include "lldb/Core/ModuleSpec.h"
2324
#include "lldb/Host/Host.h"
25+
#include "lldb/Host/HostInfo.h"
2426
#include "lldb/Symbol/ObjectFile.h"
2527
#include "lldb/Utility/ArchSpec.h"
2628
#include "lldb/Utility/DataBuffer.h"
@@ -108,12 +110,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
108110
if (dsym_url.get()) {
109111
if (::CFURLGetFileSystemRepresentation(
110112
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
111-
if (log) {
112-
LLDB_LOGF(log,
113-
"DebugSymbols framework returned dSYM path of %s for "
114-
"UUID %s -- looking for the dSYM",
115-
path, uuid->GetAsString().c_str());
116-
}
113+
LLDB_LOGF(log,
114+
"DebugSymbols framework returned dSYM path of %s for "
115+
"UUID %s -- looking for the dSYM",
116+
path, uuid->GetAsString().c_str());
117117
FileSpec dsym_filespec(path);
118118
if (path[0] == '~')
119119
FileSystem::Instance().Resolve(dsym_filespec);
@@ -147,16 +147,54 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
147147
uuid_dict = static_cast<CFDictionaryRef>(
148148
::CFDictionaryGetValue(dict.get(), uuid_cfstr.get()));
149149
}
150-
if (uuid_dict) {
150+
151+
// Check to see if we have the file on the local filesystem.
152+
if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
153+
ModuleSpec exe_spec;
154+
exe_spec.GetFileSpec() = module_spec.GetFileSpec();
155+
exe_spec.GetUUID() = module_spec.GetUUID();
156+
ModuleSP module_sp;
157+
module_sp.reset(new Module(exe_spec));
158+
if (module_sp && module_sp->GetObjectFile() &&
159+
module_sp->MatchesModuleSpec(exe_spec)) {
160+
success = true;
161+
return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
162+
LLDB_LOGF(log, "using original binary filepath %s for UUID %s",
163+
module_spec.GetFileSpec().GetPath().c_str(),
164+
uuid->GetAsString().c_str());
165+
++items_found;
166+
}
167+
}
168+
169+
// Check if the requested image is in our shared cache.
170+
if (!success) {
171+
SharedCacheImageInfo image_info = HostInfo::GetSharedCacheImageInfo(
172+
module_spec.GetFileSpec().GetPath());
173+
174+
// If we found it and it has the correct UUID, let's proceed with
175+
// creating a module from the memory contents.
176+
if (image_info.uuid && (!module_spec.GetUUID() ||
177+
module_spec.GetUUID() == image_info.uuid)) {
178+
success = true;
179+
return_module_spec.GetFileSpec() = module_spec.GetFileSpec();
180+
LLDB_LOGF(log,
181+
"using binary from shared cache for filepath %s for "
182+
"UUID %s",
183+
module_spec.GetFileSpec().GetPath().c_str(),
184+
uuid->GetAsString().c_str());
185+
++items_found;
186+
}
187+
}
188+
189+
// Use the DBGSymbolRichExecutable filepath if present
190+
if (!success && uuid_dict) {
151191
CFStringRef exec_cf_path =
152192
static_cast<CFStringRef>(::CFDictionaryGetValue(
153193
uuid_dict, CFSTR("DBGSymbolRichExecutable")));
154194
if (exec_cf_path && ::CFStringGetFileSystemRepresentation(
155195
exec_cf_path, path, sizeof(path))) {
156-
if (log) {
157-
LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
158-
path, uuid->GetAsString().c_str());
159-
}
196+
LLDB_LOGF(log, "plist bundle has exec path of %s for UUID %s",
197+
path, uuid->GetAsString().c_str());
160198
++items_found;
161199
FileSpec exec_filespec(path);
162200
if (path[0] == '~')
@@ -168,20 +206,17 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
168206
}
169207
}
170208

209+
// Look next to the dSYM for the binary file.
171210
if (!success) {
172-
// No dictionary, check near the dSYM bundle for an executable that
173-
// matches...
174211
if (::CFURLGetFileSystemRepresentation(
175212
dsym_url.get(), true, (UInt8 *)path, sizeof(path) - 1)) {
176213
char *dsym_extension_pos = ::strstr(path, ".dSYM");
177214
if (dsym_extension_pos) {
178215
*dsym_extension_pos = '\0';
179-
if (log) {
180-
LLDB_LOGF(log,
181-
"Looking for executable binary next to dSYM "
182-
"bundle with name with name %s",
183-
path);
184-
}
216+
LLDB_LOGF(log,
217+
"Looking for executable binary next to dSYM "
218+
"bundle with name with name %s",
219+
path);
185220
FileSpec file_spec(path);
186221
FileSystem::Instance().Resolve(file_spec);
187222
ModuleSpecList module_specs;
@@ -208,12 +243,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
208243
{
209244
++items_found;
210245
return_module_spec.GetFileSpec() = bundle_exe_file_spec;
211-
if (log) {
212-
LLDB_LOGF(log,
213-
"Executable binary %s next to dSYM is "
214-
"compatible; using",
215-
path);
216-
}
246+
LLDB_LOGF(log,
247+
"Executable binary %s next to dSYM is "
248+
"compatible; using",
249+
path);
217250
}
218251
}
219252
}
@@ -238,12 +271,10 @@ int LocateMacOSXFilesUsingDebugSymbols(const ModuleSpec &module_spec,
238271
{
239272
++items_found;
240273
return_module_spec.GetFileSpec() = file_spec;
241-
if (log) {
242-
LLDB_LOGF(log,
243-
"Executable binary %s next to dSYM is "
244-
"compatible; using",
245-
path);
246-
}
274+
LLDB_LOGF(log,
275+
"Executable binary %s next to dSYM is "
276+
"compatible; using",
277+
path);
247278
}
248279
break;
249280
}
@@ -321,11 +352,9 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
321352
if (CFCString::FileSystemRepresentation(cf_str, str)) {
322353
module_spec.GetFileSpec().SetFile(str.c_str(), FileSpec::Style::native);
323354
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
324-
if (log) {
325-
LLDB_LOGF(log,
326-
"From dsymForUUID plist: Symbol rich executable is at '%s'",
327-
str.c_str());
328-
}
355+
LLDB_LOGF(log,
356+
"From dsymForUUID plist: Symbol rich executable is at '%s'",
357+
str.c_str());
329358
}
330359
}
331360

@@ -337,10 +366,7 @@ static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict,
337366
FileSpec::Style::native);
338367
FileSystem::Instance().Resolve(module_spec.GetFileSpec());
339368
success = true;
340-
if (log) {
341-
LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'",
342-
str.c_str());
343-
}
369+
LLDB_LOGF(log, "From dsymForUUID plist: dSYM is at '%s'", str.c_str());
344370
}
345371
}
346372

@@ -643,14 +669,12 @@ bool Symbols::DownloadObjectAndSymbolFile(ModuleSpec &module_spec,
643669
}
644670
}
645671
} else {
646-
if (log) {
647-
if (!uuid_str.empty())
648-
LLDB_LOGF(log, "Called %s on %s, no matches",
649-
g_dsym_for_uuid_exe_path, uuid_str.c_str());
650-
else if (file_path[0] != '\0')
651-
LLDB_LOGF(log, "Called %s on %s, no matches",
652-
g_dsym_for_uuid_exe_path, file_path);
653-
}
672+
if (!uuid_str.empty())
673+
LLDB_LOGF(log, "Called %s on %s, no matches",
674+
g_dsym_for_uuid_exe_path, uuid_str.c_str());
675+
else if (file_path[0] != '\0')
676+
LLDB_LOGF(log, "Called %s on %s, no matches",
677+
g_dsym_for_uuid_exe_path, file_path);
654678
}
655679
}
656680
}

‎lldb/test/API/commands/platform/sdk/TestPlatformSDK.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ def cleanup():
8282
lldbutil.wait_for_file_on_target(self, token)
8383

8484
# Move the binary into the 'SDK'.
85-
rel_exe_path = os.path.relpath(exe, '/')
85+
rel_exe_path = os.path.relpath(os.path.realpath(exe), '/')
8686
exe_sdk_path = os.path.join(symbols_dir, rel_exe_path)
8787
lldbutil.mkdir_p(os.path.dirname(exe_sdk_path))
8888
shutil.move(exe, exe_sdk_path)

0 commit comments

Comments
 (0)