[Lldb-commits] [lldb] 8dd1060 - [debugserver] Add platform cache support to improve performance.

[Shafik Yaghmour via lldb-commits]

So I guess we don’t use in class member initialization for m_platform b/c GetPlatform() can never be called before either Clear() or Detach()?

> On May 20, 2021, at 7:10 PM, Jonas Devlieghere via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> 
> Author: kuperxu
> Date: 2021-05-20T19:10:46-07:00
> New Revision: 8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff
> 
> URL: https://github.com/llvm/llvm-project/commit/8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff
> DIFF: https://github.com/llvm/llvm-project/commit/8dd106028b1533f0de03a1ffb4ea0dce40b5a2ff.diff
> 
> LOG: [debugserver] Add platform cache support to improve performance.
> 
> The dyld SPI used by debugserver (_dyld_process_info_create) has become
> much slower in macOS BigSur 11.3 causing a significant performance
> regression when attaching. This commit mitigates that by caching the
> result when calling the SPI to compute the platform.
> 
> Differential revision: https://reviews.llvm.org/D102833
> 
> Added: 
> 
> 
> Modified: 
>    lldb/tools/debugserver/source/MacOSX/MachProcess.h
>    lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.h b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> index b295dfecec41..33c3d628a7a0 100644
> --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.h
> @@ -252,6 +252,7 @@ class MachProcess {
>                                      struct mach_o_information &inf);
>   JSONGenerator::ObjectSP FormatDynamicLibrariesIntoJSON(
>       const std::vector<struct binary_image_information> &image_infos);
> +  uint32_t GetPlatform();
>   /// Get the runtime platform from DYLD via SPI.
>   uint32_t GetProcessPlatformViaDYLDSPI();
>   /// Use the dyld SPI present in macOS 10.12, iOS 10, tvOS 10,
> @@ -378,6 +379,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
>   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 0a6ef6161711..7eab2c6d185f 100644
> --- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> +++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
> @@ -701,7 +701,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
>   // DYLD_FORCE_PLATFORM=6. In that case, force the platform to
>   // macCatalyst and use the macCatalyst version of the host OS
>   // instead of the macOS deployment target.
> -  if (is_executable && GetProcessPlatformViaDYLDSPI() == PLATFORM_MACCATALYST) {
> +  if (is_executable && GetPlatform() == PLATFORM_MACCATALYST) {
>     info.platform = PLATFORM_MACCATALYST;
>     std::string catalyst_version = GetMacCatalystVersionString();
>     const char *major = catalyst_version.c_str();
> @@ -1094,6 +1094,12 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
>   bool privateCache;
> };
> 
> +uint32_t MachProcess::GetPlatform() {
> +  if (m_platform == 0)
> +    m_platform = MachProcess::GetProcessPlatformViaDYLDSPI();
> +  return m_platform;
> +}
> +
> uint32_t MachProcess::GetProcessPlatformViaDYLDSPI() {
>   kern_return_t kern_ret;
>   uint32_t platform = 0;
> @@ -1140,7 +1146,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
>   int pointer_size = GetInferiorAddrSize(pid);
>   std::vector<struct binary_image_information> image_infos;
>   GetAllLoadedBinariesViaDYLDSPI(image_infos);
> -  uint32_t platform = GetProcessPlatformViaDYLDSPI();
> +  uint32_t platform = GetPlatform();
>   const size_t image_count = image_infos.size();
>   for (size_t i = 0; i < image_count; i++) {
>     GetMachOInformationFromMemory(platform, image_infos[i].load_address,
> @@ -1160,7 +1166,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
> 
>   std::vector<struct binary_image_information> all_image_infos;
>   GetAllLoadedBinariesViaDYLDSPI(all_image_infos);
> -  uint32_t platform = GetProcessPlatformViaDYLDSPI();
> +  uint32_t platform = GetPlatform();
> 
>   std::vector<struct binary_image_information> image_infos;
>   const size_t macho_addresses_count = macho_addresses.size();
> @@ -1324,6 +1330,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
>   // Clear any cached thread list while the pid and task are still valid
> 
>   m_task.Clear();
> +  m_platform = 0;
>   // Now clear out all member variables
>   m_pid = INVALID_NUB_PROCESS;
>   if (!detaching)
> @@ -1615,6 +1622,7 @@ static bool FBSAddEventDataToOptions(NSMutableDictionary *options,
> 
>   // NULL our task out as we have already restored all exception ports
>   m_task.Clear();
> +  m_platform = 0;
> 
>   // Clear out any notion of the process we once were
>   const bool detaching = true;
> 
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits