From 6d84163c9be1bf45e35acff8b799279feef327b0 Mon Sep 17 00:00:00 2001
From: Junru Shao <junrushao1994@gmail.com>
Date: Wed, 20 May 2020 20:01:15 -0700
Subject: [PATCH] [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<dmlc::LogMessageFatal>(                           \
-      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<dmlc::LogMessageFatal>(                     \
-      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 <dmlc/logging.h>
+#include <gtest/gtest.h>
+
+TEST(LoggingThrow, exception) {
+  EXPECT_THROW({
+    LOG(FATAL) << "message";
+  }, dmlc::Error);
+}