From db38fc9afbee4f61cee88f22713da154ac21d500 Mon Sep 17 00:00:00 2001 From: Matthew Kotila Date: Mon, 13 Jun 2022 06:26:35 -0700 Subject: [PATCH] Exit with error on failed stabilization (#114) * Add failed stabilization error exit * Addressed comments --- .../client_backend/client_backend.cc | 15 +++- .../client_backend/client_backend.h | 12 ++- src/c++/perf_analyzer/inference_profiler.cc | 81 ++++++++++--------- src/c++/perf_analyzer/inference_profiler.h | 27 ++++--- src/c++/perf_analyzer/perf_analyzer.cc | 2 +- 5 files changed, 83 insertions(+), 54 deletions(-) diff --git a/src/c++/perf_analyzer/client_backend/client_backend.cc b/src/c++/perf_analyzer/client_backend/client_backend.cc index 6796fe9dc..1f91f54f8 100644 --- a/src/c++/perf_analyzer/client_backend/client_backend.cc +++ b/src/c++/perf_analyzer/client_backend/client_backend.cc @@ -44,15 +44,24 @@ namespace triton { namespace perfanalyzer { namespace clientbackend { //================================================ -const Error Error::Success(""); +const Error Error::Success; +const Error Error::Failure(""); -Error::Error(const std::string& msg) : msg_(msg) {} +Error::Error() +{ + has_error_ = false; +} + +Error::Error(const std::string& msg) : msg_(msg) +{ + has_error_ = true; +} std::ostream& operator<<(std::ostream& out, const Error& err) { if (!err.msg_.empty()) { - out << err.msg_; + out << err.msg_ << std::endl; } return out; } diff --git a/src/c++/perf_analyzer/client_backend/client_backend.h b/src/c++/perf_analyzer/client_backend/client_backend.h index ae5316914..d96f37207 100644 --- a/src/c++/perf_analyzer/client_backend/client_backend.h +++ b/src/c++/perf_analyzer/client_backend/client_backend.h @@ -69,9 +69,12 @@ namespace triton { namespace perfanalyzer { namespace clientbackend { /// class Error { public: + /// Create an error + explicit Error(); + /// Create an error with the specified message. /// \param msg The message for the error - explicit Error(const std::string& msg = ""); + explicit Error(const std::string& msg); /// Accessor for the message of this error. /// \return The messsage for the error. Empty if no error. @@ -80,15 +83,20 @@ class Error { /// Does this error indicate OK status? /// \return True if this error indicates "ok"/"success", false if /// error indicates a failure. - bool IsOk() const { return msg_.empty(); } + bool IsOk() const { return !has_error_; } /// Convenience "success" value. Can be used as Error::Success to /// indicate no error. static const Error Success; + /// Convenience "failure" value. Can be used as Error::Failure to + /// indicate a generic error. + static const Error Failure; + private: friend std::ostream& operator<<(std::ostream&, const Error&); std::string msg_; + bool has_error_; }; //=================================================================================== diff --git a/src/c++/perf_analyzer/inference_profiler.cc b/src/c++/perf_analyzer/inference_profiler.cc index b42ef41dc..dea52aca5 100644 --- a/src/c++/perf_analyzer/inference_profiler.cc +++ b/src/c++/perf_analyzer/inference_profiler.cc @@ -427,36 +427,29 @@ InferenceProfiler::InferenceProfiler( cb::Error InferenceProfiler::Profile( const size_t concurrent_request_count, std::vector& summary, - bool* meets_threshold) + bool& meets_threshold, bool& is_stable) { cb::Error err; PerfStatus status_summary; status_summary.concurrency = concurrent_request_count; - bool is_stable = false; - *meets_threshold = true; + is_stable = false; + meets_threshold = true; RETURN_IF_ERROR(dynamic_cast(manager_.get()) ->ChangeConcurrencyLevel(concurrent_request_count)); err = ProfileHelper(false /* clean_starts */, status_summary, &is_stable); if (err.IsOk()) { - err = Report( - status_summary, percentile_, protocol_, verbose_, include_lib_stats_, - include_server_stats_, parser_); summary.push_back(status_summary); uint64_t stabilizing_latency_ms = status_summary.stabilizing_latency_ns / (1000 * 1000); - if (!err.IsOk()) { - std::cerr << err << std::endl; - *meets_threshold = false; - } else if ( - (stabilizing_latency_ms >= latency_threshold_ms_) && + if ((stabilizing_latency_ms >= latency_threshold_ms_) && (latency_threshold_ms_ != NO_LIMIT)) { std::cerr << "Measured latency went over the set limit of " << latency_threshold_ms_ << " msec. " << std::endl; - *meets_threshold = false; + meets_threshold = false; } else if (!is_stable) { if (measurement_mode_ == MeasurementMode::TIME_WINDOWS) { std::cerr << "Failed to obtain stable measurement within " @@ -469,7 +462,15 @@ InferenceProfiler::Profile( << concurrent_request_count << ". Please try to " << "increase the --measurement-request-count." << std::endl; } - *meets_threshold = false; + meets_threshold = false; + } else { + err = Report( + status_summary, percentile_, protocol_, verbose_, include_lib_stats_, + include_server_stats_, parser_); + if (!err.IsOk()) { + std::cerr << err; + meets_threshold = false; + } } } else { return err; @@ -481,39 +482,40 @@ InferenceProfiler::Profile( cb::Error InferenceProfiler::Profile( const double request_rate, std::vector& summary, - bool* meets_threshold) + bool& meets_threshold, bool& is_stable) { cb::Error err; PerfStatus status_summary; status_summary.request_rate = request_rate; - bool is_stable = false; - *meets_threshold = true; + is_stable = false; + meets_threshold = true; RETURN_IF_ERROR(dynamic_cast(manager_.get()) ->ChangeRequestRate(request_rate)); err = ProfileHelper(false /*clean_starts*/, status_summary, &is_stable); if (err.IsOk()) { - err = Report( - status_summary, percentile_, protocol_, verbose_, include_lib_stats_, - include_server_stats_, parser_); summary.push_back(status_summary); uint64_t stabilizing_latency_ms = status_summary.stabilizing_latency_ns / (1000 * 1000); - if (!err.IsOk()) { - std::cerr << err << std::endl; - *meets_threshold = false; - } else if ( - (stabilizing_latency_ms >= latency_threshold_ms_) && + if ((stabilizing_latency_ms >= latency_threshold_ms_) && (latency_threshold_ms_ != NO_LIMIT)) { std::cerr << "Measured latency went over the set limit of " << latency_threshold_ms_ << " msec. " << std::endl; - *meets_threshold = false; + meets_threshold = false; } else if (!is_stable) { std::cerr << "Failed to obtain stable measurement." << std::endl; - *meets_threshold = false; + meets_threshold = false; + } else { + err = Report( + status_summary, percentile_, protocol_, verbose_, include_lib_stats_, + include_server_stats_, parser_); + if (!err.IsOk()) { + std::cerr << err; + meets_threshold = false; + } } } else { return err; @@ -524,7 +526,7 @@ InferenceProfiler::Profile( cb::Error InferenceProfiler::Profile( - std::vector& summary, bool* meets_threshold) + std::vector& summary, bool& meets_threshold, bool& is_stable) { cb::Error err; PerfStatus status_summary; @@ -534,29 +536,30 @@ InferenceProfiler::Profile( RETURN_IF_ERROR(dynamic_cast(manager_.get()) ->GetCustomRequestRate(&status_summary.request_rate)); - bool is_stable = false; - *meets_threshold = true; + is_stable = false; + meets_threshold = true; err = ProfileHelper(true /* clean_starts */, status_summary, &is_stable); if (err.IsOk()) { - err = Report( - status_summary, percentile_, protocol_, verbose_, include_lib_stats_, - include_server_stats_, parser_); summary.push_back(status_summary); uint64_t stabilizing_latency_ms = status_summary.stabilizing_latency_ns / (1000 * 1000); - if (!err.IsOk()) { - std::cerr << err << std::endl; - *meets_threshold = false; - } else if ( - (stabilizing_latency_ms >= latency_threshold_ms_) && + if ((stabilizing_latency_ms >= latency_threshold_ms_) && (latency_threshold_ms_ != NO_LIMIT)) { std::cerr << "Measured latency went over the set limit of " << latency_threshold_ms_ << " msec. " << std::endl; - *meets_threshold = false; + meets_threshold = false; } else if (!is_stable) { std::cerr << "Failed to obtain stable measurement." << std::endl; - *meets_threshold = false; + meets_threshold = false; + } else { + err = Report( + status_summary, percentile_, protocol_, verbose_, include_lib_stats_, + include_server_stats_, parser_); + if (!err.IsOk()) { + std::cerr << err; + meets_threshold = false; + } } } else { return err; diff --git a/src/c++/perf_analyzer/inference_profiler.h b/src/c++/perf_analyzer/inference_profiler.h index 94d710208..c151460ae 100644 --- a/src/c++/perf_analyzer/inference_profiler.h +++ b/src/c++/perf_analyzer/inference_profiler.h @@ -228,28 +228,33 @@ class InferenceProfiler { std::vector& summary) { cb::Error err; - bool meets_threshold; + bool meets_threshold, is_stable; if (search_mode == SearchMode::NONE) { - err = Profile(summary, &meets_threshold); + err = Profile(summary, meets_threshold, is_stable); if (!err.IsOk()) { return err; } } else if (search_mode == SearchMode::LINEAR) { T current_value = start; do { - err = Profile(current_value, summary, &meets_threshold); + err = Profile(current_value, summary, meets_threshold, is_stable); if (!err.IsOk()) { return err; } current_value += step; } while (((current_value <= end) || (end == static_cast(NO_LIMIT))) && (meets_threshold)); + // If there was only one concurrency we swept over and it did not meet the + // stability threshold, we should return an error. + if (current_value == (start + step) && is_stable == false) { + return cb::Error::Failure; + } } else { - err = Profile(start, summary, &meets_threshold); + err = Profile(start, summary, meets_threshold, is_stable); if (!err.IsOk() || (!meets_threshold)) { return err; } - err = Profile(end, summary, &meets_threshold); + err = Profile(end, summary, meets_threshold, is_stable); if (!err.IsOk() || (meets_threshold)) { return err; } @@ -258,7 +263,7 @@ class InferenceProfiler { T this_end = end; while ((this_end - this_start) > step) { T current_value = (this_end + this_start) / 2; - err = Profile(current_value, summary, &meets_threshold); + err = Profile(current_value, summary, meets_threshold, is_stable); if (!err.IsOk()) { return err; } @@ -296,29 +301,33 @@ class InferenceProfiler { /// \param concurrent_request_count The concurrency level for the measurement. /// \param summary Appends the measurements summary at the end of this list. /// \param meets_threshold Returns whether the setting meets the threshold. + /// \param is_stable Returns whether the measurement is stable. /// \return cb::Error object indicating success or failure. cb::Error Profile( const size_t concurrent_request_count, std::vector& summary, - bool* meets_threshold); + bool& meets_threshold, bool& is_stable); /// Similar to above function, but instead of setting the concurrency, it /// sets the specified request rate for measurements. /// \param request_rate The request rate for inferences. /// \param summary Appends the measurements summary at the end of this list. /// \param meets_threshold Returns whether the setting meets the threshold. + /// \param is_stable Returns whether the measurement is stable. /// \return cb::Error object indicating success or failure. cb::Error Profile( const double request_rate, std::vector& summary, - bool* meets_threshold); + bool& meets_threshold, bool& is_stable); /// Measures throughput and latencies for custom load without controling /// request rate nor concurrency. Requires load manager to be loaded with /// a file specifying the time intervals. /// \param summary Appends the measurements summary at the end of this list. /// \param meets_threshold Returns whether the measurement met the threshold. + /// \param is_stable Returns whether the measurement is stable. /// \return cb::Error object indicating success /// or failure. - cb::Error Profile(std::vector& summary, bool* meets_threshold); + cb::Error Profile( + std::vector& summary, bool& meets_threshold, bool& is_stable); /// A helper function for profiling functions. /// \param clean_starts Whether or not to reset load cycle with every diff --git a/src/c++/perf_analyzer/perf_analyzer.cc b/src/c++/perf_analyzer/perf_analyzer.cc index 0fe4be6c3..b49173c6e 100644 --- a/src/c++/perf_analyzer/perf_analyzer.cc +++ b/src/c++/perf_analyzer/perf_analyzer.cc @@ -1831,7 +1831,7 @@ PerfAnalyzer::Run(int argc, char** argv) mpi_driver->MPIBarrierWorld(); if (!err.IsOk()) { - std::cerr << err << std::endl; + std::cerr << err; // In the case of early_exit, the thread does not return and continues to // report the summary if (!pa::early_exit) {