[lldb] Fix performance regression after adding GNUstep ObjC runtime

[sgraenitz]

Summary

We added support for the GNUstep ObjC runtime in 0b6264738f3d. In order to check if the target process uses
GNUstep we run an expensive symbol lookup in CreateInstance(). This turned out to cause a heavy performance
regression for non-GNUstep inferiors.

This patch puts a cheaper check in front, so that the vast majority of requests should return early. This
should fix the symptom for the moment. The conceptual question is why LanguageRuntime::FindPlugin invokes
create_callback for each available runtime unconditionally in every Process::ModulesDidLoad.

[sgraenitz]

The issue was reported in https://github.com/llvm/llvm-project/issues/64582

[jasonmolenda]

Looks good!

[theraven]

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

49

This is a bit unfortunate. I know some downstream users that link the Objective-C runtime components into another .so, so we can't really rely on the name. It would be nice if there were some mechanism for the user to specify the name of the runtime if they're using something non-standard.

[jasonmolenda]

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

49

If the runtime was merged in to a library with a known name, we could search for a name in the runtime (if it were the Darwin runtime, objc_msgSend would be a good candidate) in the list of libraries that might have the runtime in them. Doing a "search for a symbol name in any binary that is added" is expensive, doing "search these three solibs for a symbol if they're loaded" is much less expensive, doing "is a solib with this name loaded" is free.

[bulbazord]

This looks fine to me. Did you have the chance to verify that it improves performance of non-objc inferiors on Linux? We'll also probably want to make sure this gets back ported into llvm-17.

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

49

That would be a nice follow-up!

[jingham]

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

49

If we can't know up front what library contains the runtime, we could also add a setting for the library name, so we don't have to guess.

[sgraenitz]

Thanks for your quick feedback everyone!

Did you have the chance to verify that it improves performance of non-objc inferiors on Linux?

Nope, but the author of the bug report confirmed that this fixes the issue for them. Since I will be on vacation starting tomorrow, I propose to land this as is.

We'll also probably want to make sure this gets back ported into llvm-17.

Yes, let me check.

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

49

Thanks for the note. Sounds reasonable. However, this is out of scope for the regression fix.

[sgraenitz]

Backport for 17.x release: https://github.com/llvm/llvm-project/issues/64800

lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp

PluginManager::RegisterPlugin(
       GetPluginNameStatic(), "GNUstep Objective-C Language Runtime - libobjc2",
       CreateInstance);
 }
 
 void GNUstepObjCRuntime::Terminate() {
   PluginManager::UnregisterPlugin(CreateInstance);
 }
 
+static bool CanModuleBeGNUstepObjCLibrary(const ModuleSP &module_sp,
+                                          const llvm::Triple &TT) {
+  if (!module_sp)
+    return false;
+  const FileSpec &module_file_spec = module_sp->GetFileSpec();
+  if (!module_file_spec)
+    return false;
+  llvm::StringRef filename = module_file_spec.GetFilename().GetStringRef();
+  if (TT.isOSBinFormatELF())
+    return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())
+    return filename == "objc.dll";
+  return false;
+}
+
+static bool ScanForGNUstepObjCLibraryCandidate(const ModuleList &modules,
+                                               const llvm::Triple &TT) {
+  std::lock_guard<std::recursive_mutex> guard(modules.GetMutex());
+  size_t num_modules = modules.GetSize();
+  for (size_t i = 0; i < num_modules; i++) {
+    auto mod = modules.GetModuleAtIndex(i);
+    if (CanModuleBeGNUstepObjCLibrary(mod, TT))
+      return true;
+  }
+  return false;
+}
+
 LanguageRuntime *GNUstepObjCRuntime::CreateInstance(Process *process,
                                                     LanguageType language) {
   if (language != eLanguageTypeObjC)
     return nullptr;
   if (!process)
     return nullptr;
 
   Target &target = process->GetTarget();
   const llvm::Triple &TT = target.GetArchitecture().GetTriple();
   if (TT.getVendor() == llvm::Triple::VendorType::Apple)
     return nullptr;
 
   const ModuleList &images = target.GetImages();
+  if (!ScanForGNUstepObjCLibraryCandidate(images, TT))
+    return nullptr;
+
   if (TT.isOSBinFormatELF()) {
     SymbolContextList eh_pers;
     RegularExpression regex("__gnustep_objc[x]*_personality_v[0-9]+");
     images.FindSymbolsMatchingRegExAndType(regex, eSymbolTypeCode, eh_pers);
     if (eh_pers.GetSize() == 0)
       return nullptr;
   } else if (TT.isOSWindows()) {
     SymbolContextList objc_mandatory;
GNUstepObjCRuntime::GetStepThroughTrampolinePlan(Thread &thread,
   return nullptr;
 }
 
 void GNUstepObjCRuntime::UpdateISAToDescriptorMapIfNeeded() {
   // TODO: Support lazily named and dynamically loaded Objective-C classes
 }
 
 bool GNUstepObjCRuntime::IsModuleObjCLibrary(const ModuleSP &module_sp) {
-  if (!module_sp)
-    return false;
-  const FileSpec &module_file_spec = module_sp->GetFileSpec();
-  if (!module_file_spec)
-    return false;
-  llvm::StringRef filename = module_file_spec.GetFilename().GetStringRef();
   const llvm::Triple &TT = GetTargetRef().GetArchitecture().GetTriple();
-  if (TT.isOSBinFormatELF())
-    return filename.starts_with("libobjc.so");
-  if (TT.isOSWindows())
-    return filename == "objc.dll";
-  return false;
+  return CanModuleBeGNUstepObjCLibrary(module_sp, TT);
 }
 
 bool GNUstepObjCRuntime::ReadObjCLibrary(const ModuleSP &module_sp) {
   assert(m_objc_module_sp == nullptr && "Check HasReadObjCLibrary() first");
   m_objc_module_sp = module_sp;
 
   // Right now we don't use this, but we might want to check for debugger
   // runtime support symbols like 'gdb_object_getClass' in the future.
   return true;
 }
 
 void GNUstepObjCRuntime::ModulesDidLoad(const ModuleList &module_list) {
   ReadObjCLibraryIfNeeded(module_list);
 }