From 3c2cea782ee8b07e72ab2290a9941c420e9d7fb1 Mon Sep 17 00:00:00 2001 From: Junru Shao Date: Tue, 19 May 2020 19:42:18 -0700 Subject: [PATCH 1/3] Move dmlc::LogMessageFatal to the heap --- include/dmlc/logging.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/include/dmlc/logging.h b/include/dmlc/logging.h index c993fc007b..bed5d55070 100644 --- a/include/dmlc/logging.h +++ b/include/dmlc/logging.h @@ -205,7 +205,8 @@ class LogCheckError { #define CHECK_BINARY_OP(name, op, x, y) \ if (dmlc::LogCheckError _check_err = dmlc::LogCheck##name(x, y)) \ - dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ + std::unique_ptr( \ + new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ << "Check failed: " << #x " " #op " " #y << *(_check_err.str) << ": " #pragma GCC diagnostic push @@ -219,9 +220,10 @@ DEFINE_CHECK_FUNC(_NE, !=) #pragma GCC diagnostic pop // Always-on checking -#define CHECK(x) \ - if (!(x)) \ - dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ +#define CHECK(x) \ + if (!(x)) \ + std::unique_ptr( \ + new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ << "Check failed: " #x << ": " #define CHECK_LT(x, y) CHECK_BINARY_OP(_LT, <, x, y) #define CHECK_GT(x, y) CHECK_BINARY_OP(_GT, >, x, y) From 6d84163c9be1bf45e35acff8b799279feef327b0 Mon Sep 17 00:00:00 2001 From: Junru Shao Date: Wed, 20 May 2020 20:01:15 -0700 Subject: [PATCH 2/3] [Logging] Add `DMLC_NO_INLINE` to `LogMessageFatal` --- include/dmlc/base.h | 6 +++ include/dmlc/logging.h | 58 +++++++++++++++---------- test/unittest/unittest_logging_throw.cc | 11 +++++ 3 files changed, 51 insertions(+), 24 deletions(-) create mode 100644 test/unittest/unittest_logging_throw.cc diff --git a/include/dmlc/base.h b/include/dmlc/base.h index 5bac93476c..9125ed4be6 100644 --- a/include/dmlc/base.h +++ b/include/dmlc/base.h @@ -236,6 +236,12 @@ typedef unsigned __int64 uint64_t; #define noexcept(a) noexcept_##a #endif +#if defined(_MSC_VER) +#define DMLC_NO_INLINE __declspec(noinline) +#else +#define DMLC_NO_INLINE __attribute__((noinline)) +#endif + #if DMLC_USE_CXX11 #define DMLC_THROW_EXCEPTION noexcept(false) #define DMLC_NO_EXCEPTION noexcept(true) diff --git a/include/dmlc/logging.h b/include/dmlc/logging.h index bed5d55070..fbdf7f14fe 100644 --- a/include/dmlc/logging.h +++ b/include/dmlc/logging.h @@ -205,8 +205,7 @@ class LogCheckError { #define CHECK_BINARY_OP(name, op, x, y) \ if (dmlc::LogCheckError _check_err = dmlc::LogCheck##name(x, y)) \ - std::unique_ptr( \ - new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ + dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ << "Check failed: " << #x " " #op " " #y << *(_check_err.str) << ": " #pragma GCC diagnostic push @@ -220,10 +219,9 @@ DEFINE_CHECK_FUNC(_NE, !=) #pragma GCC diagnostic pop // Always-on checking -#define CHECK(x) \ - if (!(x)) \ - std::unique_ptr( \ - new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ +#define CHECK(x) \ + if (!(x)) \ + dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ << "Check failed: " #x << ": " #define CHECK_LT(x, y) CHECK_BINARY_OP(_LT, <, x, y) #define CHECK_GT(x, y) CHECK_BINARY_OP(_GT, >, x, y) @@ -428,30 +426,42 @@ class LogMessageFatal : public LogMessage { #else class LogMessageFatal { public: - LogMessageFatal(const char* file, int line) { - log_stream_ << "[" << pretty_date_.HumanDate() << "] " << file << ":" - << line << ": "; + LogMessageFatal(const char *file, int line) { + Entry::ThreadLocal()->Init(file, line); } - std::ostringstream &stream() { return log_stream_; } - ~LogMessageFatal() DMLC_THROW_EXCEPTION { + std::ostringstream &stream() { return Entry::ThreadLocal()->log_stream; } + DMLC_NO_INLINE ~LogMessageFatal() DMLC_THROW_EXCEPTION { #if DMLC_LOG_STACK_TRACE - log_stream_ << "\n" << StackTrace(1, LogStackTraceLevel()) << "\n"; -#endif - - // throwing out of destructor is evil - // hopefully we can do it here - // also log the message before throw -#if DMLC_LOG_BEFORE_THROW - LOG(ERROR) << log_stream_.str(); + Entry::ThreadLocal()->log_stream << "\n" + << StackTrace(1, LogStackTraceLevel()) + << "\n"; #endif - throw Error(log_stream_.str()); + throw Entry::ThreadLocal()->Finalize(); } private: - std::ostringstream log_stream_; - DateLogger pretty_date_; - LogMessageFatal(const LogMessageFatal&); - void operator=(const LogMessageFatal&); + struct Entry { + std::ostringstream log_stream; + DMLC_NO_INLINE void Init(const char *file, int line) { + DateLogger date; + log_stream.str(""); + log_stream.clear(); + log_stream << "[" << date.HumanDate() << "] " << file << ":" << line + << ": "; + } + dmlc::Error Finalize() { +#if DMLC_LOG_BEFORE_THROW + LOG(ERROR) << log_stream.str(); +#endif + return dmlc::Error(log_stream.str()); + } + DMLC_NO_INLINE static Entry *ThreadLocal() { + static thread_local Entry *result = new Entry(); + return result; + } + }; + LogMessageFatal(const LogMessageFatal &); + void operator=(const LogMessageFatal &); }; #endif diff --git a/test/unittest/unittest_logging_throw.cc b/test/unittest/unittest_logging_throw.cc new file mode 100644 index 0000000000..e76efcca14 --- /dev/null +++ b/test/unittest/unittest_logging_throw.cc @@ -0,0 +1,11 @@ +// Copyright by Contributors +#define DMLC_LOG_FATAL_THROW 1 + +#include +#include + +TEST(LoggingThrow, exception) { + EXPECT_THROW({ + LOG(FATAL) << "message"; + }, dmlc::Error); +} From 32acd5a70f4cf3d8e600723d732e3807a6269bd4 Mon Sep 17 00:00:00 2001 From: Junru Shao Date: Wed, 20 May 2020 20:26:58 -0700 Subject: [PATCH 3/3] fix wrongly merged code --- include/dmlc/logging.h | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/include/dmlc/logging.h b/include/dmlc/logging.h index 0226151b2c..fbdf7f14fe 100644 --- a/include/dmlc/logging.h +++ b/include/dmlc/logging.h @@ -205,8 +205,7 @@ class LogCheckError { #define CHECK_BINARY_OP(name, op, x, y) \ if (dmlc::LogCheckError _check_err = dmlc::LogCheck##name(x, y)) \ - std::unique_ptr( \ - new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ + dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ << "Check failed: " << #x " " #op " " #y << *(_check_err.str) << ": " #pragma GCC diagnostic push @@ -220,10 +219,9 @@ DEFINE_CHECK_FUNC(_NE, !=) #pragma GCC diagnostic pop // Always-on checking -#define CHECK(x) \ - if (!(x)) \ - std::unique_ptr( \ - new dmlc::LogMessageFatal(__FILE__, __LINE__))->stream() \ +#define CHECK(x) \ + if (!(x)) \ + dmlc::LogMessageFatal(__FILE__, __LINE__).stream() \ << "Check failed: " #x << ": " #define CHECK_LT(x, y) CHECK_BINARY_OP(_LT, <, x, y) #define CHECK_GT(x, y) CHECK_BINARY_OP(_GT, >, x, y)