From 697bfb14db6b897ea21aaa07fb35c8a58734e587 Mon Sep 17 00:00:00 2001 From: Yuhong Guo Date: Sat, 25 Aug 2018 07:30:51 +0800 Subject: [PATCH] Hotfix for glog PR (#2734) --- src/ray/util/logging.cc | 11 ++++++++--- src/ray/util/logging.h | 32 ++++++++++++++++++++------------ src/ray/util/logging_test.cc | 25 +++++++++++++++++-------- 3 files changed, 45 insertions(+), 23 deletions(-) diff --git a/src/ray/util/logging.cc b/src/ray/util/logging.cc index 76663fb46..5a4066a11 100644 --- a/src/ray/util/logging.cc +++ b/src/ray/util/logging.cc @@ -114,10 +114,11 @@ void RayLog::ShutDownRayLog() { #endif } -RayLog::RayLog(const char *file_name, int line_number, int severity) { +RayLog::RayLog(const char *file_name, int line_number, int severity) + // glog does not have DEBUG level, we can handle it here. + : is_enabled_(severity >= severity_threshold_) { #ifdef RAY_USE_GLOG - // glog does not have DEBUG level, we can handle it here. - if (severity >= severity_threshold_) { + if (is_enabled_) { logging_provider_.reset( new google::LogMessage(file_name, line_number, GetMappedSeverity(severity))); } @@ -129,12 +130,16 @@ RayLog::RayLog(const char *file_name, int line_number, int severity) { std::ostream &RayLog::Stream() { #ifdef RAY_USE_GLOG + // Before calling this function, user should check IsEnabled. + // When IsEnabled == false, logging_provider_ will be empty. return logging_provider_->stream(); #else return logging_provider_->Stream(); #endif } +bool RayLog::IsEnabled() const { return is_enabled_; } + RayLog::~RayLog() { logging_provider_.reset(); } } // namespace ray diff --git a/src/ray/util/logging.h b/src/ray/util/logging.h index 3881d2cd7..323ae5201 100644 --- a/src/ray/util/logging.h +++ b/src/ray/util/logging.h @@ -65,28 +65,32 @@ class RayLogBase { public: virtual ~RayLogBase(){}; + virtual bool IsEnabled() const { return false; }; + template RayLogBase &operator<<(const T &t) { - RAY_IGNORE_EXPR(t); + if (IsEnabled()) { + Stream() << t; + } else { + RAY_IGNORE_EXPR(t); + } return *this; } + + protected: + virtual std::ostream &Stream() { return std::cerr; }; }; class RayLog : public RayLogBase { public: RayLog(const char *file_name, int line_number, int severity); + virtual ~RayLog(); - template - RayLogBase &operator<<(const T &t) { - if (logging_provider_ == nullptr) { - // This means the logging level is lower than the threshold. - RAY_IGNORE_EXPR(t); - } else { - this->Stream() << t; - } - return *this; - } + /// Return whether or not logging is enabled. + /// + /// \return True if logging is enabled and false otherwise. + virtual bool IsEnabled() const; // The init function of ray log for a program which should be called only once. // If logDir is empty, the log won't output to file. @@ -96,9 +100,13 @@ class RayLog : public RayLogBase { static void ShutDownRayLog(); private: - std::ostream &Stream(); std::unique_ptr logging_provider_; + /// True if log messages should be logged and false if they should be ignored. + bool is_enabled_; static int severity_threshold_; + + protected: + virtual std::ostream &Stream(); }; // This class make RAY_CHECK compilation pass to change the << operator to void. diff --git a/src/ray/util/logging_test.cc b/src/ray/util/logging_test.cc index d48c4b6cb..a27a06250 100644 --- a/src/ray/util/logging_test.cc +++ b/src/ray/util/logging_test.cc @@ -18,11 +18,17 @@ int64_t current_time_ms() { // This file just print some information using the logging macro. void PrintLog() { - RAY_LOG(DEBUG) << "This is the DEBUG message"; - RAY_LOG(INFO) << "This is the INFO message"; - RAY_LOG(WARNING) << "This is the WARNING message"; - RAY_LOG(ERROR) << "This is the ERROR message"; - RAY_CHECK(true) << "This is a RAY_CHECK message but it won't show up"; + RAY_LOG(DEBUG) << "This is the" + << " DEBUG" + << " message"; + RAY_LOG(INFO) << "This is the" + << " INFO message"; + RAY_LOG(WARNING) << "This is the" + << " WARNING message"; + RAY_LOG(ERROR) << "This is the" + << " ERROR message"; + RAY_CHECK(true) << "This is a RAY_CHECK" + << " message but it won't show up"; // The following 2 lines should not run since it will cause program failure. // RAY_LOG(FATAL) << "This is the FATAL message"; // RAY_CHECK(false) << "This is a RAY_CHECK message but it won't show up"; @@ -47,7 +53,8 @@ TEST(LogPerfTest, PerfTest) { int64_t start_time = current_time_ms(); for (int i = 0; i < rounds; ++i) { - RAY_LOG(DEBUG) << "This is the RAY_DEBUG message"; + RAY_LOG(DEBUG) << "This is the " + << "RAY_DEBUG message"; } int64_t elapsed = current_time_ms() - start_time; std::cout << "Testing DEBUG log for " << rounds << " rounds takes " << elapsed << " ms." @@ -55,7 +62,8 @@ TEST(LogPerfTest, PerfTest) { start_time = current_time_ms(); for (int i = 0; i < rounds; ++i) { - RAY_LOG(ERROR) << "This is the RAY_ERROR message"; + RAY_LOG(ERROR) << "This is the " + << "RAY_ERROR message"; } elapsed = current_time_ms() - start_time; std::cout << "Testing RAY_ERROR log for " << rounds << " rounds takes " << elapsed @@ -63,7 +71,8 @@ TEST(LogPerfTest, PerfTest) { start_time = current_time_ms(); for (int i = 0; i < rounds; ++i) { - RAY_CHECK(i >= 0) << "This is a RAY_CHECK message but it won't show up"; + RAY_CHECK(i >= 0) << "This is a RAY_CHECK " + << "message but it won't show up"; } elapsed = current_time_ms() - start_time; std::cout << "Testing RAY_CHECK(true) for " << rounds << " rounds takes " << elapsed