[Lldb-commits] [lldb] [lldb-dap] Refactor handling progress events. (PR #224957)
via lldb-commits
lldb-commits at lists.llvm.org
Sun Sep 20 12:11:52 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Ebuka Ezike (da-viper)
<details>
<summary>Changes</summary>
lldb-dap now only uses one thread to receive and throttle received progress events.
For every progress with a progressId, we delay reporting the progress for 1000ms until we have recieved a new progress and only send an update if the progress has a new version after 250ms.
This means for every progress integration test we do nothing for at least 1250ms. The previous `TestDAP_progress` runs for at least 8s regardless of how fast the computer is.
Moved most of the test to unittest where we can simulate progress delays and time passing. We now have only one `TestDAP_progress` test to verify the client receives progress Events.
When the ProgressEventThread reports a new progressEvent, the reporter then.
- Create or update the `PendingProgress` with that progressId.
- Determines the action to take based on the ProgressState, k_start_delay and k_update_interval.
- Sends the apropriate Event based on the action calculated.
Migrate the API test to the new infrastructure.
Add unit tests for the ProgressEventReporter and protocol progressXXXX POD.
---
Patch is 58.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/224957.diff
11 Files Affected:
- (modified) lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py (+47-86)
- (modified) lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py (+64-80)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+35-150)
- (modified) lldb/tools/lldb-dap/DAP.h (-5)
- (modified) lldb/tools/lldb-dap/ProgressEvent.cpp (+100-209)
- (modified) lldb/tools/lldb-dap/ProgressEvent.h (+92-135)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.cpp (+28)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolEvents.h (+68)
- (modified) lldb/unittests/DAP/CMakeLists.txt (+1)
- (added) lldb/unittests/DAP/ProgressEventTest.cpp (+254)
- (modified) lldb/unittests/DAP/ProtocolEventsTest.cpp (+63)
``````````diff
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 0bf785e3201b0..46098b5d46c4b 100644
--- a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -1,111 +1,72 @@
-import inspect
import optparse
import shlex
-import sys
import time
import lldb
-class ProgressTesterCommand:
- program = "test-progress"
-
- @classmethod
- def register_lldb_command(cls, debugger, module_name):
- parser = cls.create_options()
- cls.__doc__ = parser.format_help()
- # Add any commands contained in this module to LLDB
- command = "command script add -c %s.%s %s" % (
- module_name,
- cls.__name__,
- cls.program,
- )
- debugger.HandleCommand(command)
- print(
- 'The "{0}" command has been installed, type "help {0}" or "{0} '
- '--help" for detailed help.'.format(cls.program)
- )
-
- @classmethod
- def create_options(cls):
- usage = "usage: %prog [options]"
- description = "SBProgress testing tool"
- # Opt parse is deprecated, but leaving this the way it is because it allows help formating
- # Additionally all our commands use optparse right now, ideally we migrate them all in one go.
- parser = optparse.OptionParser(
- description=description, prog=cls.program, usage=usage
- )
-
- parser.add_option(
- "--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(
- "--seconds",
- dest="seconds",
- help="Total number of seconds to wait between increments",
- 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):
- return "Progress Tester"
-
- def get_long_help(self):
- return self.help_string
-
- def __init__(self, debugger, unused):
- self.parser = self.create_options()
- self.help_string = self.parser.format_help()
+def make_parser():
+ parser = optparse.OptionParser(
+ prog="send-progress",
+ description="SBProgress testing tool",
+ usage="usage: %prog [options]",
+ )
+ parser.add_option(
+ "--total",
+ type="int",
+ default=None,
+ help="Total items in this progress object. Omit for indeterminate progress.",
+ )
+ parser.add_option(
+ "--seconds",
+ type="float",
+ default=0.0,
+ help="Seconds to sleep between increments.",
+ )
+ parser.add_option(
+ "--no-details",
+ action="store_true",
+ default=False,
+ help="Do not attach a per-step detail string.",
+ )
+ return parser
+
+
+class SendProgressCommand:
+ """Drive an lldb.SBProgress for lldb-dap tests."""
+
+ def __init__(self, debugger, internal_dict):
+ pass
def __call__(self, debugger, command, exe_ctx, result):
- command_args = shlex.split(command)
try:
- (cmd_options, args) = self.parser.parse_args(command_args)
- except:
+ parser = make_parser()
+ opts, _ = parser.parse_args(shlex.split(command))
+ except SystemExit:
result.SetError("option parsing failed")
return
- total = cmd_options.total
- if total is None:
+ if opts.total is None:
progress = lldb.SBProgress(
"Progress tester", "Initial Indeterminate Detail", debugger
)
+ iterations = 5
else:
progress = lldb.SBProgress(
- "Progress tester", "Initial Detail", total, debugger
+ "Progress tester", "Initial Detail", opts.total, debugger
)
- # Check to see if total is set to None to indicate an indeterminate
- # progress then default to 3 steps.
- with progress:
- if total is None:
- total = 3
+ iterations = opts.total - 1
- for i in range(1, total):
- if cmd_options.no_details:
+ with progress:
+ for i in range(iterations):
+ if opts.no_details:
progress.Increment(1)
else:
progress.Increment(1, f"Step {i}")
- time.sleep(cmd_options.seconds)
+ time.sleep(opts.seconds)
-def __lldb_init_module(debugger, dict):
- # Register all classes that have a register_lldb_command method
- for _name, cls in inspect.getmembers(sys.modules[__name__]):
- if inspect.isclass(cls) and callable(
- getattr(cls, "register_lldb_command", None)
- ):
- cls.register_lldb_command(debugger, __name__)
+def __lldb_init_module(debugger, internal_dict):
+ debugger.HandleCommand(
+ f"command script add -c {__name__}.SendProgressCommand send-progress"
+ )
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 3f57dfb66024d..2eeddb3342f30 100755
--- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -1,100 +1,84 @@
"""
-Test lldb-dap output events
+Test lldb-dap progress events (smoke test).
+
+This test only verifies that `ProgressReport` events sent actually reaches the client
+from lldb.
+
+The throttling check is covered by `lldb/unittests/DAP/ProgressEventTest.cpp`.
"""
-from lldbsuite.test.decorators import *
-from lldbsuite.test.lldbtest import *
-import json
import os
-import time
-import re
-import lldbdap_testcase
+from lldbsuite.test.decorators import *
+from lldbsuite.test.tools.lldb_dap import DAPTestCaseBase, DAPTestSession
+from lldbsuite.test.tools.lldb_dap.types import *
+_ProgressEvent = Union[ProgressStartEvent, ProgressUpdateEvent, ProgressEndEvent]
-class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
- def verify_progress_events(
- self,
- expected_title,
- expected_message=None,
- expected_message_regex=None,
- expected_not_in_message=None,
- only_verify_first_update=False,
- ):
- self.dap_server.wait_for_event(["progressEnd"])
- 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_message_regex is not None:
- self.assertTrue(re.match(expected_message_regex, 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)
- self.dap_server.progress_events.clear()
- @skipIfWindows
- def test(self):
- program = self.getBuildArtifact("a.out")
- self.build_and_launch(program, stopOnEntry=True)
- progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
- self.dap_server.request_evaluate(
- f"`command script import {progress_emitter}", context="repl"
- )
+class TestDAP_Progress(DAPTestCaseBase):
+ def collect_progress_events(self, session: DAPTestSession, *, after):
+ """Collect ProgressXXXX events between `after` and the next ProgressEndEvent."""
+ events: List[_ProgressEvent] = []
- # Test details.
- self.dap_server.request_evaluate(
- "`test-progress --total 3 --seconds 1", context="repl"
- )
+ def matches_progress_end(evt) -> bool:
+ events.append(evt)
+ return isinstance(evt, ProgressEndEvent)
- self.verify_progress_events(
- expected_title="Progress tester",
- expected_not_in_message="Progress tester",
+ session.wait_for_any_event(
+ (ProgressStartEvent, ProgressUpdateEvent, ProgressEndEvent),
+ after=after,
+ until=matches_progress_end,
+ timeout_msg="Collecting ProgressXXXXEvents until ProgressEndEvent",
)
+ return events
- # Test no details.
- self.dap_server.request_evaluate(
- "`test-progress --total 3 --seconds 1 --no-details", context="repl"
+ def verify_progress_events(
+ self,
+ events: List[_ProgressEvent],
+ *,
+ expected_title: str,
+ expected_message: Optional[str] = None,
+ expected_message_regex: Optional[str] = None,
+ expected_not_in_message: Optional[str] = None,
+ ):
+ # A progress group is shaped: [ProgressStart, ProgressUpdate*, ProgressEnd].
+ self.assertGreaterEqual(
+ len(events), 3, "expected at least start + one update + end"
)
+ [start, *updates, end] = events
- self.verify_progress_events(
- expected_title="Progress tester",
- expected_message="Initial Detail",
- )
+ self.assertIsInstance(start, ProgressStartEvent)
+ self.assertIn(expected_title, start.body.title)
+ self.assertIsInstance(end, ProgressEndEvent)
- # Test details indeterminate.
- self.dap_server.request_evaluate("`test-progress --seconds 1", context="repl")
+ for update in updates:
+ self.assertIsInstance(update, ProgressUpdateEvent)
+ message = update.body.message or ""
- self.verify_progress_events(
- expected_title="Progress tester: Initial Indeterminate Detail",
- expected_message_regex=r"Step [0-9]+",
- )
+ if expected_message is not None:
+ self.assertIn(expected_message, message)
+ if expected_message_regex is not None:
+ self.assertTrue(re.match(expected_message_regex, message))
+ if expected_not_in_message is not None:
+ self.assertNotIn(expected_not_in_message, message)
- # Test no details indeterminate.
- self.dap_server.request_evaluate(
- "`test-progress --seconds 1 --no-details", context="repl"
- )
+ @skipIfWindows
+ def test_progress(self):
+ program = self.getBuildArtifact("a.out")
+ session = self.build_and_create_session()
+ process_event = session.launch(LaunchArgs(program, stopOnEntry=True))
+ stopped = session.verify_stopped_on_entry(after=process_event)
+
+ progress_emitter = self.getSourcePath("Progress_emitter.py")
+ session.evaluate(f"`command script import {progress_emitter}", context="repl")
+ # Test details.
+ # 1 progress every 200ms, 10 times = 2s.
+ session.evaluate("`send-progress --total 10 --seconds 0.2", context="repl")
+ events = self.collect_progress_events(session, after=stopped)
self.verify_progress_events(
- expected_title="Progress tester: Initial Indeterminate Detail",
- expected_message="Initial Indeterminate Detail",
- only_verify_first_update=True,
+ events,
+ expected_title="Progress tester",
+ expected_not_in_message="Progress tester",
)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 7e59541e6c834..c76ea99caaa4f 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -16,6 +16,7 @@
#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "OutputRedirector.h"
+#include "ProgressEvent.h"
#include "Protocol/ProtocolBase.h"
#include "Protocol/ProtocolEvents.h"
#include "Protocol/ProtocolRequests.h"
@@ -120,11 +121,8 @@ DAP::DAP(Log &log, const ReplMode default_repl_mode,
const std::vector<String> &pre_init_commands, bool no_lldbinit,
llvm::StringRef client_name, DAPTransport &transport, MainLoop &loop)
: log(log), transport(transport), reference_storage(log, configuration),
- broadcaster("lldb-dap"),
- progress_event_reporter(
- [&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- repl_mode(default_repl_mode), no_lldbinit(no_lldbinit),
- m_client_name(client_name), m_loop(loop) {
+ broadcaster("lldb-dap"), repl_mode(default_repl_mode),
+ no_lldbinit(no_lldbinit), m_client_name(client_name), m_loop(loop) {
configuration.preInitCommands = pre_init_commands;
RegisterRequests();
}
@@ -423,104 +421,6 @@ void DAP::SendOutput(OutputType o, const llvm::StringRef output) {
} while (idx < output.size());
}
-// interface ProgressStartEvent extends Event {
-// event: 'progressStart';
-//
-// body: {
-// /**
-// * An ID that must be used in subsequent 'progressUpdate' and
-// 'progressEnd'
-// * events to make them refer to the same progress reporting.
-// * IDs must be unique within a debug session.
-// */
-// progressId: string;
-//
-// /**
-// * Mandatory (short) title of the progress reporting. Shown in the UI to
-// * describe the long running operation.
-// */
-// title: string;
-//
-// /**
-// * The request ID that this progress report is related to. If specified a
-// * debug adapter is expected to emit
-// * progress events for the long running request until the request has
-// been
-// * either completed or cancelled.
-// * If the request ID is omitted, the progress report is assumed to be
-// * related to some general activity of the debug adapter.
-// */
-// requestId?: number;
-//
-// /**
-// * If true, the request that reports progress may be canceled with a
-// * 'cancel' request.
-// * So this property basically controls whether the client should use UX
-// that
-// * supports cancellation.
-// * Clients that don't support cancellation are allowed to ignore the
-// * setting.
-// */
-// cancellable?: boolean;
-//
-// /**
-// * Optional, more detailed progress message.
-// */
-// message?: string;
-//
-// /**
-// * Optional progress percentage to display (value range: 0 to 100). If
-// * omitted no percentage will be shown.
-// */
-// percentage?: number;
-// };
-// }
-//
-// interface ProgressUpdateEvent extends Event {
-// event: 'progressUpdate';
-//
-// body: {
-// /**
-// * The ID that was introduced in the initial 'progressStart' event.
-// */
-// progressId: string;
-//
-// /**
-// * Optional, more detailed progress message. If omitted, the previous
-// * message (if any) is used.
-// */
-// message?: string;
-//
-// /**
-// * Optional progress percentage to display (value range: 0 to 100). If
-// * omitted no percentage will be shown.
-// */
-// percentage?: number;
-// };
-// }
-//
-// interface ProgressEndEvent extends Event {
-// event: 'progressEnd';
-//
-// body: {
-// /**
-// * The ID that was introduced in the initial 'ProgressStartEvent'.
-// */
-// progressId: string;
-//
-// /**
-// * Optional, more detailed progress message. If omitted, the previous
-// * message (if any) is used.
-// */
-// message?: string;
-// };
-// }
-
-void DAP::SendProgressEvent(uint64_t progress_id, const char *message,
- uint64_t completed, uint64_t total) {
- progress_event_reporter.Push(progress_id, message, completed, total);
-}
-
src_ref_t DAP::CreateSourceReference(lldb::addr_t address) {
std::lock_guard<std::mutex> guard(m_source_references_mutex);
auto iter = llvm::find(m_source_references, address);
@@ -1380,57 +1280,42 @@ llvm::Error DAP::InitializeDebugger() {
}
void DAP::ProgressEventThread(lldb::SBListener listener) {
+ using namespace std::chrono;
+
+ ProgressEventReporter reporter(
+ [this](protocol::Event event) { Send(std::move(event)); });
+ constexpr uint32_t start_delay =
+ duration_cast<seconds>(ProgressEventReporter::k_start_delay).count();
+
lldb::SBEvent event;
bool done = false;
while (!done) {
- if (listener.WaitForEvent(UINT32_MAX, event)) {
- const auto event_mask = event.GetType();
- if (event.BroadcasterMatchesRef(broadcaster)) {
- if (event_mask & eBroadcastBitStopProgressThread) {
- done = true;
- }
- } else {
- 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");
- 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");
- SendProgressEvent(progress_id, title.c_str(), completed, total);
- if (!details.empty())
- 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.
- SendProgressEvent(progress_id, details.c_str(), completed, total);
- }
- }
+ const uint32_t poll_time = reporter.HasPending() ? start_delay : UINT32_MAX;
+ if (!listener.WaitForEvent(poll_time, event)) {
+ reporter.Drain(steady_clock::now());
+ continue;
+ }
+
+ const auto event_mask = event.GetType();
+ if (event.BroadcasterMatchesRef(broadcaster)) {
+ if (event_mask & eBroadcastBitStopProgressThread)
+ done = true;
+ continue;
}
+
+ 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");
+
+ std::optional<std::string> title = st...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/224957
More information about the lldb-commits
mailing list