[Lldb-commits] [lldb] 39239f9 - [lldb-vscode] improve modules request
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 21 13:19:04 PST 2021
Author: Walter Erquinigo
Date: 2021-01-21T13:18:50-08:00
New Revision: 39239f9b5666bebb059fa562badeffb9f1c3afab
URL: https://github.com/llvm/llvm-project/commit/39239f9b5666bebb059fa562badeffb9f1c3afab
DIFF: https://github.com/llvm/llvm-project/commit/39239f9b5666bebb059fa562badeffb9f1c3afab.diff
LOG: [lldb-vscode] improve modules request
lldb-vsdode was communicating the list of modules to the IDE with events, which in practice ended up having some drawbacks
- when debugging large targets, the number of these events were easily 10k, which polluted the messages being transmitted, which caused the following: a harder time debugging the messages, a lag after terminated the process because of these messages being processes (this could easily take several seconds). The latter was specially bad, as users were complaining about it even when they didn't check the modules view.
- these events were rarely used, as users only check the modules view when something is wrong and they try to debug things.
After getting some feedback from users, we realized that it's better to not used events but make this simply a request and is triggered by users whenever they needed.
This diff achieves that and does some small clean up in the existing code.
Differential Revision: https://reviews.llvm.org/D94033
Added:
Modified:
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
lldb/tools/lldb-vscode/VSCode.cpp
lldb/tools/lldb-vscode/lldb-vscode.cpp
Removed:
################################################################################
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 70f29cdd3d75..0962e3612929 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,7 +113,6 @@ def __init__(self, recv, send, init_commands):
self.initialize_body = None
self.thread_stop_reasons = {}
self.breakpoint_events = []
- self.module_events = {}
self.sequence = 1
self.threads = None
self.recv_thread.start()
@@ -134,8 +133,12 @@ def validate_response(cls, command, response):
if command['seq'] != response['request_seq']:
raise ValueError('seq mismatch in response')
- def get_active_modules(self):
- return self.module_events
+ def get_modules(self):
+ module_list = self.request_modules()['body']['modules']
+ modules = {}
+ for module in module_list:
+ modules[module['name']] = module
+ return modules
def get_output(self, category, timeout=0.0, clear=True):
self.output_condition.acquire()
@@ -222,14 +225,6 @@ def handle_recv_packet(self, packet):
self.breakpoint_events.append(packet)
# no need to add 'breakpoint' event packets to our packets list
return keepGoing
- elif event == 'module':
- reason = body['reason']
- if (reason == 'new' or reason == 'changed'):
- self.module_events[body['module']['name']] = body['module']
- elif reason == 'removed':
- if body['module']['name'] in self.module_events:
- self.module_events.pop(body['module']['name'])
- return keepGoing
elif packet_type == 'response':
if packet['command'] == 'disconnect':
@@ -782,10 +777,10 @@ def request_setFunctionBreakpoints(self, names, condition=None,
}
return self.send_recv(command_dict)
- def request_getCompileUnits(self, moduleId):
+ def request_compileUnits(self, moduleId):
args_dict = {'moduleId': moduleId}
command_dict = {
- 'command': 'getCompileUnits',
+ 'command': 'compileUnits',
'type': 'request',
'arguments': args_dict
}
@@ -804,6 +799,12 @@ def request_completions(self, text):
}
return self.send_recv(command_dict)
+ def request_modules(self):
+ return self.send_recv({
+ 'command': 'modules',
+ 'type': 'request'
+ })
+
def request_stackTrace(self, threadId=None, startFrame=None, levels=None,
dump=False):
if threadId is None:
diff --git a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
index 7fa5f7d45267..40ac7c4e9d19 100644
--- a/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ b/lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -24,7 +24,7 @@ def run_test(self, symbol_basename, expect_debug_info_size):
breakpoint_ids = self.set_function_breakpoints(functions)
self.assertEquals(len(breakpoint_ids), len(functions), 'expect one breakpoint')
self.continue_to_breakpoints(breakpoint_ids)
- active_modules = self.vscode.get_active_modules()
+ active_modules = self.vscode.get_modules()
program_module = active_modules[program_basename]
self.assertIn(program_basename, active_modules, '%s module is in active modules' % (program_basename))
self.assertIn('name', program_module, 'make sure name is in module')
@@ -36,7 +36,7 @@ def run_test(self, symbol_basename, expect_debug_info_size):
self.vscode.request_evaluate('`%s' % ('target symbols add -s "%s" "%s"' % (program, symbols_path)))
def checkSymbolsLoadedWithSize():
- active_modules = self.vscode.get_active_modules()
+ active_modules = self.vscode.get_modules()
program_module = active_modules[program_basename]
self.assertIn('symbolFilePath', program_module)
self.assertIn(symbols_path, program_module['symbolFilePath'])
@@ -45,17 +45,15 @@ def checkSymbolsLoadedWithSize():
if expect_debug_info_size:
self.waitUntil(checkSymbolsLoadedWithSize)
- active_modules = self.vscode.get_active_modules()
+ active_modules = self.vscode.get_modules()
program_module = active_modules[program_basename]
self.assertEqual(program_basename, program_module['name'])
self.assertEqual(program, program_module['path'])
self.assertIn('addressRange', program_module)
@skipIfWindows
- @skipUnlessDarwin
@skipIfRemote
- #TODO: Update the Makefile so that this test runs on Linux
- def test_module_event(self):
+ def test_modules(self):
'''
Mac or linux.
@@ -64,27 +62,21 @@ def test_module_event(self):
sections are in .o files.
On other platforms, we expect a.out to have debug info, so we will expect a size.
- expect_debug_info_size = platform.system() != 'Darwin'
- return self.run_test("a.out", expect_debug_info_size)
'''
- expect_debug_info_size = platform.system() != 'Darwin'
- return self.run_test("a.out", expect_debug_info_size)
+ return self.run_test("a.out", expect_debug_info_size=platform.system() != 'Darwin')
- @skipIfWindows
@skipUnlessDarwin
@skipIfRemote
- def test_module_event_dsym(self):
+ def test_modules_dsym(self):
'''
Darwin only test with dSYM file.
On mac, if we load a.out.dSYM as our symbol file, we will have debug symbols and we
will have DWARF sections added to the module, so we will expect a size.
- return self.run_test("a.out.dSYM", True)
'''
- return self.run_test("a.out.dSYM", True)
+ return self.run_test("a.out.dSYM", expect_debug_info_size=True)
@skipIfWindows
- @skipUnlessDarwin
@skipIfRemote
def test_compile_units(self):
program = self.getBuildArtifact("a.out")
@@ -95,11 +87,10 @@ def test_compile_units(self):
lines = [breakpoint1_line]
breakpoint_ids = self.set_source_breakpoints(source, lines)
self.continue_to_breakpoints(breakpoint_ids)
- moduleId = self.vscode.get_active_modules()['a.out']['id']
- response = self.vscode.request_getCompileUnits(moduleId)
+ moduleId = self.vscode.get_modules()['a.out']['id']
+ response = self.vscode.request_compileUnits(moduleId)
self.assertTrue(response['body'])
self.assertTrue(len(response['body']['compileUnits']) == 1,
'Only one source file should exist')
self.assertTrue(response['body']['compileUnits'][0]['compileUnitPath'] == main_source_path,
'Real path to main.cpp matches')
-
diff --git a/lldb/tools/lldb-vscode/VSCode.cpp b/lldb/tools/lldb-vscode/VSCode.cpp
index 9450cdf3132a..e9fdc17f4147 100644
--- a/lldb/tools/lldb-vscode/VSCode.cpp
+++ b/lldb/tools/lldb-vscode/VSCode.cpp
@@ -355,11 +355,6 @@ void VSCode::SetTarget(const lldb::SBTarget target) {
lldb::SBTarget::eBroadcastBitBreakpointChanged);
listener.StartListeningForEvents(this->broadcaster,
eBroadcastBitStopEventThread);
- listener.StartListeningForEvents(
- this->target.GetBroadcaster(),
- lldb::SBTarget::eBroadcastBitModulesLoaded |
- lldb::SBTarget::eBroadcastBitModulesUnloaded |
- lldb::SBTarget::eBroadcastBitSymbolsLoaded);
}
}
diff --git a/lldb/tools/lldb-vscode/lldb-vscode.cpp b/lldb/tools/lldb-vscode/lldb-vscode.cpp
index b64829423e30..dcb5cfeef9c7 100644
--- a/lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ b/lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -440,30 +440,6 @@ void EventThreadFunction() {
g_vsc.SendJSON(llvm::json::Value(std::move(bp_event)));
}
}
- } else if (lldb::SBTarget::EventIsTargetEvent(event)) {
- if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded ||
- event_mask & lldb::SBTarget::eBroadcastBitModulesUnloaded ||
- event_mask & lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
- int num_modules = lldb::SBTarget::GetNumModulesFromEvent(event);
- for (int i = 0; i < num_modules; i++) {
- auto module = lldb::SBTarget::GetModuleAtIndexFromEvent(i, event);
- auto module_event = CreateEventObject("module");
- llvm::json::Value module_value = CreateModule(module);
- llvm::json::Object body;
- if (event_mask & lldb::SBTarget::eBroadcastBitModulesLoaded) {
- body.try_emplace("reason", "new");
- } else if (event_mask &
- lldb::SBTarget::eBroadcastBitModulesUnloaded) {
- body.try_emplace("reason", "removed");
- } else if (event_mask &
- lldb::SBTarget::eBroadcastBitSymbolsLoaded) {
- body.try_emplace("reason", "changed");
- }
- body.try_emplace("module", module_value);
- module_event.try_emplace("body", std::move(body));
- g_vsc.SendJSON(llvm::json::Value(std::move(module_event)));
- }
- }
} else if (event.BroadcasterMatchesRef(g_vsc.broadcaster)) {
if (event_mask & eBroadcastBitStopEventThread) {
done = true;
@@ -1196,26 +1172,26 @@ void request_evaluate(const llvm::json::Object &request) {
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
-// "getCompileUnitsRequest": {
+// "compileUnitsRequest": {
// "allOf": [ { "$ref": "#/definitions/Request" }, {
// "type": "object",
// "description": "Compile Unit request; value of command field is
-// 'getCompileUnits'.",
+// 'compileUnits'.",
// "properties": {
// "command": {
// "type": "string",
-// "enum": [ "getCompileUnits" ]
+// "enum": [ "compileUnits" ]
// },
// "arguments": {
-// "$ref": "#/definitions/getCompileUnitRequestArguments"
+// "$ref": "#/definitions/compileUnitRequestArguments"
// }
// },
// "required": [ "command", "arguments" ]
// }]
// },
-// "getCompileUnitsRequestArguments": {
+// "compileUnitsRequestArguments": {
// "type": "object",
-// "description": "Arguments for 'getCompileUnits' request.",
+// "description": "Arguments for 'compileUnits' request.",
// "properties": {
// "moduleId": {
// "type": "string",
@@ -1224,23 +1200,21 @@ void request_evaluate(const llvm::json::Object &request) {
// },
// "required": [ "moduleId" ]
// },
-// "getCompileUnitsResponse": {
+// "compileUnitsResponse": {
// "allOf": [ { "$ref": "#/definitions/Response" }, {
// "type": "object",
-// "description": "Response to 'getCompileUnits' request.",
+// "description": "Response to 'compileUnits' request.",
// "properties": {
// "body": {
-// "description": "Response to 'getCompileUnits' request. Array of
+// "description": "Response to 'compileUnits' request. Array of
// paths of compile units."
// }
// }
// }]
// }
-
-void request_getCompileUnits(const llvm::json::Object &request) {
+void request_compileUnits(const llvm::json::Object &request) {
llvm::json::Object response;
FillResponse(request, response);
- lldb::SBProcess process = g_vsc.target.GetProcess();
llvm::json::Object body;
llvm::json::Array units;
auto arguments = request.getObject("arguments");
@@ -1262,6 +1236,48 @@ void request_getCompileUnits(const llvm::json::Object &request) {
g_vsc.SendJSON(llvm::json::Value(std::move(response)));
}
+// "modulesRequest": {
+// "allOf": [ { "$ref": "#/definitions/Request" }, {
+// "type": "object",
+// "description": "Modules request; value of command field is
+// 'modules'.",
+// "properties": {
+// "command": {
+// "type": "string",
+// "enum": [ "modules" ]
+// },
+// },
+// "required": [ "command" ]
+// }]
+// },
+// "modulesResponse": {
+// "allOf": [ { "$ref": "#/definitions/Response" }, {
+// "type": "object",
+// "description": "Response to 'modules' request.",
+// "properties": {
+// "body": {
+// "description": "Response to 'modules' request. Array of
+// module objects."
+// }
+// }
+// }]
+// }
+void request_modules(const llvm::json::Object &request) {
+ llvm::json::Object response;
+ FillResponse(request, response);
+
+ llvm::json::Array modules;
+ for (size_t i = 0; i < g_vsc.target.GetNumModules(); i++) {
+ lldb::SBModule module = g_vsc.target.GetModuleAtIndex(i);
+ modules.emplace_back(CreateModule(module));
+ }
+
+ llvm::json::Object body;
+ body.try_emplace("modules", std::move(modules));
+ response.try_emplace("body", std::move(body));
+ g_vsc.SendJSON(llvm::json::Value(std::move(response)));
+}
+
// "InitializeRequest": {
// "allOf": [ { "$ref": "#/definitions/Request" }, {
// "type": "object",
@@ -2901,7 +2917,6 @@ void RegisterRequestCallbacks() {
g_vsc.RegisterRequestCallback("disconnect", request_disconnect);
g_vsc.RegisterRequestCallback("evaluate", request_evaluate);
g_vsc.RegisterRequestCallback("exceptionInfo", request_exceptionInfo);
- g_vsc.RegisterRequestCallback("getCompileUnits", request_getCompileUnits);
g_vsc.RegisterRequestCallback("initialize", request_initialize);
g_vsc.RegisterRequestCallback("launch", request_launch);
g_vsc.RegisterRequestCallback("next", request_next);
@@ -2919,6 +2934,9 @@ void RegisterRequestCallbacks() {
g_vsc.RegisterRequestCallback("stepOut", request_stepOut);
g_vsc.RegisterRequestCallback("threads", request_threads);
g_vsc.RegisterRequestCallback("variables", request_variables);
+ // Custom requests
+ g_vsc.RegisterRequestCallback("compileUnits", request_compileUnits);
+ g_vsc.RegisterRequestCallback("modules", request_modules);
// Testing requests
g_vsc.RegisterRequestCallback("_testGetTargetBreakpoints",
request__testGetTargetBreakpoints);
More information about the lldb-commits
mailing list