[Lldb-commits] [lldb] 91d5bfd - Add "indexedVariables" to variables with lots of children.
Greg Clayton via lldb-commits
lldb-commits at lists.llvm.org
Wed May 11 16:18:05 PDT 2022
Author: Greg Clayton
Date: 2022-05-11T16:17:57-07:00
New Revision: 91d5bfdb7996b353097c886128fc984e310f7122
URL: https://github.com/llvm/llvm-project/commit/91d5bfdb7996b353097c886128fc984e310f7122
DIFF: https://github.com/llvm/llvm-project/commit/91d5bfdb7996b353097c886128fc984e310f7122.diff
LOG: Add "indexedVariables" to variables with lots of children.
Prior to this fix if we have a really large array or collection class, we would end up always creating all of the child variables for an array or collection class. If the number of children was very high this can cause delays when expanding variables. By adding the "indexedVariables" to variables with lots of children, we can keep good performance in the variables view at all times. This patch will add the "indexedVariables" key/value pair to any "Variable" JSON dictionairies when we have an array of synthetic child provider that will create more than 100 children.
We have to be careful to not call "uint32_t SBValue::GetNumChildren()" on any lldb::SBValue that we use because it can cause a class, struct or union to complete the type in order to be able to properly tell us how many children it has and this can be expensive if you have a lot of variables. By default LLDB won't need to complete a type if we have variables that are classes, structs or unions unless the user expands the variable in the variable view. So we try to only get the GetNumChildren() when we have an array, as this is a cheap operation, or a synthetic child provider, most of which are for showing collections that typically fall into this category. We add a variable reference, which indicates that something can be expanded, when the function "bool SBValue::MightHaveChildren()" is true as this call doesn't need to complete the type in order to return true. This way if no one ever expands class variables, we don't need to complete the type.
Differential Revision: https://reviews.llvm.org/D125347
Added:
Modified:
lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
lldb/test/API/tools/lldb-vscode/variables/main.cpp
lldb/tools/lldb-vscode/JSONUtils.cpp
Removed:
################################################################################
diff --git a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
index 9a4a5de40e718..2a473df67c78a 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -30,8 +30,8 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
verify_value = verify[key]
actual_value = actual[key]
self.assertEqual(verify_value, actual_value,
- '"%s" keys don\'t match (%s != %s)' % (
- key, actual_value, verify_value))
+ '"%s" keys don\'t match (%s != %s) from:\n%s' % (
+ key, actual_value, verify_value, actual))
if 'startswith' in verify_dict:
verify = verify_dict['startswith']
for key in verify:
@@ -43,6 +43,11 @@ def verify_values(self, verify_dict, actual, varref_dict=None, expression=None):
' "%s")') % (
key, actual_value,
verify_value))
+ if 'missing' in verify_dict:
+ missing = verify_dict['missing']
+ for key in missing:
+ self.assertTrue(key not in actual,
+ 'key "%s" is not expected in %s' % (key, actual))
hasVariablesReference = 'variablesReference' in actual
varRef = None
if hasVariablesReference:
@@ -310,22 +315,39 @@ def test_scopes_and_evaluate_expansion(self):
locals = self.vscode.get_local_variables()
buffer_children = make_buffer_verify_dict(0, 32)
verify_locals = {
- "argc": {"equals": {"type": "int", "value": "1"}},
+ "argc": {
+ "equals": {"type": "int", "value": "1"},
+ "missing": ["indexedVariables"],
+ },
"argv": {
"equals": {"type": "const char **"},
"startswith": {"value": "0x"},
"hasVariablesReference": True,
+ "missing": ["indexedVariables"],
},
"pt": {
"equals": {"type": "PointType"},
"hasVariablesReference": True,
+ "missing": ["indexedVariables"],
"children": {
- "x": {"equals": {"type": "int", "value": "11"}},
- "y": {"equals": {"type": "int", "value": "22"}},
- "buffer": {"children": buffer_children},
+ "x": {
+ "equals": {"type": "int", "value": "11"},
+ "missing": ["indexedVariables"],
+ },
+ "y": {
+ "equals": {"type": "int", "value": "22"},
+ "missing": ["indexedVariables"],
+ },
+ "buffer": {
+ "children": buffer_children,
+ "equals": {"indexedVariables": 32}
+ },
},
},
- "x": {"equals": {"type": "int"}},
+ "x": {
+ "equals": {"type": "int"},
+ "missing": ["indexedVariables"],
+ },
}
self.verify_variables(verify_locals, locals)
@@ -336,6 +358,7 @@ def test_scopes_and_evaluate_expansion(self):
"response": {
"equals": {"type": "PointType"},
"startswith": {"result": "PointType @ 0x"},
+ "missing": ["indexedVariables"],
"hasVariablesReference": True,
},
"children": {
@@ -408,3 +431,36 @@ def test_scopes_and_evaluate_expansion(self):
self.assertEquals(scope.get("presentationHint"), "locals")
if scope["name"] == "Registers":
self.assertEquals(scope.get("presentationHint"), "registers")
+
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_indexedVariables(self):
+ """
+ Tests that arrays and lldb.SBValue objects that have synthetic child
+ providers have "indexedVariables" key/value pairs. This helps the IDE
+ not to fetch too many children all at once.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ breakpoint1_line = line_number(source, "// breakpoint 4")
+ lines = [breakpoint1_line]
+ # Set breakpoint in the thread function so we can step the threads
+ breakpoint_ids = self.set_source_breakpoints(source, lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+
+ # Verify locals
+ locals = self.vscode.get_local_variables()
+ buffer_children = make_buffer_verify_dict(0, 32)
+ verify_locals = {
+ "small_array": {"equals": {"indexedVariables": 5}},
+ "large_array": {"equals": {"indexedVariables": 200}},
+ "small_vector": {"equals": {"indexedVariables": 5}},
+ "large_vector": {"equals": {"indexedVariables": 200}},
+ "pt": {"missing": ["indexedVariables"]},
+ }
+ self.verify_variables(verify_locals, locals)
diff --git a/lldb/test/API/tools/lldb-vscode/variables/main.cpp b/lldb/test/API/tools/lldb-vscode/variables/main.cpp
index 6ec5df906ba7c..d81a9a20544a8 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/main.cpp
+++ b/lldb/test/API/tools/lldb-vscode/variables/main.cpp
@@ -5,10 +5,10 @@ struct PointType {
int y;
int buffer[BUFFER_SIZE];
};
-
+#include <vector>
int g_global = 123;
static int s_global = 234;
-
+int test_indexedVariables();
int main(int argc, char const *argv[]) {
static float s_local = 2.25;
PointType pt = { 11,22, {0}};
@@ -22,5 +22,15 @@ int main(int argc, char const *argv[]) {
s_global = x; // breakpoint 2
}
}
- return 0; // breakpoint 3
+ return test_indexedVariables(); // breakpoint 3
+}
+
+int test_indexedVariables() {
+ int small_array[5] = {1, 2, 3, 4, 5};
+ int large_array[200];
+ std::vector<int> small_vector;
+ std::vector<int> large_vector;
+ small_vector.assign(5, 0);
+ large_vector.assign(200, 0);
+ return 0; // breakpoint 4
}
diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp
index 58a08796e738d..55d734e65fb5d 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -9,6 +9,7 @@
#include <algorithm>
#include <iomanip>
#include <sstream>
+#include <string.h>
#include "llvm/ADT/Optional.h"
#include "llvm/Support/FormatAdapters.h"
@@ -1024,7 +1025,37 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
if (format_hex)
v.SetFormat(lldb::eFormatHex);
SetValueForKey(v, object, "value");
- auto type_cstr = v.GetType().GetDisplayTypeName();
+ auto type_obj = v.GetType();
+ auto type_cstr = type_obj.GetDisplayTypeName();
+ // If we have a type with many many children, we would like to be able to
+ // give a hint to the IDE that the type has indexed children so that the
+ // request can be broken up in grabbing only a few children at a time. We want
+ // to be careful and only call "v.GetNumChildren()" if we have an array type
+ // or if we have a synthetic child provider. We don't want to call
+ // "v.GetNumChildren()" on all objects as class, struct and union types don't
+ // need to be completed if they are never expanded. So we want to avoid
+ // calling this to only cases where we it makes sense to keep performance high
+ // during normal debugging.
+
+ // If we have an array type, say that it is indexed and provide the number of
+ // children in case we have a huge array. If we don't do this, then we might
+ // take a while to produce all children at onces which can delay your debug
+ // session.
+ const bool is_array = type_obj.IsArrayType();
+ const bool is_synthetic = v.IsSynthetic();
+ if (is_array || is_synthetic) {
+ const auto num_children = v.GetNumChildren();
+ if (is_array) {
+ object.try_emplace("indexedVariables", num_children);
+ } else {
+ // If a type has a synthetic child provider, then the SBType of "v" won't
+ // tell us anything about what might be displayed. So we can check if the
+ // first child's name is "[0]" and then we can say it is indexed.
+ const char *first_child_name = v.GetChildAtIndex(0).GetName();
+ if (first_child_name && strcmp(first_child_name, "[0]") == 0)
+ object.try_emplace("indexedVariables", num_children);
+ }
+ }
EmplaceSafeString(object, "type", type_cstr ? type_cstr : NO_TYPENAME);
if (varID != INT64_MAX)
object.try_emplace("id", varID);
More information about the lldb-commits
mailing list