Skip to content

control Darwin parallel image loading with target.parallel-module-load #134437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 7, 2025

Conversation

zhyty
Copy link
Contributor

@zhyty zhyty commented Apr 4, 2025

A requested follow-up from #130912 by @JDevlieghere to control Darwin parallel image loading with the same target.parallel-module-load that controls the POSIX dyld parallel image loading. Darwin parallel image loading was introduced by #110646.

This small change:

  • removes plugin.dynamic-loader.darwin.experimental.enable-parallel-image-load and associated code.
  • changes setting call site in DynamicLoaderDarwin::PreloadModulesFromImageInfos to use the new setting.

Tested by running ninja check-lldb and loading some targets.

@zhyty zhyty requested a review from JDevlieghere as a code owner April 4, 2025 19:07
@llvmbot llvmbot added the lldb label Apr 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 4, 2025

@llvm/pr-subscribers-lldb

Author: Tom Yang (zhyty)

Changes

A requested follow-up from #130912 by @JDevlieghere to control Darwin parallel image loading with the same target.parallel-module-load that controls the POSIX dyld parallel image loading.

This small change:

  • removes plugin.dynamic-loader.darwin.experimental.enable-parallel-image-load and associated code.
  • changes setting call site in DynamicLoaderDarwin::PreloadModulesFromImageInfos to use the new setting.

Tested by running ninja check-lldb and loading some targets.


Full diff: https://github.com/llvm/llvm-project/pull/134437.diff

8 Files Affected:

  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt (-13)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp (+1-14)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h (-2)
  • (removed) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.cpp (-53)
  • (removed) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.h (-34)
  • (removed) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td (-8)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp (+1-7)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h (-2)
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
index 77a560541fcb1..7308374c8bfba 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/CMakeLists.txt
@@ -1,16 +1,7 @@
-lldb_tablegen(DynamicLoaderDarwinProperties.inc -gen-lldb-property-defs
-  SOURCE DynamicLoaderDarwinProperties.td
-  TARGET LLDBPluginDynamicLoaderDarwinPropertiesGen)
-
-lldb_tablegen(DynamicLoaderDarwinPropertiesEnum.inc -gen-lldb-property-enum-defs
-  SOURCE DynamicLoaderDarwinProperties.td
-  TARGET LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
-
 add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
   DynamicLoaderMacOSXDYLD.cpp
   DynamicLoaderMacOS.cpp
   DynamicLoaderDarwin.cpp
-  DynamicLoaderDarwinProperties.cpp
 
   LINK_LIBS
     lldbBreakpoint
@@ -25,7 +16,3 @@ add_lldb_library(lldbPluginDynamicLoaderMacOSXDYLD PLUGIN
     Support
     TargetParser
   )
-
-add_dependencies(lldbPluginDynamicLoaderMacOSXDYLD
-  LLDBPluginDynamicLoaderDarwinPropertiesGen
-  LLDBPluginDynamicLoaderDarwinPropertiesEnumGen)
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index f9b49c50355d5..e25c4ff55e408 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -8,7 +8,6 @@
 
 #include "DynamicLoaderDarwin.h"
 
-#include "DynamicLoaderDarwinProperties.h"
 #include "lldb/Breakpoint/StoppointCallbackContext.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/Module.h"
@@ -79,17 +78,6 @@ void DynamicLoaderDarwin::DidLaunch() {
   SetNotificationBreakpoint();
 }
 
-void DynamicLoaderDarwin::CreateSettings(lldb_private::Debugger &debugger) {
-  if (!PluginManager::GetSettingForDynamicLoaderPlugin(
-          debugger, DynamicLoaderDarwinProperties::GetSettingName())) {
-    const bool is_global_setting = true;
-    PluginManager::CreateSettingForDynamicLoaderPlugin(
-        debugger,
-        DynamicLoaderDarwinProperties::GetGlobal().GetValueProperties(),
-        "Properties for the DynamicLoaderDarwin plug-in.", is_global_setting);
-  }
-}
-
 // Clear out the state of this class.
 void DynamicLoaderDarwin::Clear(bool clear_process) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
@@ -670,8 +658,7 @@ DynamicLoaderDarwin::PreloadModulesFromImageInfos(
         image_info, FindTargetModuleForImageInfo(image_info, true, nullptr));
   };
   auto it = image_infos.begin();
