diff --git a/lldb/bindings/interface/SBProgressDocstrings.i b/lldb/bindings/interface/SBProgressDocstrings.i index 5459d1af5155c..7b7e1dc79187c 100644 --- a/lldb/bindings/interface/SBProgressDocstrings.i +++ b/lldb/bindings/interface/SBProgressDocstrings.i @@ -11,7 +11,48 @@ The Progress class helps make sure that progress is correctly reported and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one -to be sent.") lldb::SBProgress; +to be sent. + +Progress can either be deterministic, incrementing up to a known total or non-deterministic +with an unbounded total. Deterministic is better if you know the items of work in advance, but non-deterministic +exposes a way to update a user during a long running process that work is taking place. + +For all progresses the details provided in the constructor will be sent until an increment detail +is provided. This detail will also continue to be broadcasted on any subsequent update that doesn't +specify a new detail. Some implementations differ on throttling updates and this behavior differs primarily +if the progress is deterministic or non-deterministic. For DAP, non-deterministic update messages have a higher +throttling rate than deterministic ones. + +Below are examples in Python for deterministic and non-deterministic progresses. + + deterministic_progress1 = lldb.SBProgress('Deterministic Progress', 'Detail', 3, lldb.SBDebugger) + for i in range(3): + deterministic_progress1.Increment(1, f'Update {i}') + # The call to Finalize() is a no-op as we already incremented the right amount of + # times and caused the end event to be sent. + deterministic_progress1.Finalize() + + deterministic_progress2 = lldb.SBProgress('Deterministic Progress', 'Detail', 10, lldb.SBDebugger) + for i in range(3): + deterministic_progress2.Increment(1, f'Update {i}') + # Cause the progress end event to be sent even if we didn't increment the right + # number of times. Real world examples would be in a try-finally block to ensure + # progress clean-up. + deterministic_progress2.Finalize() + +If you don't call Finalize() when the progress is not done, the progress object will eventually get +garbage collected by the Python runtime, the end event will eventually get sent, but it is best not to +rely on the garbage collection when using lldb.SBProgress. + +Non-deterministic progresses behave the same, but omit the total in the constructor. + + non_deterministic_progress = lldb.SBProgress('Non deterministic progress, 'Detail', lldb.SBDebugger) + for i in range(10): + non_deterministic_progress.Increment(1) + # Explicitly send a progressEnd, otherwise this will be sent + # when the python runtime cleans up this object. + non_deterministic_progress.Finalize() +") lldb::SBProgress; %feature("docstring", "Finalize the SBProgress, which will cause a progress end event to be emitted. This diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py index 7f4055cab9ddd..e94a09676e067 100644 --- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py +++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py @@ -37,7 +37,11 @@ def create_options(cls): ) parser.add_option( - "--total", dest="total", help="Total to count up.", type="int" + "--total", + dest="total", + help="Total items in this progress object. When this option is not specified, this will be an indeterminate progress.", + type="int", + default=None, ) parser.add_option( @@ -47,6 +51,14 @@ def create_options(cls): type="int", ) + parser.add_option( + "--no-details", + dest="no_details", + help="Do not display details", + action="store_true", + default=False, + ) + return parser def get_short_help(self): @@ -68,12 +80,30 @@ def __call__(self, debugger, command, exe_ctx, result): return total = cmd_options.total - progress = lldb.SBProgress("Progress tester", "Detail", total, debugger) + if total is None: + progress = lldb.SBProgress( + "Progress tester", "Initial Indeterminate Detail", debugger + ) + else: + progress = lldb.SBProgress( + "Progress tester", "Initial Detail", total, debugger + ) + + # Check to see if total is set to None to indicate an indeterminate progress + # then default to 10 steps. + if total is None: + total = 10 for i in range(1, total): - progress.Increment(1, f"Step {i}") + if cmd_options.no_details: + progress.Increment(1) + else: + progress.Increment(1, f"Step {i}") time.sleep(cmd_options.seconds) + # Not required for deterministic progress, but required for indeterminate progress. + progress.Finalize() + def __lldb_init_module(debugger, dict): # Register all classes that have a register_lldb_command method diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py index 36c0cef9c4714..f723a2d254825 100755 --- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py +++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py @@ -4,6 +4,7 @@ from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * +import json import os import time @@ -11,14 +12,46 @@ class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase): + def verify_progress_events( + self, + expected_title, + expected_message=None, + expected_not_in_message=None, + only_verify_first_update=False, + ): + self.dap_server.wait_for_event("progressEnd", 15) + self.assertTrue(len(self.dap_server.progress_events) > 0) + start_found = False + update_found = False + end_found = False + for event in self.dap_server.progress_events: + event_type = event["event"] + if "progressStart" in event_type: + title = event["body"]["title"] + self.assertIn(expected_title, title) + start_found = True + if "progressUpdate" in event_type: + message = event["body"]["message"] + if only_verify_first_update and update_found: + continue + if expected_message is not None: + self.assertIn(expected_message, message) + if expected_not_in_message is not None: + self.assertNotIn(expected_not_in_message, message) + update_found = True + if "progressEnd" in event_type: + end_found = True + + self.assertTrue(start_found) + self.assertTrue(update_found) + self.assertTrue(end_found) + @skipIfWindows def test_output(self): program = self.getBuildArtifact("a.out") self.build_and_launch(program) progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py") - print(f"Progress emitter path: {progress_emitter}") source = "main.cpp" - # Set breakpoint in the thread function so we can step the threads breakpoint_ids = self.set_source_breakpoints( source, [line_number(source, "// break here")] ) @@ -30,20 +63,73 @@ def test_output(self): "`test-progress --total 3 --seconds 1", context="repl" ) - self.dap_server.wait_for_event("progressEnd", 15) - # Expect at least a start, an update, and end event - # However because the underlying Progress instance is an RAII object and we can't guaruntee - # it's deterministic destruction in the python API, we verify just start and update - # otherwise this test could be flakey. - self.assertTrue(len(self.dap_server.progress_events) > 0) - start_found = False - update_found = False - for event in self.dap_server.progress_events: - event_type = event["event"] - if "progressStart" in event_type: - start_found = True - if "progressUpdate" in event_type: - update_found = True + self.verify_progress_events( + expected_title="Progress tester", + expected_not_in_message="Progress tester", + ) - self.assertTrue(start_found) - self.assertTrue(update_found) + @skipIfWindows + def test_output_nodetails(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py") + source = "main.cpp" + breakpoint_ids = self.set_source_breakpoints( + source, [line_number(source, "// break here")] + ) + self.continue_to_breakpoints(breakpoint_ids) + self.dap_server.request_evaluate( + f"`command script import {progress_emitter}", context="repl" + ) + self.dap_server.request_evaluate( + "`test-progress --total 3 --seconds 1 --no-details", context="repl" + ) + + self.verify_progress_events( + expected_title="Progress tester", + expected_message="Initial Detail", + ) + + @skipIfWindows + def test_output_indeterminate(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py") + source = "main.cpp" + breakpoint_ids = self.set_source_breakpoints( + source, [line_number(source, "// break here")] + ) + self.continue_to_breakpoints(breakpoint_ids) + self.dap_server.request_evaluate( + f"`command script import {progress_emitter}", context="repl" + ) + self.dap_server.request_evaluate("`test-progress --seconds 1", context="repl") + + self.verify_progress_events( + expected_title="Progress tester: Initial Indeterminate Detail", + expected_message="Step 1", + only_verify_first_update=True, + ) + + @skipIfWindows + def test_output_nodetails_indeterminate(self): + program = self.getBuildArtifact("a.out") + self.build_and_launch(program) + progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py") + source = "main.cpp" + breakpoint_ids = self.set_source_breakpoints( + source, [line_number(source, "// break here")] + ) + self.dap_server.request_evaluate( + f"`command script import {progress_emitter}", context="repl" + ) + + self.dap_server.request_evaluate( + "`test-progress --seconds 1 --no-details", context="repl" + ) + + self.verify_progress_events( + expected_title="Progress tester: Initial Indeterminate Detail", + expected_message="Initial Indeterminate Detail", + only_verify_first_update=True, + ) diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index e9041f3985523..167ea4758992e 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -18,8 +18,32 @@ using namespace lldb; namespace lldb_dap { -static void ProgressEventThreadFunction(DAP &dap) { - llvm::set_thread_name(dap.name + ".progress_handler"); +static std::string GetStringFromStructuredData(lldb::SBStructuredData &data, + const char *key) { + lldb::SBStructuredData keyValue = data.GetValueForKey(key); + if (!keyValue) + return std::string(); + + const size_t length = keyValue.GetStringValue(nullptr, 0); + + if (length == 0) + return std::string(); + + std::string str(length + 1, 0); + keyValue.GetStringValue(&str[0], length + 1); + return str; +} + +static uint64_t GetUintFromStructuredData(lldb::SBStructuredData &data, + const char *key) { + lldb::SBStructuredData keyValue = data.GetValueForKey(key); + + if (!keyValue.IsValid()) + return 0; + return keyValue.GetUnsignedIntegerValue(); +} + +void ProgressEventThreadFunction(DAP &dap) { lldb::SBListener listener("lldb-dap.progress.listener"); dap.debugger.GetBroadcaster().AddListener( listener, lldb::SBDebugger::eBroadcastBitProgress | @@ -35,14 +59,47 @@ static void ProgressEventThreadFunction(DAP &dap) { done = true; } } else { - uint64_t progress_id = 0; - uint64_t completed = 0; - uint64_t total = 0; - bool is_debugger_specific = false; - const char *message = lldb::SBDebugger::GetProgressFromEvent( - event, progress_id, completed, total, is_debugger_specific); - if (message) - dap.SendProgressEvent(progress_id, message, completed, total); + lldb::SBStructuredData data = + lldb::SBDebugger::GetProgressDataFromEvent(event); + + const uint64_t progress_id = + GetUintFromStructuredData(data, "progress_id"); + const uint64_t completed = GetUintFromStructuredData(data, "completed"); + const uint64_t total = GetUintFromStructuredData(data, "total"); + const std::string details = + GetStringFromStructuredData(data, "details"); + + if (completed == 0) { + if (total == UINT64_MAX) { + // This progress is non deterministic and won't get updated until it + // is completed. Send the "message" which will be the combined title + // and detail. The only other progress event for thus + // non-deterministic progress will be the completed event So there + // will be no need to update the detail. + const std::string message = + GetStringFromStructuredData(data, "message"); + dap.SendProgressEvent(progress_id, message.c_str(), completed, + total); + } else { + // This progress is deterministic and will receive updates, + // on the progress creation event VSCode will save the message in + // the create packet and use that as the title, so we send just the + // title in the progressCreate packet followed immediately by a + // detail packet, if there is any detail. + const std::string title = + GetStringFromStructuredData(data, "title"); + dap.SendProgressEvent(progress_id, title.c_str(), completed, total); + if (!details.empty()) + dap.SendProgressEvent(progress_id, details.c_str(), completed, + total); + } + } else { + // This progress event is either the end of the progress dialog, or an + // update with possible detail. The "detail" string we send to VS Code + // will be appended to the progress dialog's initial text from when it + // was created. + dap.SendProgressEvent(progress_id, details.c_str(), completed, total); + } } } }