https://github.com/barsolo2000 created https://github.com/llvm/llvm-project/pull/180101
Draft for now, following up from https://discourse.llvm.org/t/improving-performance-of-multiple-threads-stepping-over-the-same-breakpoint/89637 >From 774b855c6d08d551063e8674a3aa13a8458df02f Mon Sep 17 00:00:00 2001 From: Bar Soloveychik <[email protected]> Date: Thu, 5 Feb 2026 17:53:25 -0800 Subject: [PATCH] Improving performance of multiple threads stepping over the same breakpoint --- lldb/include/lldb/Target/ThreadList.h | 21 ++++ .../Target/ThreadPlanStepOverBreakpoint.h | 14 +++ lldb/source/Target/ThreadList.cpp | 101 ++++++++++++++++++ .../Target/ThreadPlanStepOverBreakpoint.cpp | 18 +++- 4 files changed, 150 insertions(+), 4 deletions(-) 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..d2cea98318ba7 100644 --- a/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h +++ b/lldb/include/lldb/Target/ThreadPlanStepOverBreakpoint.h @@ -36,6 +36,19 @@ 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; + } + protected: bool DoPlanExplainsStop(Event *event_ptr) override; bool DoWillResume(lldb::StateType resume_state, bool current_plan) override; @@ -47,6 +60,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..b9ebfe871ce34 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" @@ -576,6 +577,12 @@ 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; + for (pos = m_threads.begin(); pos != end; ++pos) { ThreadSP thread_sp(*pos); if (thread_sp->GetResumeState() != eStateSuspended) { @@ -589,6 +596,14 @@ 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(); + 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: @@ -597,6 +612,30 @@ bool ThreadList::WillResume(RunDirection &direction) { } } } + + // 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. + 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(); + ThreadPlanStepOverBreakpoint *bp_plan = + static_cast<ThreadPlanStepOverBreakpoint *>(plan); + bp_plan->SetDeferReenableBreakpointSite(true); + } + } + // Single thread at breakpoint - keeps default behavior (re-enable + // directly) + } } if (thread_to_run != nullptr) { @@ -801,3 +840,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()); + } } } } _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
