[Lldb-commits] [PATCH] D125616: [NFC] two small perf fixes for when using a DebugSymbols DBGShellCommands to find a dSYM
Jason Molenda via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Sat May 14 13:06:07 PDT 2022
jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
`SymbolVendorMacOSX::CreateInstance()` calls `LocateMacOSXFilesUsingDebugSymbols()`, to the DebugSymbols framework on macOS, to find a dSYM on the local computer or from a remote server. This method constructs a Module, and it finds a dSYM, creates an ObjectFile for it, then calls SymbolVendor::AddSymbolFileRepresentation with that ObjectFile. The location of the dSYM hasn't been recorded in the Module anywhere, so AddSymbolFileRepresentation eventually (under many layers) calls `LocateMacOSXFilesUsingDebugSymbols()` again. So we call into the DebugSymbols framework twice. It's easy to fix; have `SymbolVendorMacOSX::CreateInstance()` add the dSYM path we found from the first call into `LocateMacOSXFilesUsingDebugSymbols()` in the Module before we call AddSymbolFileRepresentation().
The second perf fix is to `LocateMacOSXFilesUsingDebugSymbols()` so when it is handed a ModuleSpec that has a binary file path, and that file exists, one of the keys we get back from the DebugSymbols framework gives us a "symbol rich binary" (possibly unstripped). This isn't adding anything when we have the dSYM - we can use the original (possibly stripped) binary that we had to begin with. The symbol rich binary may even be over an NFS filesystem, introducing a large performance cost to using it over the local stripped binary.
Pretty straightforward stuff, not sure who to ask for review, this is more my area than anyone else I can think of these days. If anyone has comments, please let me know.
rdar://84576917
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D125616
Files:
lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
===================================================================
--- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
+++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp
@@ -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"
@@ -147,7 +149,52 @@
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 (!module_spec.GetFileSpec().GetPath().empty() &&
+ 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();
+ if (log) {
+ 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();
+ if (log) {
+ 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")));
@@ -168,9 +215,8 @@
}
}
+ // 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");
Index: lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
===================================================================
--- lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
+++ lldb/source/Plugins/SymbolVendor/MacOSX/SymbolVendorMacOSX.cpp
@@ -149,6 +149,12 @@
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.get() && !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()) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D125616.429484.patch
Type: text/x-patch
Size: 4580 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220514/b55062e7/attachment.bin>
More information about the lldb-commits
mailing list