[Lldb-commits] [lldb] r179838 - Optimized the way breakpoint conditions are evaluated.
Sean Callanan
scallanan at apple.com
Fri Apr 19 00:09:16 PDT 2013
Author: spyffe
Date: Fri Apr 19 02:09:15 2013
New Revision: 179838
URL: http://llvm.org/viewvc/llvm-project?rev=179838&view=rev
Log:
Optimized the way breakpoint conditions are evaluated.
Previously, the options for a breakopint or its
locations stored only the text of the breakpoint
condition (ironically, they used ClangUserExpression
as a glorified std::string) and, each time the condition
had to be evaluated in the StopInfo code, the expression
parser would be invoked via a static method to parse and
then execute the expression.
I made several changes here:
- Each breakpoint location now has its own
ClangUserExpressionSP containing a version of
the breakpoint expression compiled for that exact
location.
- Whenever the breakpoint is hit, the breakpoint
condition expression is simply re-run to determine
whether to stop.
- If the process changes (e.g., it's re-run) or
the source code of the expression changes (we use
a hash so as to avoid doing string comparisons)
the ClangUserExpressionSP is re-generated.
This should improve performance of breakpoint
conditions significantly, and takes advantage of
the recent expression re-use work.
Modified:
lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h
lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
lldb/trunk/include/lldb/Expression/ClangUserExpression.h
lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
lldb/trunk/source/Expression/ClangUserExpression.cpp
lldb/trunk/source/Target/StopInfo.cpp
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointLocation.h Fri Apr 19 02:09:15 2013
@@ -24,6 +24,7 @@
#include "lldb/Core/Address.h"
#include "lldb/Target/Process.h"
#include "lldb/Core/StringList.h"
+#include "lldb/Expression/ClangUserExpression.h"
namespace lldb_private {
@@ -175,7 +176,10 @@ public:
// condition has been set.
//------------------------------------------------------------------
const char *
- GetConditionText () const;
+ GetConditionText (size_t *hash = NULL) const;
+
+ bool
+ ConditionSaysStop (ExecutionContext &exe_ctx, Error &error);
//------------------------------------------------------------------
@@ -381,6 +385,8 @@ private:
Breakpoint &m_owner; ///< The breakpoint that produced this object.
std::unique_ptr<BreakpointOptions> m_options_ap; ///< Breakpoint options pointer, NULL if we're using our breakpoint's options.
lldb::BreakpointSiteSP m_bp_site_sp; ///< Our breakpoint site (it may be shared by more than one location.)
+ ClangUserExpression::ClangUserExpressionSP m_user_expression_sp; ///< The compiled expression to use in testing our condition.
+ size_t m_condition_hash; ///< For testing whether the condition source code changed.
void
SendBreakpointLocationChangedEvent (lldb::BreakpointEventType eventKind);
Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/BreakpointOptions.h Fri Apr 19 02:09:15 2013
@@ -183,7 +183,7 @@ public:
/// A pointer to the condition expression text, or NULL if no
// condition has been set.
//------------------------------------------------------------------
- const char *GetConditionText () const;
+ const char *GetConditionText (size_t *hash = NULL) const;
//------------------------------------------------------------------
// Enabled/Ignore Count
@@ -349,8 +349,8 @@ private:
bool m_one_shot;
uint32_t m_ignore_count; // Number of times to ignore this breakpoint
std::unique_ptr<ThreadSpec> m_thread_spec_ap; // Thread for which this breakpoint will take
- std::unique_ptr<ClangUserExpression> m_condition_ap; // The condition to test.
-
+ std::string m_condition_text; // The condition to test.
+ size_t m_condition_text_hash; // Its hash, so that locations know when the condition is updated.
};
} // namespace lldb_private
Modified: lldb/trunk/include/lldb/Expression/ClangUserExpression.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Expression/ClangUserExpression.h?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Expression/ClangUserExpression.h (original)
+++ lldb/trunk/include/lldb/Expression/ClangUserExpression.h Fri Apr 19 02:09:15 2013
@@ -21,6 +21,7 @@
#include "lldb/lldb-forward.h"
#include "lldb/lldb-private.h"
+#include "lldb/Core/Address.h"
#include "lldb/Core/ClangForward.h"
#include "lldb/Expression/ClangExpression.h"
#include "lldb/Expression/ClangExpressionVariable.h"
@@ -112,6 +113,9 @@ public:
return m_can_interpret;
}
+ bool
+ MatchesContext (ExecutionContext &exe_ctx);
+
//------------------------------------------------------------------
/// Execute the parsed expression
///
@@ -383,34 +387,16 @@ private:
lldb::addr_t &cmd_ptr);
void
- InstallContext (ExecutionContext &exe_ctx)
- {
- m_process_wp = exe_ctx.GetProcessSP();
- m_target_wp = exe_ctx.GetTargetSP();
- m_frame_wp = exe_ctx.GetFrameSP();
- }
+ InstallContext (ExecutionContext &exe_ctx);
bool
LockAndCheckContext (ExecutionContext &exe_ctx,
lldb::TargetSP &target_sp,
lldb::ProcessSP &process_sp,
- lldb::StackFrameSP &frame_sp)
- {
- target_sp = m_target_wp.lock();
- process_sp = m_process_wp.lock();
- frame_sp = m_frame_wp.lock();
-
- if ((target_sp && target_sp.get() != exe_ctx.GetTargetPtr()) ||
- (process_sp && process_sp.get() != exe_ctx.GetProcessPtr()) ||
- (frame_sp && frame_sp.get() != exe_ctx.GetFramePtr()))
- return false;
-
- return true;
- }
+ lldb::StackFrameSP &frame_sp);
- lldb::TargetWP m_target_wp; ///< The target used as the context for the expression.
lldb::ProcessWP m_process_wp; ///< The process used as the context for the expression.
- lldb::StackFrameWP m_frame_wp; ///< The stack frame used as context for the expression.
+ Address m_address; ///< The address the process is stopped in.
std::string m_expr_text; ///< The text of the expression, as typed by the user
std::string m_expr_prefix; ///< The text of the translation-level definitions, as provided by the user
Modified: lldb/trunk/source/Breakpoint/BreakpointLocation.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointLocation.cpp?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointLocation.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointLocation.cpp Fri Apr 19 02:09:15 2013
@@ -240,9 +240,109 @@ BreakpointLocation::SetCondition (const
}
const char *
-BreakpointLocation::GetConditionText () const
+BreakpointLocation::GetConditionText (size_t *hash) const
{
- return GetOptionsNoCreate()->GetConditionText();
+ return GetOptionsNoCreate()->GetConditionText(hash);
+}
+
+bool
+BreakpointLocation::ConditionSaysStop (ExecutionContext &exe_ctx, Error &error)
+{
+ Log *log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_BREAKPOINTS);
+
+ size_t condition_hash;
+ const char *condition_text = GetConditionText(&condition_hash);
+
+ if (!condition_text)
+ return false;
+
+ if (condition_hash != m_condition_hash ||
+ !m_user_expression_sp ||
+ !m_user_expression_sp->MatchesContext(exe_ctx))
+ {
+ m_user_expression_sp.reset(new ClangUserExpression(condition_text,
+ NULL,
+ lldb::eLanguageTypeUnknown,
+ ClangUserExpression::eResultTypeAny));
+
+ StreamString errors;
+
+ if (!m_user_expression_sp->Parse(errors,
+ exe_ctx,
+ eExecutionPolicyOnlyWhenNeeded,
+ true))
+ {
+ error.SetErrorStringWithFormat("Couldn't parse conditional expression:\n%s",
+ errors.GetData());
+ m_user_expression_sp.reset();
+ return false;
+ }
+
+ m_condition_hash = condition_hash;
+ }
+
+ // We need to make sure the user sees any parse errors in their condition, so we'll hook the
+ // constructor errors up to the debugger's Async I/O.
+
+ ValueObjectSP result_value_sp;
+ const bool unwind_on_error = true;
+ const bool ignore_breakpoints = true;
+ const bool try_all_threads = true;
+
+ Error expr_error;
+
+ StreamString execution_errors;
+
+ ClangExpressionVariableSP result_variable_sp;
+
+ ExecutionResults result_code =
+ m_user_expression_sp->Execute(execution_errors,
+ exe_ctx,
+ unwind_on_error,
+ ignore_breakpoints,
+ m_user_expression_sp,
+ result_variable_sp,
+ try_all_threads,
+ ClangUserExpression::kDefaultTimeout);
+
+ bool ret;
+
+ if (result_code == eExecutionCompleted)
+ {
+ result_value_sp = result_variable_sp->GetValueObject();
+
+ if (result_value_sp)
+ {
+ Scalar scalar_value;
+ if (result_value_sp->ResolveValue (scalar_value))
+ {
+ if (scalar_value.ULongLong(1) == 0)
+ ret = false;
+ else
+ ret = true;
+ if (log)
+ log->Printf("Condition successfully evaluated, result is %s.\n",
+ ret ? "true" : "false");
+ }
+ else
+ {
+ ret = false;
+ error.SetErrorString("Failed to get an integer result from the expression");
+ }
+ }
+ else
+ {
+ ret = false;
+ error.SetErrorString("Failed to get any result from the expression");
+ }
+ }
+ else
+ {
+ ret = false;
+ error.SetErrorStringWithFormat("Couldn't execute expression:\n%s", execution_errors.GetData());
+ }
+
+ return ret;
}
uint32_t
Modified: lldb/trunk/source/Breakpoint/BreakpointOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointOptions.cpp?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/source/Breakpoint/BreakpointOptions.cpp (original)
+++ lldb/trunk/source/Breakpoint/BreakpointOptions.cpp Fri Apr 19 02:09:15 2013
@@ -42,7 +42,8 @@ BreakpointOptions::BreakpointOptions() :
m_one_shot (false),
m_ignore_count (0),
m_thread_spec_ap (),
- m_condition_ap()
+ m_condition_text (),
+ m_condition_text_hash (0)
{
}
@@ -56,13 +57,12 @@ BreakpointOptions::BreakpointOptions(con
m_enabled (rhs.m_enabled),
m_one_shot (rhs.m_one_shot),
m_ignore_count (rhs.m_ignore_count),
- m_thread_spec_ap (),
- m_condition_ap ()
+ m_thread_spec_ap ()
{
if (rhs.m_thread_spec_ap.get() != NULL)
m_thread_spec_ap.reset (new ThreadSpec(*rhs.m_thread_spec_ap.get()));
- if (rhs.m_condition_ap.get())
- m_condition_ap.reset (new ClangUserExpression (rhs.m_condition_ap->GetUserText(), NULL, lldb::eLanguageTypeUnknown, ClangUserExpression::eResultTypeAny));
+ m_condition_text = rhs.m_condition_text;
+ m_condition_text_hash = rhs.m_condition_text_hash;
}
//----------------------------------------------------------------------
@@ -79,8 +79,8 @@ BreakpointOptions::operator=(const Break
m_ignore_count = rhs.m_ignore_count;
if (rhs.m_thread_spec_ap.get() != NULL)
m_thread_spec_ap.reset(new ThreadSpec(*rhs.m_thread_spec_ap.get()));
- if (rhs.m_condition_ap.get())
- m_condition_ap.reset (new ClangUserExpression (rhs.m_condition_ap->GetUserText(), NULL, lldb::eLanguageTypeUnknown, ClangUserExpression::eResultTypeAny));
+ m_condition_text = rhs.m_condition_text;
+ m_condition_text_hash = rhs.m_condition_text_hash;
return *this;
}
@@ -162,24 +162,25 @@ BreakpointOptions::HasCallback ()
void
BreakpointOptions::SetCondition (const char *condition)
{
- if (condition == NULL || condition[0] == '\0')
- {
- if (m_condition_ap.get())
- m_condition_ap.reset();
- }
- else
- {
- m_condition_ap.reset(new ClangUserExpression (condition, NULL, lldb::eLanguageTypeUnknown, ClangUserExpression::eResultTypeAny));
- }
+ m_condition_text.assign(condition);
+ std::hash<std::string> hasher;
+ m_condition_text_hash = hasher(m_condition_text);
}
const char *
-BreakpointOptions::GetConditionText () const
+BreakpointOptions::GetConditionText (size_t *hash) const
{
- if (m_condition_ap.get())
- return m_condition_ap->GetUserText();
+ if (!m_condition_text.empty())
+ {
+ if (hash)
+ *hash = m_condition_text_hash;
+
+ return m_condition_text.c_str();
+ }
else
+ {
return NULL;
+ }
}
const ThreadSpec *
@@ -250,12 +251,12 @@ BreakpointOptions::GetDescription (Strea
m_callback_baton_sp->GetDescription (s, level);
}
}
- if (m_condition_ap.get())
+ if (!m_condition_text.empty())
{
if (level != eDescriptionLevelBrief)
{
s->EOL();
- s->Printf("Condition: %s\n", m_condition_ap->GetUserText());
+ s->Printf("Condition: %s\n", m_condition_text.c_str());
}
}
}
Modified: lldb/trunk/source/Expression/ClangUserExpression.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Expression/ClangUserExpression.cpp?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/source/Expression/ClangUserExpression.cpp (original)
+++ lldb/trunk/source/Expression/ClangUserExpression.cpp Fri Apr 19 02:09:15 2013
@@ -329,6 +329,54 @@ ClangUserExpression::ScanContext(Executi
}
}
+void
+ClangUserExpression::InstallContext (ExecutionContext &exe_ctx)
+{
+ m_process_wp = exe_ctx.GetProcessSP();
+
+ lldb::StackFrameSP frame_sp = exe_ctx.GetFrameSP();
+
+ if (frame_sp)
+ m_address = frame_sp->GetFrameCodeAddress();
+}
+
+bool
+ClangUserExpression::LockAndCheckContext (ExecutionContext &exe_ctx,
+ lldb::TargetSP &target_sp,
+ lldb::ProcessSP &process_sp,
+ lldb::StackFrameSP &frame_sp)
+{
+ lldb::ProcessSP expected_process_sp = m_process_wp.lock();
+ process_sp = exe_ctx.GetProcessSP();
+
+ if (process_sp != expected_process_sp)
+ return false;
+
+ process_sp = exe_ctx.GetProcessSP();
+ target_sp = exe_ctx.GetTargetSP();
+ frame_sp = exe_ctx.GetFrameSP();
+
+ if (m_address.IsValid())
+ {
+ if (!frame_sp)
+ return false;
+ else
+ return (0 == Address::CompareLoadAddress(m_address, frame_sp->GetFrameCodeAddress(), target_sp.get()));
+ }
+
+ return true;
+}
+
+bool
+ClangUserExpression::MatchesContext (ExecutionContext &exe_ctx)
+{
+ lldb::TargetSP target_sp;
+ lldb::ProcessSP process_sp;
+ lldb::StackFrameSP frame_sp;
+
+ return LockAndCheckContext(exe_ctx, target_sp, process_sp, frame_sp);
+}
+
// This is a really nasty hack, meant to fix Objective-C expressions of the form
// (int)[myArray count]. Right now, because the type information for count is
// not available, [myArray count] returns id, which can't be directly cast to
@@ -545,7 +593,7 @@ ClangUserExpression::PrepareToExecuteJIT
process,
frame))
{
- error_stream.Printf("The context has changed before we could JIT the expression!");
+ error_stream.Printf("The context has changed before we could JIT the expression!\n");
return false;
}
Modified: lldb/trunk/source/Target/StopInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/StopInfo.cpp?rev=179838&r1=179837&r2=179838&view=diff
==============================================================================
--- lldb/trunk/source/Target/StopInfo.cpp (original)
+++ lldb/trunk/source/Target/StopInfo.cpp Fri Apr 19 02:09:15 2013
@@ -387,79 +387,36 @@ protected:
// First run the condition for the breakpoint. If that says we should stop, then we'll run
// the callback for the breakpoint. If the callback says we shouldn't stop that will win.
- bool condition_says_stop = true;
if (bp_loc_sp->GetConditionText() != NULL)
{
- // We need to make sure the user sees any parse errors in their condition, so we'll hook the
- // constructor errors up to the debugger's Async I/O.
+ Error condition_error;
+ bool condition_says_stop = bp_loc_sp->ConditionSaysStop(exe_ctx, condition_error);
- ValueObjectSP result_valobj_sp;
-
- ExecutionResults result_code;
- ValueObjectSP result_value_sp;
- const bool unwind_on_error = true;
- const bool ignore_breakpoints = true;
- Error error;
- result_code = ClangUserExpression::EvaluateWithError (exe_ctx,
- eExecutionPolicyOnlyWhenNeeded,
- lldb::eLanguageTypeUnknown,
- ClangUserExpression::eResultTypeAny,
- unwind_on_error,
- ignore_breakpoints,
- bp_loc_sp->GetConditionText(),
- NULL,
- result_value_sp,
- error,
- true,
- ClangUserExpression::kDefaultTimeout);
- if (result_code == eExecutionCompleted)
- {
- if (result_value_sp)
- {
- Scalar scalar_value;
- if (result_value_sp->ResolveValue (scalar_value))
- {
- if (scalar_value.ULongLong(1) == 0)
- condition_says_stop = false;
- else
- condition_says_stop = true;
- if (log)
- log->Printf("Condition successfully evaluated, result is %s.\n",
- m_should_stop ? "true" : "false");
- }
- else
- {
- condition_says_stop = true;
- if (log)
- log->Printf("Failed to get an integer result from the expression.");
- }
- }
- }
- else
+ if (!condition_error.Success())
{
Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
StreamSP error_sp = debugger.GetAsyncErrorStream ();
error_sp->Printf ("Stopped due to an error evaluating condition of breakpoint ");
bp_loc_sp->GetDescription (error_sp.get(), eDescriptionLevelBrief);
- error_sp->Printf (": \"%s\"",
+ error_sp->Printf (": \"%s\"",
bp_loc_sp->GetConditionText());
error_sp->EOL();
- const char *err_str = error.AsCString("<Unknown Error>");
+ const char *err_str = condition_error.AsCString("<Unknown Error>");
if (log)
log->Printf("Error evaluating condition: \"%s\"\n", err_str);
error_sp->PutCString (err_str);
- error_sp->EOL();
+ error_sp->EOL();
error_sp->Flush();
// If the condition fails to be parsed or run, we should stop.
condition_says_stop = true;
}
+ else
+ {
+ if (!condition_says_stop)
+ continue;
+ }
}
-
- // If this location's condition says we should aren't going to stop,
- // then don't run the callback for this location.
- if (!condition_says_stop)
- continue;
bool callback_says_stop;
More information about the lldb-commits
mailing list