[Lldb-commits] [lldb] [lldb] Avoid (unlimited) GetNumChildren calls when printing values (PR #93946)

via lldb-commits lldb-commits at lists.llvm.org
Fri May 31 03:24:06 PDT 2024


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Pavel Labath (labath)

<details>
<summary>Changes</summary>

For some data formatters, even getting the number of children can be an expensive operations (e.g., needing to walk a linked list to determine the number of elements). This is then wasted work when we know we will be printing only small number of them.

This patch replaces the calls to GetNumChildren (at least those on the "frame var" path) with the calls to the capped version, passing the value of `max-children-count` setting (plus one)

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


4 Files Affected:

- (modified) lldb/source/DataFormatters/FormatManager.cpp (+7-3) 
- (modified) lldb/source/DataFormatters/ValueObjectPrinter.cpp (+14-13) 
- (modified) lldb/test/API/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py (+11) 
- (modified) lldb/test/API/functionalities/data-formatter/synthcapping/fooSynthProvider.py (+15-1) 


``````````diff
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index d7ba5b4b70c94..84c0c7eed1431 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -9,6 +9,7 @@
 #include "lldb/DataFormatters/FormatManager.h"
 
 #include "lldb/Core/Debugger.h"
+#include "lldb/Core/ValueObject.h"
 #include "lldb/DataFormatters/FormattersHelpers.h"
 #include "lldb/DataFormatters/LanguageCategory.h"
 #include "lldb/Interpreter/ScriptInterpreter.h"
@@ -442,16 +443,19 @@ lldb::Format FormatManager::GetSingleItemFormat(lldb::Format vector_format) {
 }
 
 bool FormatManager::ShouldPrintAsOneLiner(ValueObject &valobj) {
+  TargetSP target_sp = valobj.GetTargetSP();
   // if settings say no oneline whatsoever
-  if (valobj.GetTargetSP().get() &&
-      !valobj.GetTargetSP()->GetDebugger().GetAutoOneLineSummaries())
+  if (target_sp && !target_sp->GetDebugger().GetAutoOneLineSummaries())
     return false; // then don't oneline
 
   // if this object has a summary, then ask the summary
   if (valobj.GetSummaryFormat().get() != nullptr)
     return valobj.GetSummaryFormat()->IsOneLiner();
 
-  auto num_children = valobj.GetNumChildren();
+  size_t max_num_children =
+      (target_sp ? *target_sp : Target::GetGlobalProperties())
+          .GetMaximumNumberOfChildrenToDisplay();
+  auto num_children = valobj.GetNumChildren(max_num_children);
   if (!num_children) {
     llvm::consumeError(num_children.takeError());
     return true;
diff --git a/lldb/source/DataFormatters/ValueObjectPrinter.cpp b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
index bbdc2a9981570..c2933d8574583 100644
--- a/lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ b/lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -14,6 +14,8 @@
 #include "lldb/Target/Language.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
+#include "llvm/Support/MathExtras.h"
+#include <cstdint>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -628,22 +630,21 @@ ValueObjectPrinter::GetMaxNumChildrenToPrint(bool &print_dotdotdot) {
   if (m_options.m_pointer_as_array)
     return m_options.m_pointer_as_array.m_element_count;
 
-  auto num_children_or_err = synth_valobj.GetNumChildren();
+  const uint32_t max_num_children =
+      m_options.m_ignore_cap ? UINT32_MAX
+                             : GetMostSpecializedValue()
+                                   .GetTargetSP()
+                                   ->GetMaximumNumberOfChildrenToDisplay();
+  // Ask for one more child than the maximum to see if we should print "...".
+  auto num_children_or_err = synth_valobj.GetNumChildren(
+      llvm::SaturatingAdd(max_num_children, uint32_t(1)));
   if (!num_children_or_err)
     return num_children_or_err;
-  uint32_t num_children = *num_children_or_err;
-  print_dotdotdot = false;
-  if (num_children) {
-    const size_t max_num_children = GetMostSpecializedValue()
-                                        .GetTargetSP()
-                                        ->GetMaximumNumberOfChildrenToDisplay();
-
-    if (num_children > max_num_children && !m_options.m_ignore_cap) {
-      print_dotdotdot = true;
-      return max_num_children;
-    }
+  if (*num_children_or_err > max_num_children) {
+    print_dotdotdot = true;
+    return max_num_children;
   }
-  return num_children;
+  return num_children_or_err;
 }
 
 void ValueObjectPrinter::PrintChildrenPostamble(bool print_dotdotdot) {
diff --git a/lldb/test/API/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py b/lldb/test/API/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py
index d53dadef836e5..9ca232abefa03 100644
--- a/lldb/test/API/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py
+++ b/lldb/test/API/functionalities/data-formatter/synthcapping/TestSyntheticCapping.py
@@ -68,6 +68,11 @@ def cleanup():
                 "r = 34",
             ],
         )
+        # num_children() should be called with at most max_num_children=257
+        # (target.max-children-count + 1)
+        self.expect(
+            "script fooSynthProvider.reset_max_num_children_max()", substrs=["257"]
+        )
 
         # check that capping works
         self.runCmd("settings set target.max-children-count 2", check=False)
@@ -80,9 +85,15 @@ def cleanup():
                 "...",
             ],
         )
+        self.expect(
+            "script fooSynthProvider.reset_max_num_children_max()", substrs=["3"]
+        )
 
         self.expect("frame variable f00_1", matching=False, substrs=["r = 34"])
 
         self.runCmd("settings set target.max-children-count 256", check=False)
 
         self.expect("frame variable f00_1", matching=True, substrs=["r = 34"])
+        self.expect(
+            "script fooSynthProvider.reset_max_num_children_max()", substrs=["257"]
+        )
diff --git a/lldb/test/API/functionalities/data-formatter/synthcapping/fooSynthProvider.py b/lldb/test/API/functionalities/data-formatter/synthcapping/fooSynthProvider.py
index 3bfa3130cc01c..3778ad9790880 100644
--- a/lldb/test/API/functionalities/data-formatter/synthcapping/fooSynthProvider.py
+++ b/lldb/test/API/functionalities/data-formatter/synthcapping/fooSynthProvider.py
@@ -2,11 +2,25 @@
 
 
 class fooSynthProvider:
+
+    # For testing purposes, we'll keep track of the maximum value of
+    # max_num_children we've been called with.
+    MAX_NUM_CHILDREN_MAX = 0
+
+    @classmethod
+    def reset_max_num_children_max(cls):
+        old_value = fooSynthProvider.MAX_NUM_CHILDREN_MAX
+        fooSynthProvider.MAX_NUM_CHILDREN_MAX = 0
+        return old_value
+
     def __init__(self, valobj, dict):
         self.valobj = valobj
         self.int_type = valobj.GetType().GetBasicType(lldb.eBasicTypeInt)
 
-    def num_children(self):
+    def num_children(self, max_num_children):
+        fooSynthProvider.MAX_NUM_CHILDREN_MAX = max(
+            fooSynthProvider.MAX_NUM_CHILDREN_MAX, max_num_children
+        )
         return 3
 
     def get_child_at_index(self, index):

``````````

</details>


https://github.com/llvm/llvm-project/pull/93946


More information about the lldb-commits mailing list