[Lldb-commits] [lldb] [lldb-vscode] Make descriptive summaries and raw child for synthetics configurable (PR #65687)
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Thu Sep 7 15:38:11 PDT 2023
https://github.com/walter-erquinigo created https://github.com/llvm/llvm-project/pull/65687:
"descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false.
Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to true.
Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.
>From 5b1250c20f34c0236e98f055af2547e5bdba0366 Mon Sep 17 00:00:00 2001
From: walter erquinigo <walter at modular.com>
Date: Thu, 7 Sep 2023 18:35:10 -0400
Subject: [PATCH] [lldb-vscode] Make descriptive summaries and raw child for
synthetics configurable
"descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false.
Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to true.
Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.
---
.../tools/lldb-vscode/lldbvscode_testcase.py | 15 +++-
.../test/tools/lldb-vscode/vscode.py | 6 +-
.../evaluate/TestVSCode_evaluate.py | 16 ++---
.../variables/TestVSCode_variables.py | 71 +++++++++++++++----
lldb/tools/lldb-vscode/JSONUtils.cpp | 12 +++-
lldb/tools/lldb-vscode/VSCode.cpp | 3 +-
lldb/tools/lldb-vscode/VSCode.h | 2 +
lldb/tools/lldb-vscode/lldb-vscode.cpp | 11 ++-
lldb/tools/lldb-vscode/package.json | 20 ++++++
9 files changed, 126 insertions(+), 30 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
index a6c370ccd2036de..1716920be1ad596 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/lldbvscode_testcase.py
@@ -1,8 +1,9 @@
-from lldbsuite.test.lldbtest import *
import os
-import vscode
import time
+import vscode
+from lldbsuite.test.lldbtest import *
+
class VSCodeTestCaseBase(TestBase):
NO_DEBUG_INFO_TESTCASE = True
@@ -267,7 +268,7 @@ def disassemble(self, threadId=None, frameIndex=None):
if memoryReference not in self.vscode.disassembled_instructions:
self.vscode.request_disassemble(memoryReference=memoryReference)
-
+
return self.vscode.disassembled_instructions[memoryReference]
def attach(
@@ -348,6 +349,8 @@ def launch(
runInTerminal=False,
expectFailure=False,
postRunCommands=None,
+ descriptiveSummaries=False,
+ rawChildForSynthetics=True,
):
"""Sending launch request to vscode"""
@@ -384,6 +387,8 @@ def cleanup():
sourceMap=sourceMap,
runInTerminal=runInTerminal,
postRunCommands=postRunCommands,
+ descriptiveSummaries=descriptiveSummaries,
+ rawChildForSynthetics=rawChildForSynthetics,
)
if expectFailure:
@@ -418,6 +423,8 @@ def build_and_launch(
disconnectAutomatically=True,
postRunCommands=None,
lldbVSCodeEnv=None,
+ descriptiveSummaries=False,
+ rawChildForSynthetics=True,
):
"""Build the default Makefile target, create the VSCode debug adaptor,
and launch the process.
@@ -446,4 +453,6 @@ def build_and_launch(
runInTerminal=runInTerminal,
disconnectAutomatically=disconnectAutomatically,
postRunCommands=postRunCommands,
+ descriptiveSummaries=descriptiveSummaries,
+ rawChildForSynthetics=rawChildForSynthetics,
)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
index 14f0bf0a2d4ed3d..6df3609ae3abbc3 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -648,7 +648,7 @@ def request_disconnect(self, terminateDebuggee=None):
"arguments": args_dict,
}
return self.send_recv(command_dict)
-
+
def request_disassemble(self, memoryReference, offset=-50, instructionCount=200, resolveSymbols=True):
args_dict = {
"memoryReference": memoryReference,
@@ -727,6 +727,8 @@ def request_launch(
sourceMap=None,
runInTerminal=False,
postRunCommands=None,
+ descriptiveSummaries=False,
+ rawChildForSynthetics=False,
):
args_dict = {"program": program}
if args:
@@ -768,6 +770,8 @@ def request_launch(
args_dict["runInTerminal"] = runInTerminal
if postRunCommands:
args_dict["postRunCommands"] = postRunCommands
+ args_dict["descriptiveSummaries"] = descriptiveSummaries
+ args_dict["rawChildForSynthetics"] = rawChildForSynthetics
command_dict = {"command": "launch", "type": "request", "arguments": args_dict}
response = self.send_recv(command_dict)
diff --git a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
index 85bd23d9abfdef7..140079e4b96acba 100644
--- a/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
+++ b/lldb/test/API/tools/lldb-vscode/evaluate/TestVSCode_evaluate.py
@@ -28,13 +28,13 @@ def assertEvaluateFailure(self, expression):
def isExpressionParsedExpected(self):
return self.context != "hover"
- def run_test_evaluate_expressions(self, context=None):
+ def run_test_evaluate_expressions(self, context=None, descriptiveSummaries=False):
"""
Tests the evaluate expression request at different breakpoints
"""
self.context = context
program = self.getBuildArtifact("a.out")
- self.build_and_launch(program)
+ self.build_and_launch(program, descriptiveSummaries=descriptiveSummaries)
source = "main.cpp"
self.set_source_breakpoints(
source,
@@ -55,7 +55,7 @@ def run_test_evaluate_expressions(self, context=None):
self.assertEvaluate("var2", "21")
self.assertEvaluate("static_int", "42")
self.assertEvaluate("non_static_int", "43")
- self.assertEvaluate("struct1", "{foo:15}")
+ self.assertEvaluate("struct1", "{foo:15}" if descriptiveSummaries else "my_struct @ 0x")
self.assertEvaluate("struct1.foo", "15")
self.assertEvaluate("struct2->foo", "16")
@@ -85,7 +85,7 @@ def run_test_evaluate_expressions(self, context=None):
self.assertEvaluate(
"non_static_int", "10"
) # different variable with the same name
- self.assertEvaluate("struct1", "{foo:15}")
+ self.assertEvaluate("struct1", "{foo:15}" if descriptiveSummaries else "my_struct @ 0x")
self.assertEvaluate("struct1.foo", "15")
self.assertEvaluate("struct2->foo", "16")
@@ -146,22 +146,22 @@ def run_test_evaluate_expressions(self, context=None):
@skipIfRemote
def test_generic_evaluate_expressions(self):
# Tests context-less expression evaluations
- self.run_test_evaluate_expressions()
+ self.run_test_evaluate_expressions(descriptiveSummaries=False)
@skipIfWindows
@skipIfRemote
def test_repl_evaluate_expressions(self):
# Tests expression evaluations that are triggered from the Debug Console
- self.run_test_evaluate_expressions("repl")
+ self.run_test_evaluate_expressions("repl", descriptiveSummaries=True)
@skipIfWindows
@skipIfRemote
def test_watch_evaluate_expressions(self):
# Tests expression evaluations that are triggered from a watch expression
- self.run_test_evaluate_expressions("watch")
+ self.run_test_evaluate_expressions("watch", descriptiveSummaries=False)
@skipIfWindows
@skipIfRemote
def test_hover_evaluate_expressions(self):
# Tests expression evaluations that are triggered when hovering on the editor
- self.run_test_evaluate_expressions("hover")
+ self.run_test_evaluate_expressions("hover", descriptiveSummaries=True)
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 41292c71fa459e5..1b2be4aaa3684f1 100644
--- a/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
+++ b/lldb/test/API/tools/lldb-vscode/variables/TestVSCode_variables.py
@@ -127,15 +127,13 @@ def darwin_dwarf_missing_obj(self, initCommands):
varref_dict = {}
self.verify_variables(verify_locals, locals, varref_dict)
- @skipIfWindows
- @skipIfRemote
- def test_scopes_variables_setVariable_evaluate(self):
+ def do_test_scopes_variables_setVariable_evaluate(self, descriptiveSummaries: bool):
"""
Tests the "scopes", "variables", "setVariable", and "evaluate"
packets.
"""
program = self.getBuildArtifact("a.out")
- self.build_and_launch(program)
+ self.build_and_launch(program, descriptiveSummaries=descriptiveSummaries)
source = "main.cpp"
breakpoint1_line = line_number(source, "// breakpoint 1")
lines = [breakpoint1_line]
@@ -219,12 +217,22 @@ def test_scopes_variables_setVariable_evaluate(self):
},
"pt": {
"equals": {"type": "PointType"},
- "startswith": {"result": "{x:11, y:22}"},
+ "startswith": {
+ "result":
+ "{x:11, y:22}"
+ if descriptiveSummaries
+ else "PointType @ 0x"
+ },
"hasVariablesReference": True,
},
"pt.buffer": {
"equals": {"type": "int[32]"},
- "startswith": {"result": "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"},
+ "startswith": {
+ "result":
+ "{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, ...}"
+ if descriptiveSummaries
+ else "int[32] @ 0x"
+ },
"hasVariablesReference": True,
},
"argv": {
@@ -347,13 +355,21 @@ def test_scopes_variables_setVariable_evaluate(self):
@skipIfWindows
@skipIfRemote
- def test_scopes_and_evaluate_expansion(self):
+ def test_scopes_variables_setVariable_evaluate(self):
+ self.do_test_scopes_variables_setVariable_evaluate(descriptiveSummaries=False)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_scopes_variables_setVariable_evaluate_with_descriptive_summaries(self):
+ self.do_test_scopes_variables_setVariable_evaluate(descriptiveSummaries=True)
+
+ def do_test_scopes_and_evaluate_expansion(self, descriptiveSummaries: bool):
"""
Tests the evaluated expression expands successfully after "scopes" packets
and permanent expressions persist.
"""
program = self.getBuildArtifact("a.out")
- self.build_and_launch(program)
+ self.build_and_launch(program, descriptiveSummaries=descriptiveSummaries)
source = "main.cpp"
breakpoint1_line = line_number(source, "// breakpoint 1")
lines = [breakpoint1_line]
@@ -410,7 +426,12 @@ def test_scopes_and_evaluate_expansion(self):
"name": "pt",
"response": {
"equals": {"type": "PointType"},
- "startswith": {"result": "{x:11, y:22}"},
+ "startswith": {
+ "result":
+ "{x:11, y:22}"
+ if descriptiveSummaries
+ else "PointType @ 0x"
+ },
"missing": ["indexedVariables"],
"hasVariablesReference": True,
},
@@ -487,14 +508,22 @@ def test_scopes_and_evaluate_expansion(self):
@skipIfWindows
@skipIfRemote
- def test_indexedVariables(self):
+ def test_scopes_and_evaluate_expansion(self):
+ self.do_test_scopes_and_evaluate_expansion(descriptiveSummaries=False)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_scopes_and_evaluate_expansion_with_descriptive_summaries(self):
+ self.do_test_scopes_and_evaluate_expansion(descriptiveSummaries=True)
+
+ def do_test_indexedVariables(self, rawChildForSynthetics: bool):
"""
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)
+ self.build_and_launch(program, rawChildForSynthetics=rawChildForSynthetics)
source = "main.cpp"
breakpoint1_line = line_number(source, "// breakpoint 4")
lines = [breakpoint1_line]
@@ -507,13 +536,14 @@ def test_indexedVariables(self):
# Verify locals
locals = self.vscode.get_local_variables()
- # The vector variables will have one additional entry from the fake
+ # The vector variables might have one additional entry from the fake
# "[raw]" child.
+ raw_child_count = 1 if rawChildForSynthetics else 0
verify_locals = {
"small_array": {"equals": {"indexedVariables": 5}},
"large_array": {"equals": {"indexedVariables": 200}},
- "small_vector": {"equals": {"indexedVariables": 6}},
- "large_vector": {"equals": {"indexedVariables": 201}},
+ "small_vector": {"equals": {"indexedVariables": 5 + raw_child_count}},
+ "large_vector": {"equals": {"indexedVariables": 200 + raw_child_count}},
"pt": {"missing": ["indexedVariables"]},
}
self.verify_variables(verify_locals, locals)
@@ -526,11 +556,22 @@ def test_indexedVariables(self):
"[2]": {"equals": {"type": "int", "value": "0"}},
"[3]": {"equals": {"type": "int", "value": "0"}},
"[4]": {"equals": {"type": "int", "value": "0"}},
- "[raw]": {"contains": {"type": ["vector"]}},
}
+ if rawChildForSynthetics:
+ verify_children["[raw]"] = {"contains": {"type": ["vector"]}},
+
children = self.vscode.request_variables(locals[2]["variablesReference"])["body"]["variables"]
self.verify_variables(verify_children, children)
+ @skipIfWindows
+ @skipIfRemote
+ def test_indexedVariables(self):
+ self.do_test_indexedVariables(rawChildForSynthetics=False)
+
+ @skipIfWindows
+ @skipIfRemote
+ def test_indexedVariables_with_raw_child_for_synthetics(self):
+ self.do_test_indexedVariables(rawChildForSynthetics=True)
@skipIfWindows
@skipIfRemote
diff --git a/lldb/tools/lldb-vscode/JSONUtils.cpp b/lldb/tools/lldb-vscode/JSONUtils.cpp
index e34448c9b7f2f85..34f878b763394af 100644
--- a/lldb/tools/lldb-vscode/JSONUtils.cpp
+++ b/lldb/tools/lldb-vscode/JSONUtils.cpp
@@ -137,6 +137,12 @@ std::vector<std::string> GetStrings(const llvm::json::Object *obj,
/// glance.
static std::optional<std::string>
GetSyntheticSummaryForContainer(lldb::SBValue &v) {
+ // We gate this feature because it performs GetNumChildren(), which can
+ // cause performance issues because LLDB needs to complete possibly huge
+ // types.
+ if (!g_vsc.show_descriptive_summaries)
+ return std::nullopt;
+
if (v.TypeIsPointerType() || !v.MightHaveChildren())
return std::nullopt;
/// As this operation can be potentially slow, we limit the total time spent
@@ -191,6 +197,9 @@ GetSyntheticSummaryForContainer(lldb::SBValue &v) {
/// Return whether we should dereference an SBValue in order to generate a more
/// meaningful summary string.
static bool ShouldBeDereferencedForSummary(lldb::SBValue &v) {
+ if (!g_vsc.show_descriptive_summaries)
+ return false;
+
if (!v.GetType().IsPointerType() && !v.GetType().IsReferenceType())
return false;
@@ -1137,7 +1146,8 @@ llvm::json::Value CreateVariable(lldb::SBValue v, int64_t variablesReference,
// We create a "[raw]" fake child for each synthetic type, so we have to
// account for it when returning indexed variables. We don't need to do this
// for non-indexed ones.
- int actual_num_children = num_children + (is_synthetic ? 1 : 0);
+ bool has_raw_child = is_synthetic && g_vsc.show_raw_child_for_synthetics;
+ int actual_num_children = num_children + (has_raw_child ? 1 : 0);
if (is_array) {
object.try_emplace("indexedVariables", actual_num_children);
} else if (num_children > 0) {
diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp
index 85e1f4b4ac06e4e..7574a5a9ba0aeb1 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -40,7 +40,8 @@ VSCode::VSCode()
{"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift},
{"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}),
focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false),
- stop_at_entry(false), is_attach(false),
+ stop_at_entry(false), is_attach(false), show_descriptive_summaries(false),
+ show_raw_child_for_synthetics(false),
restarting_process_id(LLDB_INVALID_PROCESS_ID),
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
diff --git a/lldb/tools/lldb-vscode/VSCode.h b/lldb/tools/lldb-vscode/VSCode.h
index 730223046b3c44a..14d730d4c94cf86 100644
--- a/lldb/tools/lldb-vscode/VSCode.h
+++ b/lldb/tools/lldb-vscode/VSCode.h
@@ -167,6 +167,8 @@ struct VSCode {
std::atomic<bool> sent_terminated_event;
bool stop_at_entry;
bool is_attach;
+ bool show_descriptive_summaries;
+ bool show_raw_child_for_synthetics;
// The process event thread normally responds to process exited events by
// shutting down the entire adapter. When we're restarting, we keep the id of
// the old process here so we can detect this case and keep running.
diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index 5480edd74f24f47..56801dc1c382c31 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -647,6 +647,10 @@ void request_attach(const llvm::json::Object &request) {
std::vector<std::string> postRunCommands =
GetStrings(arguments, "postRunCommands");
const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
+ g_vsc.show_descriptive_summaries =
+ GetBoolean(arguments, "descriptiveSummaries", false);
+ g_vsc.show_raw_child_for_synthetics =
+ GetBoolean(arguments, "rawChildForSynthetics", true);
// This is a hack for loading DWARF in .o files on Mac where the .o files
// in the debug map of the main executable have relative paths which require
@@ -1794,6 +1798,10 @@ void request_launch(const llvm::json::Object &request) {
GetStrings(arguments, "postRunCommands");
g_vsc.stop_at_entry = GetBoolean(arguments, "stopOnEntry", false);
const llvm::StringRef debuggerRoot = GetString(arguments, "debuggerRoot");
+ g_vsc.show_descriptive_summaries =
+ GetBoolean(arguments, "descriptiveSummaries", false);
+ g_vsc.show_raw_child_for_synthetics =
+ GetBoolean(arguments, "rawChildForSynthetics", true);
// This is a hack for loading DWARF in .o files on Mac where the .o files
// in the debug map of the main executable have relative paths which require
@@ -3292,7 +3300,8 @@ void request_variables(const llvm::json::Object &request) {
// "[raw]" child that can be used to inspect the raw version of a
// synthetic member. That eliminates the need for the user to go to the
// debug console and type `frame var <variable> to get these values.
- if (variable.IsSynthetic() && i == num_children)
+ if (g_vsc.show_raw_child_for_synthetics && variable.IsSynthetic() &&
+ i == num_children)
addChild(variable.GetNonSyntheticValue(), "[raw]");
}
}
diff --git a/lldb/tools/lldb-vscode/package.json b/lldb/tools/lldb-vscode/package.json
index cc6df03097a70e4..884657b774118aa 100644
--- a/lldb/tools/lldb-vscode/package.json
+++ b/lldb/tools/lldb-vscode/package.json
@@ -240,6 +240,16 @@
"timeout": {
"type": "string",
"description": "The time in seconds to wait for a program to stop at entry point when launching with \"launchCommands\". Defaults to 30 seconds."
+ },
+ "descriptiveSummaries": {
+ "type": "boolean",
+ "description": "Show more descriptive variable summaries, which might incur in worse performance.",
+ "default": false
+ },
+ "rawChildForSynthetics": {
+ "type": "boolean",
+ "description": "Show an additional child for synthetic variables with its raw information.",
+ "default": true
}
}
},
@@ -319,6 +329,16 @@
"timeout": {
"type": "string",
"description": "The time in seconds to wait for a program to stop when attaching using \"attachCommands\". Defaults to 30 seconds."
+ },
+ "descriptiveSummaries": {
+ "type": "boolean",
+ "description": "Show more descriptive variable summaries, which might incur in worse performance. Defaults to false.",
+ "default": false
+ },
+ "rawChildForSynthetics": {
+ "type": "boolean",
+ "description": "Show an additional child for synthetic variables with its raw information. Defaults to true.",
+ "default": true
}
}
}
More information about the lldb-commits
mailing list