[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
This revision was automatically updated to reflect the committed changes. Closed by commit rL288326: Use Timeout<> in Process::RunThreadPlan (authored by labath). Changed prior to commit: https://reviews.llvm.org/D27258?vs=79737&id=79886#toc Repository: rL LLVM https://reviews.llvm.org/D27258 Files: lldb/trunk/source/Target/Process.cpp Index: lldb/trunk/source/Target/Process.cpp === --- lldb/trunk/source/Target/Process.cpp +++ lldb/trunk/source/Target/Process.cpp @@ -13,6 +13,7 @@ #include // Other libraries and framework includes +#include "llvm/Support/ScopedPrinter.h" // Project includes #include "Plugins/Process/Utility/InferiorCallPOSIX.h" #include "lldb/Breakpoint/BreakpointLocation.h" @@ -4799,6 +4800,45 @@ }; } // anonymous namespace +static microseconds +GetOneThreadExpressionTimeout(const EvaluateExpressionOptions &options) { + const milliseconds default_one_thread_timeout(250); + + // If the overall wait is forever, then we don't need to worry about it. + if (options.GetTimeoutUsec() == 0) { +if (options.GetOneThreadTimeoutUsec() != 0) + return microseconds(options.GetOneThreadTimeoutUsec()); +return default_one_thread_timeout; + } + + // If the one thread timeout is set, use it. + if (options.GetOneThreadTimeoutUsec() != 0) +return microseconds(options.GetOneThreadTimeoutUsec()); + + // Otherwise use half the total timeout, bounded by the + // default_one_thread_timeout. + return std::min(default_one_thread_timeout, +microseconds(options.GetTimeoutUsec()) / 2); +} + +static Timeout +GetExpressionTimeout(const EvaluateExpressionOptions &options, + bool before_first_timeout) { + // If we are going to run all threads the whole time, or if we are only + // going to run one thread, we can just return the overall timeout. + if (!options.GetStopOthers() || !options.GetTryAllThreads()) +return ConvertTimeout(microseconds(options.GetTimeoutUsec())); + + if (before_first_timeout) +return GetOneThreadExpressionTimeout(options); + + if (options.GetTimeoutUsec() == 0) +return llvm::None; + else +return microseconds(options.GetTimeoutUsec()) - + GetOneThreadExpressionTimeout(options); +} + ExpressionResults Process::RunThreadPlan(ExecutionContext &exe_ctx, lldb::ThreadPlanSP &thread_plan_sp, @@ -4879,6 +4919,16 @@ } } + // Make sure the timeout values make sense. The one thread timeout needs to be + // smaller than the overall timeout. + if (options.GetOneThreadTimeoutUsec() != 0 && options.GetTimeoutUsec() != 0 && + options.GetTimeoutUsec() < options.GetOneThreadTimeoutUsec()) { +diagnostic_manager.PutString(eDiagnosticSeverityError, + "RunThreadPlan called with one thread " + "timeout greater than total timeout"); +return eExpressionSetupError; + } + StackID ctx_frame_id = selected_frame_sp->GetStackID(); // N.B. Running the target may unset the currently selected thread and frame. @@ -4985,67 +5035,20 @@ // that we have to halt the target. bool do_resume = true; bool handle_running_event = true; -const uint64_t default_one_thread_timeout_usec = 25; // This is just for accounting: uint32_t num_resumes = 0; -uint32_t timeout_usec = options.GetTimeoutUsec(); -uint32_t one_thread_timeout_usec; -uint32_t all_threads_timeout_usec = 0; - // If we are going to run all threads the whole time, or if we are only -// going to run one thread, -// then we don't need the first timeout. So we set the final timeout, and -// pretend we are after the -// first timeout already. - -if (!options.GetStopOthers() || !options.GetTryAllThreads()) { +// going to run one thread, then we don't need the first timeout. So we +// pretend we are after the first timeout already. +if (!options.GetStopOthers() || !options.GetTryAllThreads()) before_first_timeout = false; - one_thread_timeout_usec = 0; - all_threads_timeout_usec = timeout_usec; -} else { - uint32_t option_one_thread_timeout = options.GetOneThreadTimeoutUsec(); - - // If the overall wait is forever, then we only need to set the one thread - // timeout: - if (timeout_usec == 0) { -if (option_one_thread_timeout != 0) - one_thread_timeout_usec = option_one_thread_timeout; -else - one_thread_timeout_usec = default_one_thread_timeout_usec; - } else { -// Otherwise, if the one thread timeout is set, make sure it isn't -// longer than the overall timeout, -// and use it, otherwise use half the total timeout, bounded by the -// default_one_thread_timeout_usec. -uint64_t computed_one_thread_timeout; -if (option_one_thread_
[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks correct to me as well. Thanks for taking the time to clean this up. Comment at: source/Target/Process.cpp:4926-4928 +diagnostic_manager.PutString(eDiagnosticSeverityError, + "RunThreadPlan called without one thread " + "timeout greater than total timeout"); without -> with The error was in the original. https://reviews.llvm.org/D27258 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D27258: Use Timeout<> in Process::RunThreadPlan
labath created this revision. labath added a reviewer: jingham. labath added a subscriber: lldb-commits. Since the function is way too big already, I tried at least to factor out the timeout computation stuff into a separate function. I've tried to make the new code semantically equivalent, and it also makes sense when I look at it as a done deal, but I could use another pair of eyes for this. https://reviews.llvm.org/D27258 Files: source/Target/Process.cpp Index: source/Target/Process.cpp === --- source/Target/Process.cpp +++ source/Target/Process.cpp @@ -13,6 +13,7 @@ #include // Other libraries and framework includes +#include "llvm/Support/ScopedPrinter.h" // Project includes #include "Plugins/Process/Utility/InferiorCallPOSIX.h" #include "lldb/Breakpoint/BreakpointLocation.h" @@ -4799,6 +4800,45 @@ }; } // anonymous namespace +static microseconds +GetOneThreadExpressionTimeout(const EvaluateExpressionOptions &options) { + const milliseconds default_one_thread_timeout(250); + + // If the overall wait is forever, then we don't need to worry about it. + if (options.GetTimeoutUsec() == 0) { +if (options.GetOneThreadTimeoutUsec() != 0) + return microseconds(options.GetOneThreadTimeoutUsec()); +return default_one_thread_timeout; + } + + // If the one thread timeout is set, use it. + if (options.GetOneThreadTimeoutUsec() != 0) +return microseconds(options.GetOneThreadTimeoutUsec()); + + // Otherwise use half the total timeout, bounded by the + // default_one_thread_timeout. + return std::min(default_one_thread_timeout, +microseconds(options.GetTimeoutUsec()) / 2); +} + +static Timeout +GetExpressionTimeout(const EvaluateExpressionOptions &options, + bool before_first_timeout) { + // If we are going to run all threads the whole time, or if we are only + // going to run one thread, we can just return the overall timeout. + if (!options.GetStopOthers() || !options.GetTryAllThreads()) +return ConvertTimeout(microseconds(options.GetTimeoutUsec())); + + if (before_first_timeout) +return GetOneThreadExpressionTimeout(options); + + if (options.GetTimeoutUsec() == 0) +return llvm::None; + else +return microseconds(options.GetTimeoutUsec()) - + GetOneThreadExpressionTimeout(options); +} + ExpressionResults Process::RunThreadPlan(ExecutionContext &exe_ctx, lldb::ThreadPlanSP &thread_plan_sp, @@ -4879,6 +4919,16 @@ } } + // Make sure the timeout values make sense. The one thread timeout needs to be + // smaller than the overall timeout. + if (options.GetOneThreadTimeoutUsec() != 0 && options.GetTimeoutUsec() != 0 && + options.GetTimeoutUsec() < options.GetOneThreadTimeoutUsec()) { +diagnostic_manager.PutString(eDiagnosticSeverityError, + "RunThreadPlan called without one thread " + "timeout greater than total timeout"); +return eExpressionSetupError; + } + StackID ctx_frame_id = selected_frame_sp->GetStackID(); // N.B. Running the target may unset the currently selected thread and frame. @@ -4985,67 +5035,20 @@ // that we have to halt the target. bool do_resume = true; bool handle_running_event = true; -const uint64_t default_one_thread_timeout_usec = 25; // This is just for accounting: uint32_t num_resumes = 0; -uint32_t timeout_usec = options.GetTimeoutUsec(); -uint32_t one_thread_timeout_usec; -uint32_t all_threads_timeout_usec = 0; - // If we are going to run all threads the whole time, or if we are only -// going to run one thread, -// then we don't need the first timeout. So we set the final timeout, and -// pretend we are after the -// first timeout already. - -if (!options.GetStopOthers() || !options.GetTryAllThreads()) { +// going to run one thread, then we don't need the first timeout. So we +// pretend we are after the first timeout already. +if (!options.GetStopOthers() || !options.GetTryAllThreads()) before_first_timeout = false; - one_thread_timeout_usec = 0; - all_threads_timeout_usec = timeout_usec; -} else { - uint32_t option_one_thread_timeout = options.GetOneThreadTimeoutUsec(); - - // If the overall wait is forever, then we only need to set the one thread - // timeout: - if (timeout_usec == 0) { -if (option_one_thread_timeout != 0) - one_thread_timeout_usec = option_one_thread_timeout; -else - one_thread_timeout_usec = default_one_thread_timeout_usec; - } else { -// Otherwise, if the one thread timeout is set, make sure it isn't -// longer than the overall timeout, -// and use it, otherwise use half the total timeout, bounded by the -// default_one_thread_ti