[Lldb-commits] [lldb] Improving performance of multiple threads stepping over the same brea… (PR #180101)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Feb 10 09:50:41 PST 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: None (barsolo2000)
<details>
<summary>Changes</summary>
Draft for now, following up from https://discourse.llvm.org/t/improving-performance-of-multiple-threads-stepping-over-the-same-breakpoint/89637
---
Patch is 31.16 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/180101.diff
6 Files Affected:
- (modified) lldb/include/lldb/Target/ThreadList.h (+21)
- (modified) lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h (+19)
- (modified) lldb/source/Target/ThreadList.cpp (+169-1)
- (modified) lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp (+14-4)
- (added) lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py (+216)
- (added) lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentBatchedBreakpointStepOver.py (+151)
``````````diff
diff --git a/lldb/include/lldb/Target/ThreadList.h b/lldb/include/lldb/Target/ThreadList.h
index c108962003598..97541ed83b200 100644
--- a/lldb/include/lldb/Target/ThreadList.h
+++ b/lldb/include/lldb/Target/ThreadList.h
@@ -9,7 +9,9 @@
#ifndef LLDB_TARGET_THREADLIST_H
#define LLDB_TARGET_THREADLIST_H
+#include <map>
#include <mutex>
+#include <set>
#include <vector>
#include "lldb/Target/Thread.h"
@@ -141,6 +143,19 @@ class ThreadList : public ThreadCollection {
/// Precondition: both thread lists must be belong to the same process.
void Update(ThreadList &rhs);
+ /// Called by ThreadPlanStepOverBreakpoint when a thread finishes stepping
+ /// over a breakpoint. This tracks which threads are still stepping over
+ /// each breakpoint address, and only re-enables the breakpoint when ALL
+ /// threads have finished stepping over it.
+ void ThreadFinishedSteppingOverBreakpoint(lldb::addr_t breakpoint_addr,
+ lldb::tid_t tid);
+
+ /// Register a thread that is about to step over a breakpoint.
+ /// The breakpoint will be re-enabled only after all registered threads
+ /// have called ThreadFinishedSteppingOverBreakpoint.
+ void RegisterThreadSteppingOverBreakpoint(lldb::addr_t breakpoint_addr,
+ lldb::tid_t tid);
+
protected:
void SetShouldReportStop(Vote vote);
@@ -154,6 +169,12 @@ class ThreadList : public ThreadCollection {
m_selected_tid; ///< For targets that need the notion of a current thread.
std::vector<lldb::tid_t> m_expression_tid_stack;
+ /// Tracks which threads are currently stepping over each breakpoint address.
+ /// Key: breakpoint address, Value: set of thread IDs stepping over it.
+ /// When a thread finishes stepping, it's removed from the set. When the set
+ /// becomes empty, the breakpoint is re-enabled.
+ std::map<lldb::addr_t, std::set<lldb::tid_t>> m_threads_stepping_over_bp;
+
private:
ThreadList() = delete;
};
diff --git a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
index 0da8dbf44ffd8..0f0bbc0be4142 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h
@@ -36,6 +36,24 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan {
lldb::addr_t GetBreakpointLoadAddress() const { return m_breakpoint_addr; }
+ /// When set to true, the breakpoint site will NOT be re-enabled directly
+ /// by this plan. Instead, the plan will call
+ /// ThreadList::ThreadFinishedSteppingOverBreakpoint() when it completes,
+ /// allowing ThreadList to track all threads stepping over the same
+ /// breakpoint and only re-enable it when ALL threads have finished.
+ void SetDeferReenableBreakpointSite(bool defer) {
+ m_defer_reenable_breakpoint_site = defer;
+ }
+
+ bool GetDeferReenableBreakpointSite() const {
+ return m_defer_reenable_breakpoint_site;
+ }
+
+ /// Mark the breakpoint site as already re-enabled, suppressing any
+ /// re-enable in DidPop()/ThreadDestroyed(). Used when discarding plans
+ /// during WillResume cleanup to avoid spurious breakpoint toggles.
+ void SetReenabledBreakpointSite() { m_reenabled_breakpoint_site = true; }
+
protected:
bool DoPlanExplainsStop(Event *event_ptr) override;
bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
@@ -47,6 +65,7 @@ class ThreadPlanStepOverBreakpoint : public ThreadPlan {
lldb::user_id_t m_breakpoint_site_id;
bool m_auto_continue;
bool m_reenabled_breakpoint_site;
+ bool m_defer_reenable_breakpoint_site = false;
ThreadPlanStepOverBreakpoint(const ThreadPlanStepOverBreakpoint &) = delete;
const ThreadPlanStepOverBreakpoint &
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index 77a1c40b95f70..fcfcf88e7fe15 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -15,6 +15,7 @@
#include "lldb/Target/Thread.h"
#include "lldb/Target/ThreadList.h"
#include "lldb/Target/ThreadPlan.h"
+#include "lldb/Target/ThreadPlanStepOverBreakpoint.h"
#include "lldb/Utility/LLDBAssert.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
@@ -504,9 +505,31 @@ bool ThreadList::WillResume(RunDirection &direction) {
collection::iterator pos, end = m_threads.end();
+ // Clear tracking state from the previous stop and pop any leftover
+ // StepOverBreakpoint plans. This gives us a clean slate: plans will be
+ // recreated fresh by SetupToStepOverBreakpointIfNeeded below, and the
+ // batching logic will recompute deferred state from scratch.
+ m_threads_stepping_over_bp.clear();
+ for (pos = m_threads.begin(); pos != end; ++pos) {
+ ThreadSP thread_sp(*pos);
+ ThreadPlan *plan = thread_sp->GetCurrentPlan();
+ if (plan && plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) {
+ // Suppress the re-enable side effect in DidPop() — the breakpoint
+ // may still be disabled from the previous batch, and we don't want
+ // to toggle it. The new plans will handle disable/re-enable correctly.
+ static_cast<ThreadPlanStepOverBreakpoint *>(plan)
+ ->SetReenabledBreakpointSite();
+ thread_sp->DiscardPlan();
+ }
+ }
+
// Go through the threads and see if any thread wants to run just itself.
// if so then pick one and run it.
+ // Collect threads for batched vCont - multiple threads at the same breakpoint
+ // can be stepped together in a single vCont packet.
+ std::vector<ThreadSP> batched_step_threads;
+
ThreadList run_me_only_list(m_process);
run_me_only_list.SetStopID(m_process.GetStopID());
@@ -576,6 +599,13 @@ bool ThreadList::WillResume(RunDirection &direction) {
assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction);
}
} else {
+ // Pre-scan to find all threads that need to step over a breakpoint,
+ // and group them by breakpoint address. This optimization allows us to
+ // step multiple threads over the same breakpoint with minimal breakpoint
+ // swaps - only the last thread in each group will re-enable the breakpoint.
+ std::map<lldb::addr_t, std::vector<ThreadSP>> breakpoint_groups;
+ bool found_run_before_public_stop = false;
+
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
if (thread_sp->GetResumeState() != eStateSuspended) {
@@ -589,14 +619,71 @@ bool ThreadList::WillResume(RunDirection &direction) {
assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
// You can't say "stop others" and also want yourself to be suspended.
assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+
+ // Get the breakpoint address from the step-over-breakpoint plan
+ ThreadPlan *current_plan = thread_sp->GetCurrentPlan();
+ if (current_plan &&
+ current_plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) {
+ ThreadPlanStepOverBreakpoint *bp_plan =
+ static_cast<ThreadPlanStepOverBreakpoint *>(current_plan);
+ lldb::addr_t bp_addr = bp_plan->GetBreakpointLoadAddress();
+ breakpoint_groups[bp_addr].push_back(thread_sp);
+ }
+
thread_to_run = thread_sp;
if (thread_sp->ShouldRunBeforePublicStop()) {
// This takes precedence, so if we find one of these, service it:
+ found_run_before_public_stop = true;
break;
}
}
}
}
+
+ // Only apply batching optimization if we have a complete picture of
+ // breakpoint groups. If a ShouldRunBeforePublicStop thread caused the
+ // scan to exit early, the groups are incomplete and the priority thread
+ // must run solo. Deferred state will be cleaned up on next WillResume().
+ if (!found_run_before_public_stop) {
+ // For each group of threads at the same breakpoint, register them with
+ // ThreadList and set them to use deferred re-enable. The breakpoint will
+ // only be re-enabled when ALL threads have finished stepping over it.
+ // Also collect threads for batched vCont if multiple threads at same BP.
+ for (auto &group : breakpoint_groups) {
+ lldb::addr_t bp_addr = group.first;
+ std::vector<ThreadSP> &threads = group.second;
+
+ if (threads.size() > 1) {
+ // Multiple threads stepping over the same breakpoint - use tracking
+ for (ThreadSP &thread_sp : threads) {
+ // Register this thread as stepping over the breakpoint
+ RegisterThreadSteppingOverBreakpoint(bp_addr, thread_sp->GetID());
+
+ // Set the plan to defer re-enabling (use callback instead).
+ ThreadPlan *plan = thread_sp->GetCurrentPlan();
+ // Verify the plan is actually a StepOverBreakpoint plan.
+ if (plan &&
+ plan->GetKind() == ThreadPlan::eKindStepOverBreakpoint) {
+ ThreadPlanStepOverBreakpoint *bp_plan =
+ static_cast<ThreadPlanStepOverBreakpoint *>(plan);
+ bp_plan->SetDeferReenableBreakpointSite(true);
+ }
+ }
+
+ // Collect for batched vCont - pick the largest group
+ if (threads.size() > batched_step_threads.size()) {
+ batched_step_threads = threads;
+ }
+ }
+ // Single thread at breakpoint - keeps default behavior (re-enable
+ // directly)
+ }
+
+ // If we found a batch, use the first thread as thread_to_run
+ if (!batched_step_threads.empty()) {
+ thread_to_run = batched_step_threads[0];
+ }
+ }
}
if (thread_to_run != nullptr) {
@@ -615,7 +702,26 @@ bool ThreadList::WillResume(RunDirection &direction) {
bool need_to_resume = true;
- if (thread_to_run == nullptr) {
+ if (!batched_step_threads.empty()) {
+ // Batched stepping: all threads in the batch step together,
+ // all other threads stay suspended.
+ std::set<lldb::tid_t> batch_tids;
+ for (ThreadSP &thread_sp : batched_step_threads) {
+ batch_tids.insert(thread_sp->GetID());
+ }
+
+ for (pos = m_threads.begin(); pos != end; ++pos) {
+ ThreadSP thread_sp(*pos);
+ if (batch_tids.count(thread_sp->GetID()) > 0) {
+ // This thread is in the batch - let it step
+ if (!thread_sp->ShouldResume(thread_sp->GetCurrentPlan()->RunState()))
+ need_to_resume = false;
+ } else {
+ // Not in the batch - suspend it
+ thread_sp->ShouldResume(eStateSuspended);
+ }
+ }
+ } else if (thread_to_run == nullptr) {
// Everybody runs as they wish:
for (pos = m_threads.begin(); pos != end; ++pos) {
ThreadSP thread_sp(*pos);
@@ -801,3 +907,65 @@ ThreadList::ExpressionExecutionThreadPusher::ExpressionExecutionThreadPusher(
m_thread_list->PushExpressionExecutionThread(m_tid);
}
}
+
+void ThreadList::RegisterThreadSteppingOverBreakpoint(addr_t breakpoint_addr,
+ tid_t tid) {
+ std::lock_guard<std::recursive_mutex> guard(GetMutex());
+ m_threads_stepping_over_bp[breakpoint_addr].insert(tid);
+
+ Log *log = GetLog(LLDBLog::Step);
+ LLDB_LOGF(log,
+ "ThreadList::%s: Registered thread 0x%" PRIx64
+ " stepping over breakpoint at 0x%" PRIx64 " (now %zu threads)",
+ __FUNCTION__, tid, breakpoint_addr,
+ m_threads_stepping_over_bp[breakpoint_addr].size());
+}
+
+void ThreadList::ThreadFinishedSteppingOverBreakpoint(addr_t breakpoint_addr,
+ tid_t tid) {
+ std::lock_guard<std::recursive_mutex> guard(GetMutex());
+
+ Log *log = GetLog(LLDBLog::Step);
+
+ auto it = m_threads_stepping_over_bp.find(breakpoint_addr);
+ if (it == m_threads_stepping_over_bp.end()) {
+ // No threads registered for this breakpoint - just re-enable it directly
+ LLDB_LOGF(log,
+ "ThreadList::%s: Thread 0x%" PRIx64
+ " finished stepping over breakpoint at 0x%" PRIx64
+ " but no threads were registered, re-enabling directly",
+ __FUNCTION__, tid, breakpoint_addr);
+ BreakpointSiteSP bp_site_sp(
+ m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr));
+ if (bp_site_sp) {
+ m_process.EnableBreakpointSite(bp_site_sp.get());
+ }
+ return;
+ }
+
+ // Remove this thread from the set
+ it->second.erase(tid);
+
+ LLDB_LOGF(log,
+ "ThreadList::%s: Thread 0x%" PRIx64
+ " finished stepping over breakpoint at 0x%" PRIx64
+ " (%zu threads remaining)",
+ __FUNCTION__, tid, breakpoint_addr, it->second.size());
+
+ // If no more threads are stepping over this breakpoint, re-enable it
+ if (it->second.empty()) {
+ LLDB_LOGF(log,
+ "ThreadList::%s: All threads finished stepping over breakpoint "
+ "at 0x%" PRIx64 ", re-enabling breakpoint",
+ __FUNCTION__, breakpoint_addr);
+
+ BreakpointSiteSP bp_site_sp(
+ m_process.GetBreakpointSiteList().FindByAddress(breakpoint_addr));
+ if (bp_site_sp) {
+ m_process.EnableBreakpointSite(bp_site_sp.get());
+ }
+
+ // Clean up the entry
+ m_threads_stepping_over_bp.erase(it);
+ }
+}
diff --git a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
index 3602527a9231b..70538e21153b6 100644
--- a/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp
@@ -10,6 +10,7 @@
#include "lldb/Target/Process.h"
#include "lldb/Target/RegisterContext.h"
+#include "lldb/Target/ThreadList.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/Log.h"
#include "lldb/Utility/Stream.h"
@@ -155,10 +156,19 @@ bool ThreadPlanStepOverBreakpoint::MischiefManaged() {
void ThreadPlanStepOverBreakpoint::ReenableBreakpointSite() {
if (!m_reenabled_breakpoint_site) {
m_reenabled_breakpoint_site = true;
- BreakpointSiteSP bp_site_sp(
- m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr));
- if (bp_site_sp) {
- m_process.EnableBreakpointSite(bp_site_sp.get());
+
+ if (m_defer_reenable_breakpoint_site) {
+ // Let ThreadList track all threads stepping over this breakpoint.
+ // It will re-enable the breakpoint only when ALL threads have finished.
+ m_process.GetThreadList().ThreadFinishedSteppingOverBreakpoint(
+ m_breakpoint_addr, GetThread().GetID());
+ } else {
+ // Default behavior: re-enable the breakpoint directly
+ BreakpointSiteSP bp_site_sp(
+ m_process.GetBreakpointSiteList().FindByAddress(m_breakpoint_addr));
+ if (bp_site_sp) {
+ m_process.EnableBreakpointSite(bp_site_sp.get());
+ }
}
}
}
diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py
new file mode 100644
index 0000000000000..4954037565c76
--- /dev/null
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestBatchedBreakpointStepOver.py
@@ -0,0 +1,216 @@
+"""
+Test that when multiple threads are stopped at the same breakpoint, LLDB sends
+a batched vCont with multiple step actions and only one breakpoint disable/
+re-enable pair, rather than stepping each thread individually with repeated
+breakpoint toggles.
+
+Uses a mock GDB server to directly verify the packets LLDB sends.
+"""
+
+import re
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbclient import GDBRemoteTestBase
+
+
+class TestBatchedBreakpointStepOver(GDBRemoteTestBase):
+ @skipIfXmlSupportMissing
+ def test(self):
+ BP_ADDR = 0x0000000000401020
+ # PC after stepping past the breakpoint instruction.
+ STEPPED_PC = BP_ADDR + 1
+ NUM_THREADS = 10
+ TIDS = [0x101 + i for i in range(NUM_THREADS)]
+
+ class MyResponder(MockGDBServerResponder):
+ def __init__(self):
+ MockGDBServerResponder.__init__(self)
+ self.resume_count = 0
+ # Track which threads have completed their step
+ self.stepped_threads = set()
+
+ def qSupported(self, client_supported):
+ return (
+ "PacketSize=3fff;QStartNoAckMode+;"
+ "qXfer:features:read+;swbreak+;hwbreak+"
+ )
+
+ def qfThreadInfo(self):
+ return "m" + ",".join("{:x}".format(t) for t in TIDS)
+
+ def qsThreadInfo(self):
+ return "l"
+
+ def haltReason(self):
+ # All threads stopped at the breakpoint address.
+ threads_str = ",".join("{:x}".format(t) for t in TIDS)
+ pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS)
+ return "T05thread:{:x};threads:{};thread-pcs:{};" "swbreak:;".format(
+ TIDS[0], threads_str, pcs_str
+ )
+
+ def threadStopInfo(self, threadnum):
+ threads_str = ",".join("{:x}".format(t) for t in TIDS)
+ pcs_str = ",".join("{:x}".format(BP_ADDR) for _ in TIDS)
+ return "T05thread:{:x};threads:{};thread-pcs:{};" "swbreak:;".format(
+ threadnum, threads_str, pcs_str
+ )
+
+ def setBreakpoint(self, packet):
+ return "OK"
+
+ def readRegisters(self):
+ return "00" * 160
+
+ def readRegister(self, regno):
+ return "00" * 8
+
+ def qXferRead(self, obj, annex, offset, length):
+ if annex == "target.xml":
+ return (
+ """<?xml version="1.0"?>
+ <target version="1.0">
+ <architecture>i386:x86-64</architecture>
+ <feature name="org.gnu.gdb.i386.core">
+ <reg name="rax" bitsize="64" regnum="0" type="int" group="general"/>
+ <reg name="rbx" bitsize="64" regnum="1" type="int" group="general"/>
+ <reg name="rcx" bitsize="64" regnum="2" type="int" group="general"/>
+ <reg name="rdx" bitsize="64" regnum="3" type="int" group="general"/>
+ <reg name="rsi" bitsize="64" regnum="4" type="int" group="general"/>
+ <reg name="rdi" bitsize="64" regnum="5" type="int" group="general"/>
+ <reg name="rbp" bitsize="64" regnum="6" type="data_ptr" group="general"/>
+ <reg name="rsp" bitsize="64" regnum="7" type="data_ptr" group="general"/>
+ <reg name="r8" bitsize="64" regnum="8" type="int" group="general"/>
+ <reg name="r9" bitsize="64" regnum="9" type="int" group="general"/>
+ <reg name="r10" bitsize="64" regnum="10" type="int" group="general"/>
+ <reg name="r11" bitsize="64" regnum="11" type="int" group="general"/>
+ <reg name="r12" bitsize="64" regnum="12" type="int" group="general"/>
+ <reg name="r13" bitsize="64" regnum="13" type="int" group="general"/>
+ <reg name="r14" bitsize="64" regnum="14" type="int" group="general"/>
+ <reg name="r15" bitsize="64" regnum="15" type="int" group="general"/>
+ <reg name="rip" bitsize="64" regnum="16" type="code_ptr" group="general"/>
+ <reg name="eflags" bitsize="32" regnum="17" type="int" group="general"/>
+ <reg name="cs" bitsize="32" regnum="18" type="int" group="general"/>
+ <reg name="ss" bitsize="32" regnum="19" type="int" group="general"/>
+ </feature>
+ </target>""",
+ False,
+ )
+ return None, False
+
+ def other(self, packet):
+ if packet == "vCont?"...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/180101
More information about the lldb-commits
mailing list