jasonmolenda created this revision.
jasonmolenda added a reviewer: aprantl.
jasonmolenda added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.
In https://reviews.llvm.org/D122373 I changed how the apple simulator platform
plugins would find the SDK path given the name of an SDK -- so we would wait
until we were actually Creating the plugin to call `xcrun` to find the full
path. If a system doesn't have a given SDK installed, this xcrun call can be
very slow, so delaying that search until we know we need a specific simulator
platform to be selected, avoided that problem.
I have a new way of hitting this now, and need to make it even more lazy.
`qProcessInfo` can give lldb a list of addresses of binaries to load -- and in
the case of the kernel, the address is the address of a *kernelset*. We need
some Mach-O fileset specific code to find the kernel in the fileset. So
ProcessGDBRemote force-creates every platform, and asks it, "can you do
something with this address that might be a fileset". PlatformDarwinKernel
knows how to do something. But every platform is force-Created, so if the
create is expensive (as it is with the simulator platforms), we have a big
delay if some SDKs aren't installed on the system.
This patch moves the expansion of (primary sdk name, seconary sdk name) -> sdk
filepath until it's actually used by the simulator platform methods -- much
later than the CreateInstance, when the platform is actually being used. It
will only be calculated once.
I'd call it a NFC change tbh, but it does avoid a very expensive external cost
so it's not completely accurate. It's a big important for this use case.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143932
Files:
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
@@ -49,7 +49,8 @@
const char *class_name, const char *description, ConstString plugin_name,
llvm::Triple::OSType preferred_os,
llvm::SmallVector<llvm::StringRef, 4> supported_triples,
- llvm::StringRef sdk, XcodeSDK::Type sdk_type,
+ std::string sdk_name_primary, std::string sdk_name_secondary,
+ XcodeSDK::Type sdk_type,
CoreSimulatorSupport::DeviceType::ProductFamilyID kind);
static lldb::PlatformSP
@@ -59,7 +60,7 @@
llvm::Triple::OSType preferred_os,
llvm::SmallVector<llvm::Triple::OSType, 4> supported_os,
llvm::SmallVector<llvm::StringRef, 4> supported_triples,
- std::string sdk_name_preferred, std::string sdk_name_secondary,
+ std::string sdk_name_primary, std::string sdk_name_secondary,
XcodeSDK::Type sdk_type,
CoreSimulatorSupport::DeviceType::ProductFamilyID kind,
bool force, const ArchSpec *arch);
@@ -117,8 +118,13 @@
FileSpec GetCoreSimulatorPath();
+ llvm::StringRef GetSDKFilepath();
+
llvm::Triple::OSType m_os_type = llvm::Triple::UnknownOS;
llvm::SmallVector<llvm::StringRef, 4> m_supported_triples = {};
+ std::string m_sdk_name_primary;
+ std::string m_sdk_name_secondary;
+ bool m_have_searched_for_sdk = false;
llvm::StringRef m_sdk;
XcodeSDK::Type m_sdk_type;
Index: lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
===================================================================
--- lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -41,12 +41,14 @@
const char *class_name, const char *description, ConstString plugin_name,
llvm::Triple::OSType preferred_os,
llvm::SmallVector<llvm::StringRef, 4> supported_triples,
- llvm::StringRef sdk, lldb_private::XcodeSDK::Type sdk_type,
+ std::string sdk_name_primary, std::string sdk_name_secondary,
+ lldb_private::XcodeSDK::Type sdk_type,
CoreSimulatorSupport::DeviceType::ProductFamilyID kind)
: PlatformDarwin(true), m_class_name(class_name),
m_description(description), m_plugin_name(plugin_name), m_kind(kind),
m_os_type(preferred_os), m_supported_triples(supported_triples),
- m_sdk(sdk), m_sdk_type(sdk_type) {}
+ m_sdk_name_primary(sdk_name_primary),
+ m_sdk_name_secondary(sdk_name_secondary), m_sdk_type(sdk_type) {}
/// Destructor.
///
@@ -83,8 +85,9 @@
void PlatformAppleSimulator::GetStatus(Stream &strm) {
Platform::GetStatus(strm);
- if (!m_sdk.empty())
- strm << " SDK Path: \"" << m_sdk << "\"\n";
+ llvm::StringRef sdk = GetSDKFilepath();
+ if (!sdk.empty())
+ strm << " SDK Path: \"" << sdk << "\"\n";
else
strm << " SDK Path: error: unable to locate SDK\n";
@@ -295,13 +298,21 @@
return sdk;
}
+llvm::StringRef PlatformAppleSimulator::GetSDKFilepath() {
+ if (!m_have_searched_for_sdk) {
+ m_sdk = GetXcodeSDKDir(m_sdk_name_primary, m_sdk_name_secondary);
+ m_have_searched_for_sdk = true;
+ }
+ return m_sdk;
+}
+
PlatformSP PlatformAppleSimulator::CreateInstance(
const char *class_name, const char *description, ConstString plugin_name,
llvm::SmallVector<llvm::Triple::ArchType, 4> supported_arch,
llvm::Triple::OSType preferred_os,
llvm::SmallVector<llvm::Triple::OSType, 4> supported_os,
llvm::SmallVector<llvm::StringRef, 4> supported_triples,
- std::string sdk_name_preferred, std::string sdk_name_secondary,
+ std::string sdk_name_primary, std::string sdk_name_secondary,
lldb_private::XcodeSDK::Type sdk_type,
CoreSimulatorSupport::DeviceType::ProductFamilyID kind, bool force,
const ArchSpec *arch) {
@@ -360,11 +371,9 @@
if (create) {
LLDB_LOGF(log, "%s::%s() creating platform", class_name, __FUNCTION__);
- llvm::StringRef sdk =
- GetXcodeSDKDir(sdk_name_preferred, sdk_name_secondary);
return PlatformSP(new PlatformAppleSimulator(
class_name, description, plugin_name, preferred_os, supported_triples,
- sdk, sdk_type, kind));
+ sdk_name_primary, sdk_name_secondary, sdk_type, kind));
}
LLDB_LOGF(log, "%s::%s() aborting creation of platform", class_name,
@@ -456,9 +465,10 @@
if (platform_file.GetPath(platform_file_path, sizeof(platform_file_path))) {
char resolved_path[PATH_MAX];
- if (!m_sdk.empty()) {
+ llvm::StringRef sdk = GetSDKFilepath();
+ if (!sdk.empty()) {
::snprintf(resolved_path, sizeof(resolved_path), "%s/%s",
- m_sdk.str().c_str(), platform_file_path);
+ sdk.str().c_str(), platform_file_path);
// First try in the SDK and see if the file is in there
local_file.SetFile(resolved_path, FileSpec::Style::native);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
