[lldb] Avoid unnecessary regex check in dwim-print

[kastiglione]

An (unmeasured) improvement to performance of dwim-print when used as po.

This change lifts the check for note_shown to the top of the lambda, to avoid all subsequent work when the hint has already been shown. The main effect is to avoid performing a regex match when the hint is not going to be shown.

This change also constructs the std::regex only once, by making it static.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Dave Lee (kastiglione)

Changes

---

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

1 Files Affected:

• (modified) lldb/source/Commands/CommandObjectDWIMPrint.cpp (+8-8)

diff --git a/lldb/source/Commands/CommandObjectDWIMPrint.cpp b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
index 76bed100dc7291..ae35f6e43f6c62 100644
--- a/lldb/source/Commands/CommandObjectDWIMPrint.cpp
+++ b/lldb/source/Commands/CommandObjectDWIMPrint.cpp
@@ -101,6 +101,10 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
   // Add a hint if object description was requested, but no description
   // function was implemented.
   auto maybe_add_hint = [&](llvm::StringRef output) {
+    static bool note_shown = false;
+    if (note_shown)
+      return;
+
     // Identify the default output of object description for Swift and
     // Objective-C
     // "<Name: 0x...>. The regex is:
@@ -110,20 +114,16 @@ void CommandObjectDWIMPrint::DoExecute(StringRef command,
     // - Followed by 5 or more hex digits.
     // - Followed by ">".
     // - End with zero or more whitespace characters.
-    const std::regex swift_class_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
+    static const std::regex class_po_regex("^<\\S+: 0x[[:xdigit:]]{5,}>\\s*$");
 
     if (GetDebugger().GetShowDontUsePoHint() && target_ptr &&
         (language == lldb::eLanguageTypeSwift ||
          language == lldb::eLanguageTypeObjC) &&
-        std::regex_match(output.data(), swift_class_regex)) {
-
-      static bool note_shown = false;
-      if (note_shown)
-        return;
+        std::regex_match(output.data(), class_po_regex)) {
 
       result.GetOutputStream()
-          << "note: object description requested, but type doesn't implement "
-             "a custom object description. Consider using \"p\" instead of "
+          << "note: object description requested, but type doesn't implement a "
+             "custom object description. Consider using \"p\" instead of "
              "\"po\" (this note will only be shown once per debug session).\n";
       note_shown = true;
     }

[augusto2112]

Nice! This code lives upstream though so you'll need to open a patch there too

EDIT: never mind, for some reason I thought this PR was opened downstream 😅

[kastiglione]

I will cherry-pick this downstream to swiftlang/llvm-project too.

[JDevlieghere]

I realize this code was already there, but this isn't great. Can we store this at the debugger level, for example? I'd expect this to show up once per debugger, and again after having called SBDebugger::Terminate/Initialize.

[adrian-prantl]

If we tie it to a module, we could use the mechanism introduced in https://github.com/llvm/llvm-project/pull/112801/files to automatically show it once per session.

I'd expect this to show up once per debugger

You mean once per type and debugger, right?

[kastiglione]

I think @augusto2112 and I discussed and agreed not to do once per type. We figured that might make the hint too noisy.

[kastiglione]

I am indifferent about per-debugger. I don't consider this note as being critical, as long as it's shown once in a while, I'm good.

[adrian-prantl]

If we already landed on only once per session I'm fine with that too.
You can also manually allocate a std::once and pass it to the Debugger-wide function. Or just do the call_once here.

[augusto2112]

Yes the idea was for it not to show up too frequently, I'd be ok if it's once per debugger too.

[kastiglione]

I don't see how call_once would work here. This lambda might be called once, or it might be called multiple times (even indefinitely).

[kastiglione]

@adrian-prantl @JDevlieghere I'm going to merge this, since this change improves performance and doesn't introduce the logic being discussed. I'm happy to do a follow up, but it's still unclear to me what exactly that looks like.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.