-  bool is_parallel_load =
-      DynamicLoaderDarwinProperties::GetGlobal().GetEnableParallelImageLoad();
+  bool is_parallel_load = m_process->GetTarget().GetParallelModuleLoad();
   if (is_parallel_load) {
     llvm::ThreadPoolTaskGroup taskGroup(Debugger::GetThreadPool());
     for (size_t i = 0; i < size; ++i, ++it) {
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index bc5464d76b950..37528b88b615e 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -58,8 +58,6 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader {
 
   std::optional<lldb_private::Address> GetStartAddress() override;
 
-  static void CreateSettings(lldb_private::Debugger &debugger);
-
 protected:
   void PrivateInitialize(lldb_private::Process *process);
 
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.cpp
deleted file mode 100644
index f4d8a071e6d5d..0000000000000
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.cpp
+++ /dev/null
@@ -1,53 +0,0 @@
-//===-- DynamicLoaderDarwinProperties.cpp ---------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#include "DynamicLoaderDarwinProperties.h"
-
-using namespace lldb_private;
-
-#define LLDB_PROPERTIES_dynamicloaderdarwin_experimental
-#include "DynamicLoaderDarwinProperties.inc"
-
-enum {
-#define LLDB_PROPERTIES_dynamicloaderdarwin_experimental
-#include "DynamicLoaderDarwinPropertiesEnum.inc"
-};
-
-llvm::StringRef DynamicLoaderDarwinProperties::GetSettingName() {
-  static constexpr llvm::StringLiteral g_setting_name("darwin");
-  return g_setting_name;
-}
-
-DynamicLoaderDarwinProperties::ExperimentalProperties::ExperimentalProperties()
-    : Properties(std::make_shared<OptionValueProperties>(
-          GetExperimentalSettingsName())) {
-  m_collection_sp->Initialize(g_dynamicloaderdarwin_experimental_properties);
-}
-
-DynamicLoaderDarwinProperties::DynamicLoaderDarwinProperties()
-    : Properties(std::make_shared<OptionValueProperties>(GetSettingName())),
-      m_experimental_properties(std::make_unique<ExperimentalProperties>()) {
-  m_collection_sp->AppendProperty(
-      Properties::GetExperimentalSettingsName(),
-      "Experimental settings - setting these won't produce errors if the "
-      "setting is not present.",
-      true, m_experimental_properties->GetValueProperties());
-}
-
-bool DynamicLoaderDarwinProperties::GetEnableParallelImageLoad() const {
-  return m_experimental_properties->GetPropertyAtIndexAs<bool>(
-      ePropertyEnableParallelImageLoad,
-      g_dynamicloaderdarwin_experimental_properties
-              [ePropertyEnableParallelImageLoad]
-                  .default_uint_value != 0);
-}
-
-DynamicLoaderDarwinProperties &DynamicLoaderDarwinProperties::GetGlobal() {
-  static DynamicLoaderDarwinProperties g_settings;
-  return g_settings;
-}
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.h
deleted file mode 100644
index 4c5e800c4f3e4..0000000000000
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.h
+++ /dev/null
@@ -1,34 +0,0 @@
-//===-- DynamicLoaderDarwinProperties.h -------------------------*- C++ -*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLDB_SOURCE_PLUGINS_DYNAMICLOADER_MACOSX_DYLD_DYNAMICLOADERDARWINPROPERTIES_H
-#define LLDB_SOURCE_PLUGINS_DYNAMICLOADER_MACOSX_DYLD_DYNAMICLOADERDARWINPROPERTIES_H
-
-#include "lldb/Core/UserSettingsController.h"
-
-namespace lldb_private {
-
-class DynamicLoaderDarwinProperties : public Properties {
-public:
-  class ExperimentalProperties : public Properties {
-  public:
-    ExperimentalProperties();
-  };
-  static llvm::StringRef GetSettingName();
-  static DynamicLoaderDarwinProperties &GetGlobal();
-  DynamicLoaderDarwinProperties();
-  ~DynamicLoaderDarwinProperties() override = default;
-  bool GetEnableParallelImageLoad() const;
-
-private:
-  std::unique_ptr<ExperimentalProperties> m_experimental_properties;
-};
-
-} // namespace lldb_private
-
-#endif // LLDB_SOURCE_PLUGINS_DYNAMICLOADER_MACOSX_DYLD_DYNAMICLOADERDARWINPROPERTIES_H
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td
deleted file mode 100644
index c54580ce34729..0000000000000
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwinProperties.td
+++ /dev/null
@@ -1,8 +0,0 @@
-include "../../../../include/lldb/Core/PropertiesBase.td"
-
-let Definition = "dynamicloaderdarwin_experimental" in {
-  def EnableParallelImageLoad: Property<"enable-parallel-image-load", "Boolean">,
-    Global,
-    DefaultTrue,
-    Desc<"Load images in parallel.">;
-}
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
index b05ed1ce2c823..f839948660aa0 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.cpp
@@ -1149,8 +1149,7 @@ bool DynamicLoaderMacOSXDYLD::IsFullyInitialized() {
 
 void DynamicLoaderMacOSXDYLD::Initialize() {
   PluginManager::RegisterPlugin(GetPluginNameStatic(),
-                                GetPluginDescriptionStatic(), CreateInstance,
-                                DebuggerInitialize);
+                                GetPluginDescriptionStatic(), CreateInstance);
   DynamicLoaderMacOS::Initialize();
 }
 
@@ -1159,11 +1158,6 @@ void DynamicLoaderMacOSXDYLD::Terminate() {
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
-void DynamicLoaderMacOSXDYLD::DebuggerInitialize(
-    lldb_private::Debugger &debugger) {
-  CreateSettings(debugger);
-}
-
 llvm::StringRef DynamicLoaderMacOSXDYLD::GetPluginDescriptionStatic() {
   return "Dynamic loader plug-in that watches for shared library loads/unloads "
          "in MacOSX user processes.";
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
index 924e2fc107743..ae7451722a8d7 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderMacOSXDYLD.h
@@ -50,8 +50,6 @@ class DynamicLoaderMacOSXDYLD : public lldb_private::DynamicLoaderDarwin {
   static lldb_private::DynamicLoader *
   CreateInstance(lldb_private::Process *process, bool force);
 
-  static void DebuggerInitialize(lldb_private::Debugger &debugger);
-
   /// Called after attaching a process.
   ///
   /// Allow DynamicLoader plug-ins to execute some code after

@JDevlieghere
Copy link
Member

Thank you!

@royitaqi royitaqi merged commit 65813e0 into llvm:main Apr 7, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants