[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform (PR #140611)
https://github.com/talkeren closed https://github.com/llvm/llvm-project/pull/140611 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform (PR #140611)
talkeren wrote: Closing it following the discussion in the issue - I made a new PR that fixes it differently without change the current behaviour. https://github.com/llvm/llvm-project/pull/140611 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Don't query the platform for each load command (PR #140853)
https://github.com/talkeren created
https://github.com/llvm/llvm-project/pull/140853
This change remove the call to GetPlatform from GetDeploymentInfo and instead
pass it as an argument,so GetPlatform will be called only once for all the
load commands. The reason is that if we try to query the platform too early we
will get an invalid response which we won't cache, and since each call to
`GetProcessPlatformViaDYLDSPI` is slow, handling the load commands is getting
increasnigly slower when there are many of them.
See #140610
>From cb185b834b942790b0e7b8a167f538dbd7738bd9 Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Wed, 21 May 2025 09:12:47 +0300
Subject: [PATCH] [lldb] Don't query the platform for each load command
This change remove the call to GetPlatform from GetDeploymentInfo and instead
pass it as an argument, so GetPlatform will be called only once for all the
load commands.
The reason is that if we try to query the platform too early we wlil get an
invalid response which we won't cache, and since each call to
`GetProcessPlatformViaDYLDSPI` is slow, handling the load commands is getting
increasnigly slower when there are many of twhem.
See #140610
---
lldb/tools/debugserver/source/DNB.cpp | 13 ++---
lldb/tools/debugserver/source/DNB.h | 19 ++-
.../debugserver/source/MacOSX/MachProcess.h | 3 ++-
.../debugserver/source/MacOSX/MachProcess.mm | 14 +++---
lldb/tools/debugserver/source/RNBRemote.cpp | 7 ---
5 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/lldb/tools/debugserver/source/DNB.cpp
b/lldb/tools/debugserver/source/DNB.cpp
index f541134b43a1b..b59218456741f 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1498,7 +1498,7 @@ nub_bool_t DNBProcessSharedLibrariesUpdated(nub_process_t
pid) {
std::optional
DNBGetDeploymentInfo(nub_process_t pid, bool is_executable,
- const struct load_command &lc,
+ uint32_t dyld_platform, const struct load_command &lc,
uint64_t load_command_address, uint32_t &major_version,
uint32_t &minor_version, uint32_t &patch_version) {
MachProcessSP procSP;
@@ -1507,8 +1507,8 @@ DNBGetDeploymentInfo(nub_process_t pid, bool
is_executable,
// macOS binary) is loaded with the macCatalyst dyld platform
// override. The image info corrects for this, but qProcessInfo
// will return what is in the binary.
-auto info =
-procSP->GetDeploymentInfo(lc, load_command_address, is_executable);
+auto info = procSP->GetDeploymentInfo(dyld_platform, lc,
+ load_command_address, is_executable);
major_version = info.major_version;
minor_version = info.minor_version;
patch_version = info.patch_version;
@@ -1521,6 +1521,13 @@ DNBGetDeploymentInfo(nub_process_t pid, bool
is_executable,
return {};
}
+uint32_t DNBGetPlatform(nub_process_t pid) {
+ MachProcessSP procSP;
+ if (GetProcessSP(pid, procSP))
+return procSP->GetPlatform();
+ return 0;
+}
+
// Get the current shared library information for a process. Only return
// the shared libraries that have changed since the last shared library
// state changed event if only_changed is non-zero.
diff --git a/lldb/tools/debugserver/source/DNB.h
b/lldb/tools/debugserver/source/DNB.h
index 10d1f68794355..5475e691c7a93 100644
--- a/lldb/tools/debugserver/source/DNB.h
+++ b/lldb/tools/debugserver/source/DNB.h
@@ -52,15 +52,15 @@ nub_process_t DNBProcessLaunch(
nub_process_t DNBProcessGetPIDByName(const char *name);
nub_process_t DNBProcessAttach(nub_process_t pid, struct timespec *timeout,
- const RNBContext::IgnoredExceptions
- &ignored_exceptions,
- char *err_str,
- size_t err_len);
+ const RNBContext::IgnoredExceptions
+&ignored_exceptions,
+ char *err_str,
+size_t err_len);
nub_process_t DNBProcessAttachByName(const char *name, struct timespec
*timeout,
- const RNBContext::IgnoredExceptions
- &ignored_exceptions,
- char *err_str,
- size_t err_len);
+ const RNBContext::IgnoredExceptions
+&ignored_exceptions,
+ char *err_str,
+size_t err_len);
nub_process_t DNBProcessAttachWait(RNBContext *ctx, const char *wait_name,
bool ignore_existing,
struct timespec *timeout,
@@ -137,9 +137,10 @@ DNBProcessGetSharedLibraryInfo(nub_process_t pid,
nub_bool_t only_changed,
DNBExecutableImageInfo **image_infos)
DNB_EXPORT;
std::optional
DNBGetDeploymentInfo(nub_process_t pid,
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform #140610 (PR #140611)
https://github.com/talkeren created
https://github.com/llvm/llvm-project/pull/140611
If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
want to call it everytime as it won't change the result. So use
std::optional to cache wether we already calculated the value.
>From 788597d46b7b785e6807cb4b8ab75bdc05bd675a Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Mon, 19 May 2025 23:15:10 +0300
Subject: [PATCH] [lldb] Properly cache the result of MachProcess::GetPlatform
#140610
If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
want to call it everytime as it won't change the result. So use std::optional
to cache wether we already calculated the value.
---
lldb/tools/debugserver/source/MacOSX/MachProcess.h | 2 +-
lldb/tools/debugserver/source/MacOSX/MachProcess.mm | 12 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 56bc9d6c7461e..516a77c0bd6c8 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -398,7 +398,7 @@ class MachProcess {
pid_t m_pid; // Process ID of child process
cpu_type_t m_cpu_type; // The CPU type of this process
- uint32_t m_platform; // The platform of this process
+ std::optional m_platform; // The platform of this process
int m_child_stdin;
int m_child_stdout;
int m_child_stderr;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3afaaa2f64c00..9e777320bfbcb 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -915,8 +915,8 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
}
}
}
-
-load_cmds_p += lc.cmdsize;
+
+ load_cmds_p += lc.cmdsize;
}
return true;
}
@@ -1039,9 +1039,9 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
};
uint32_t MachProcess::GetPlatform() {
- if (m_platform == 0)
+ if (!m_platform)
m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
- return m_platform;
+ return *m_platform;
}
uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
@@ -1323,7 +1323,7 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
// Clear any cached thread list while the pid and task are still valid
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Now clear out all member variables
m_pid = INVALID_NUB_PROCESS;
if (!detaching)
@@ -1754,7 +1754,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit,
uint32_t lsbit) {
// NULL our task out as we have already restored all exception ports
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Clear out any notion of the process we once were
const bool detaching = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform #140610 (PR #140611)
https://github.com/talkeren updated
https://github.com/llvm/llvm-project/pull/140611
>From 7ccef56ba9d82fae7901bc39ef648b7be6bba473 Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Mon, 19 May 2025 23:15:10 +0300
Subject: [PATCH] [lldb] Properly cache the result of MachProcess::GetPlatform
#140610
If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
want to call it everytime as it won't change the result. So use std::optional
to cache wether we already calculated the value.
---
lldb/tools/debugserver/source/MacOSX/MachProcess.h | 2 +-
lldb/tools/debugserver/source/MacOSX/MachProcess.mm | 12 +++-
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 56bc9d6c7461e..516a77c0bd6c8 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -398,7 +398,7 @@ class MachProcess {
pid_t m_pid; // Process ID of child process
cpu_type_t m_cpu_type; // The CPU type of this process
- uint32_t m_platform; // The platform of this process
+ std::optional m_platform; // The platform of this process
int m_child_stdin;
int m_child_stdout;
int m_child_stderr;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3afaaa2f64c00..ce74dd1cadfb0 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1039,9 +1039,9 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
};
uint32_t MachProcess::GetPlatform() {
- if (m_platform == 0)
+ if (!m_platform)
m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
- return m_platform;
+ return *m_platform;
}
uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
@@ -1058,7 +1058,9 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
}
return platform;
}
-
+If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
+want to call it everytime as it won't change the result. So use
+std::optional to cache wether we already calculated the value.
void MachProcess::GetAllLoadedBinariesViaDYLDSPI(
std::vector &image_infos) {
kern_return_t kern_ret;
@@ -1323,7 +1325,7 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
// Clear any cached thread list while the pid and task are still valid
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Now clear out all member variables
m_pid = INVALID_NUB_PROCESS;
if (!detaching)
@@ -1754,7 +1756,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit,
uint32_t lsbit) {
// NULL our task out as we have already restored all exception ports
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Clear out any notion of the process we once were
const bool detaching = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform #140610 (PR #140611)
https://github.com/talkeren updated
https://github.com/llvm/llvm-project/pull/140611
>From c385e9780e980865461f877c76acaa417b4d736c Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Mon, 19 May 2025 23:15:10 +0300
Subject: [PATCH] [lldb] Properly cache the result of MachProcess::GetPlatform
#140610
If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
want to call it everytime as it won't change the result. So use std::optional
to cache wether we already calculated the value.
---
lldb/tools/debugserver/source/MacOSX/MachProcess.h | 2 +-
lldb/tools/debugserver/source/MacOSX/MachProcess.mm | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 56bc9d6c7461e..516a77c0bd6c8 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -398,7 +398,7 @@ class MachProcess {
pid_t m_pid; // Process ID of child process
cpu_type_t m_cpu_type; // The CPU type of this process
- uint32_t m_platform; // The platform of this process
+ std::optional m_platform; // The platform of this process
int m_child_stdin;
int m_child_stdout;
int m_child_stderr;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3afaaa2f64c00..7f8b0efff28b3 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1039,9 +1039,9 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
};
uint32_t MachProcess::GetPlatform() {
- if (m_platform == 0)
+ if (!m_platform)
m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
- return m_platform;
+ return *m_platform;
}
uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
@@ -1323,7 +1323,7 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
// Clear any cached thread list while the pid and task are still valid
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Now clear out all member variables
m_pid = INVALID_NUB_PROCESS;
if (!detaching)
@@ -1754,7 +1754,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit,
uint32_t lsbit) {
// NULL our task out as we have already restored all exception ports
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Clear out any notion of the process we once were
const bool detaching = true;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform (PR #140611)
https://github.com/talkeren edited https://github.com/llvm/llvm-project/pull/140611 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform (PR #140611)
https://github.com/talkeren edited https://github.com/llvm/llvm-project/pull/140611 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Properly cache the result of MachProcess::GetPlatform (PR #140611)
https://github.com/talkeren updated
https://github.com/llvm/llvm-project/pull/140611
>From c385e9780e980865461f877c76acaa417b4d736c Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Mon, 19 May 2025 23:15:10 +0300
Subject: [PATCH 1/2] [lldb] Properly cache the result of
MachProcess::GetPlatform #140610
If `MachProcess::GetProcessPlatformViaDYLDSPI` fails and return 0, we don't
want to call it everytime as it won't change the result. So use std::optional
to cache wether we already calculated the value.
---
lldb/tools/debugserver/source/MacOSX/MachProcess.h | 2 +-
lldb/tools/debugserver/source/MacOSX/MachProcess.mm | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 56bc9d6c7461e..516a77c0bd6c8 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -398,7 +398,7 @@ class MachProcess {
pid_t m_pid; // Process ID of child process
cpu_type_t m_cpu_type; // The CPU type of this process
- uint32_t m_platform; // The platform of this process
+ std::optional m_platform; // The platform of this process
int m_child_stdin;
int m_child_stdout;
int m_child_stderr;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 3afaaa2f64c00..7f8b0efff28b3 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -1039,9 +1039,9 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
};
uint32_t MachProcess::GetPlatform() {
- if (m_platform == 0)
+ if (!m_platform)
m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
- return m_platform;
+ return *m_platform;
}
uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
@@ -1323,7 +1323,7 @@ static bool mach_header_validity_test(uint32_t magic,
uint32_t cputype) {
// Clear any cached thread list while the pid and task are still valid
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Now clear out all member variables
m_pid = INVALID_NUB_PROCESS;
if (!detaching)
@@ -1754,7 +1754,7 @@ static uint64_t bits(uint64_t value, uint32_t msbit,
uint32_t lsbit) {
// NULL our task out as we have already restored all exception ports
m_task.Clear();
- m_platform = 0;
+ m_platform.reset();
// Clear out any notion of the process we once were
const bool detaching = true;
>From bc775be48deda868a1df7dbe94a2c697dcb69739 Mon Sep 17 00:00:00 2001
From: Tal Keren
Date: Mon, 19 May 2025 23:35:15 +0300
Subject: [PATCH 2/2] fix formatting
---
lldb/tools/debugserver/source/MacOSX/MachProcess.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
index 516a77c0bd6c8..948fd5318465c 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
@@ -398,7 +398,7 @@ class MachProcess {
pid_t m_pid; // Process ID of child process
cpu_type_t m_cpu_type; // The CPU type of this process
- std::optional m_platform; // The platform of this process
+ std::optional m_platform; // The platform of this process
int m_child_stdin;
int m_child_stdout;
int m_child_stderr;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
