[Lldb-commits] [PATCH] D158205: [lldb] Fix performance regression after adding GNUstep ObjC runtime

Fri Aug 18 04:54:28 PDT 2023

sgraenitz added a comment.

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.

================
Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/GNUstepObjCRuntime/GNUstepObjCRuntime.cpp:49
+  if (TT.isOSBinFormatELF())
+    return filename.starts_with("libobjc.so");
+  if (TT.isOSWindows())
----------------
jingham wrote:
> bulbazord wrote:
> > jasonmolenda wrote:
> > > theraven wrote:
> > > > 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.
> > > 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.
> > That would be a nice follow-up!
> 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.
Thanks for the note. Sounds reasonable. However, this is out of scope for the regression fix.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158205/new/

https://reviews.llvm.org/D158205