Merge pull request #226 from sergiud/zero-allocation-log-failure

Zero allocation fix
google · Oct 11, 2017 · d053142 · d053142
2 parents 2a6df66 + 2fe6508
commit d053142
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 29 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -192,6 +192,11 @@ using namespace Outer::Inner;;
 int main() { return i; }
 " HAVE_NAMESPACES)
 
+check_cxx_source_compiles ("
+__thread int tls;
+int main() { }
+" HAVE_GCC_TLS)
+
 check_cxx_source_compiles ("
 __declspec(thread) int tls;
 int main() { }
@@ -202,19 +207,15 @@ thread_local int tls;
 int main() { }
 " HAVE_CXX11_TLS)
 
-check_cxx_source_compiles ("
-__attribute__((thread)) int tls;
-int main() { }
-" HAVE_CYGWIN_TLS)
-
 if (WITH_TLS)
-  if (HAVE_CYGWIN_TLS)
-    set (GLOG_THREAD_LOCAL_STORAGE "__attribute__((thread))")
+  # Cygwin does not support the thread attribute. Don't bother.
+  if (HAVE_GCC_TLS)
+    set (GLOG_THREAD_LOCAL_STORAGE "__thread")
   elseif (HAVE_MSVC_TLS)
     set (GLOG_THREAD_LOCAL_STORAGE "__declspec(thread)")
   elseif (HAVE_CXX11_TLS)
     set (GLOG_THREAD_LOCAL_STORAGE thread_local)
-  endif (HAVE_CYGWIN_TLS)
+  endif (HAVE_GCC_TLS)
 endif (WITH_TLS)
 
 set (_PC_FIELDS

diff --git a/src/glog/logging.h.in b/src/glog/logging.h.in
@@ -1109,11 +1109,6 @@ class GOOGLE_GLOG_DLL_DECL LogStreamBuf : public std::streambuf {
     setp(buf, buf + len - 2);
   }
 
-  // Resets the buffer. Useful if we reuse it by means of TLS.
-  void reset() {
-     setp(pbase(), epptr());
-  }
-
   // This effectively ignores overflow.
   virtual int_type overflow(int_type ch) {
     return ch;
@@ -1176,7 +1171,6 @@ public:
     size_t pcount() const { return streambuf_.pcount(); }
     char* pbase() const { return streambuf_.pbase(); }
     char* str() const { return pbase(); }
-    void reset() { streambuf_.reset(); }
 
   private:
     LogStream(const LogStream&);

diff --git a/src/logging.cc b/src/logging.cc
@@ -331,7 +331,6 @@ const size_t LogMessage::kMaxLogMessageLen = 30000;
 
 struct LogMessage::LogMessageData  {
   LogMessageData();
-  void reset();
 
   int preserved_errno_;      // preserved errno
   // Buffer space; contains complete message text.
@@ -1151,17 +1150,13 @@ static LogMessage::LogMessageData fatal_msg_data_shared;
 // LogMessageData object exists (in this case glog makes zero heap memory
 // allocations).
 static GLOG_THREAD_LOCAL_STORAGE bool thread_data_available = true;
-static GLOG_THREAD_LOCAL_STORAGE LogMessage::LogMessageData thread_msg_data;
+static GLOG_THREAD_LOCAL_STORAGE char thread_msg_data[sizeof(LogMessage::LogMessageData)];
 #endif // defined(GLOG_THREAD_LOCAL_STORAGE)
 
 LogMessage::LogMessageData::LogMessageData()
   : stream_(message_text_, LogMessage::kMaxLogMessageLen, 0) {
 }
 
-void LogMessage::LogMessageData::reset() {
-    stream_.reset();
-}
-
 LogMessage::LogMessage(const char* file, int line, LogSeverity severity,
                        int ctr, void (LogMessage::*send_method)())
     : allocated_(NULL) {
@@ -1218,10 +1213,7 @@ void LogMessage::Init(const char* file,
     // No need for locking, because this is thread local.
     if (thread_data_available) {
       thread_data_available = false;
-      data_ = &thread_msg_data;
-      // Make sure to clear log data since it may have been used and filled with
-      // data. We do not want to append the new message to the previous one.
-      data_->reset();
+      data_ = new (&thread_msg_data) LogMessageData;
     } else {
       allocated_ = new LogMessageData();
       data_ = allocated_;
@@ -1299,10 +1291,16 @@ void LogMessage::Init(const char* file,
 LogMessage::~LogMessage() {
   Flush();
 #ifdef GLOG_THREAD_LOCAL_STORAGE
-  if (data_ == &thread_msg_data)
+  if (data_ == static_cast<void*>(thread_msg_data)) {
+    data_->~LogMessageData();
     thread_data_available = true;
-#endif // defined(GLOG_THREAD_LOCAL_STORAGE)
+  }
+  else {
+    delete allocated_;
+  }
+#else // !defined(GLOG_THREAD_LOCAL_STORAGE)
   delete allocated_;
+#endif // defined(GLOG_THREAD_LOCAL_STORAGE)
 }
 
 int LogMessage::preserved_errno() const {

diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc
@@ -272,7 +272,12 @@ void TestLogging(bool check_counts) {
   LOG(ERROR) << string("foo") << ' '<< j << ' ' << setw(10) << j << " "
              << setw(1) << hex << j;
 
-  LOG(ERROR) << (&LOG(ERROR) && 0) << " nested LOG";
+  {
+    google::LogMessage outer(__FILE__, __LINE__, google::ERROR);
+    outer.stream() << "outer";
+
+    LOG(ERROR) << "inner";
+  }
 
   LogMessage("foo", LogMessage::kNoLogPrefix, GLOG_INFO).stream() << "no prefix";
 

diff --git a/src/logging_unittest.err b/src/logging_unittest.err
@@ -74,8 +74,8 @@ WDATE TIME__ THREADID logging_unittest.cc:LINE] log_if this
 IDATE TIME__ THREADID logging_unittest.cc:LINE] array
 IDATE TIME__ THREADID logging_unittest.cc:LINE] const array
 EDATE TIME__ THREADID logging_unittest.cc:LINE] foo 1000 0000001000 3e8
-EDATE TIME__ THREADID logging_unittest.cc:LINE] 0 nested LOG
-EDATE TIME__ THREADID logging_unittest.cc:LINE]
+EDATE TIME__ THREADID logging_unittest.cc:LINE] inner
+EDATE TIME__ THREADID logging_unittest.cc:LINE] outer
 no prefix
 IDATE TIME__ THREADID logging_unittest.cc:LINE] RAW: foo bar 10 3.400000
 WDATE TIME__ THREADID logging_unittest.cc:LINE] RAW: array