wallace created this revision.
wallace added reviewers: clayborg, kusmour, aadsm.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94033
Files:
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
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===================================================================
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -440,30 +440,6 @@
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 @@
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 @@
// },
// "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 @@
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 @@
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 @@
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);
Index: lldb/tools/lldb-vscode/VSCode.cpp
===================================================================
--- lldb/tools/lldb-vscode/VSCode.cpp
+++ lldb/tools/lldb-vscode/VSCode.cpp
@@ -355,11 +355,6 @@
lldb::SBTarget::eBroadcastBitBreakpointChanged);
listener.StartListeningForEvents(this->broadcaster,
eBroadcastBitStopEventThread);
- listener.StartListeningForEvents(
- this->target.GetBroadcaster(),
- lldb::SBTarget::eBroadcastBitModulesLoaded |
- lldb::SBTarget::eBroadcastBitModulesUnloaded |
- lldb::SBTarget::eBroadcastBitSymbolsLoaded);
}
}
Index: lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
===================================================================
--- lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
+++ lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
@@ -24,7 +24,7 @@
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 @@
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 @@
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 @@
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 @@
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')
-
Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -113,7 +113,6 @@
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 @@
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 @@
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 @@
}
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 @@
}
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:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
