always log from LOG_EVERY_MS if interval is <= 0

Summary:
This diff changes LOG_EVERY_MS so that if the specified
interval is zero or negative, no clock call is made and LOG(severity) is
always called.  milli_interval is copied into a temporary to avoid
double-evaluation.

Test Plan:
1. unit tests
2. new unit test

Reviewed By: lesha@fb.com

FB internal diff: D1469366

Tasks: 4813858

diff --git a/folly/Logging.h b/folly/Logging.h
index 0c34de24b8e18511ed582092e85bfd01194c7772..1a9aeb4f7db92ee3fc8cbef075845b790c656c5c 100644 (file)
--- a/folly/Logging.h
+++ b/folly/Logging.h
@@ -31,21 +31,25 @@
  *
  * The implementation uses for statements to introduce variables in
  * a nice way that doesn't mess surrounding statements.  It is thread
- * safe.
+ * safe.  Non-positive intervals will always log.
  */
 #define FB_LOG_EVERY_MS(severity, milli_interval)                            \
-  for (bool FB_LEM_once = true; FB_LEM_once; )                               \
+  for (decltype(milli_interval) FB_LEM_once = 1,                             \
+                                FB_LEM_interval = (milli_interval);          \
+       FB_LEM_once; )                                                        \
     for (::std::chrono::milliseconds::rep FB_LEM_prev, FB_LEM_now =          \
+             FB_LEM_interval <= 0 ? 0 :                                      \
              ::std::chrono::duration_cast< ::std::chrono::milliseconds>(     \
                  ::std::chrono::system_clock::now().time_since_epoch()       \
                  ).count();                                                  \
          FB_LEM_once; )                                                      \
       for (static ::std::atomic< ::std::chrono::milliseconds::rep>           \
-               FB_LEM_hist; FB_LEM_once; FB_LEM_once = false)                \
-        if (FB_LEM_now - (FB_LEM_prev =                                      \
-                          FB_LEM_hist.load(std::memory_order_acquire)) <     \
-                milli_interval ||                                            \
-            !FB_LEM_hist.compare_exchange_strong(FB_LEM_prev, FB_LEM_now)) { \
+               FB_LEM_hist; FB_LEM_once; FB_LEM_once = 0)                    \
+        if (FB_LEM_interval > 0 &&                                           \
+            (FB_LEM_now - (FB_LEM_prev =                                     \
+                           FB_LEM_hist.load(std::memory_order_acquire)) <    \
+                                                          FB_LEM_interval || \
+             !FB_LEM_hist.compare_exchange_strong(FB_LEM_prev,FB_LEM_now))) {\
         } else                                                               \
           LOG(severity)
 

diff --git a/folly/test/LoggingTest.cpp b/folly/test/LoggingTest.cpp
index 19d2341822668dbafcc8f23dd6c32258533e487a..11269253a4bcf60a2573733b9aeb3765ea3fa3ab 100644 (file)
--- a/folly/test/LoggingTest.cpp
+++ b/folly/test/LoggingTest.cpp
@@ -40,6 +40,17 @@ TEST(LogEveryMs, basic) {
   EXPECT_TRUE(atLeastOneIsGood);
 }
 
+TEST(LogEveryMs, zero) {
+  int count = 0;
+
+  for (int i = 0; i < 10; ++i) {
+    FB_LOG_EVERY_MS(INFO, 0)
+      << "test msg " << ++count;
+  }
+
+  EXPECT_EQ(10, count);
+}
+
 BENCHMARK(skip_overhead, iter) {
   auto prev = FLAGS_minloglevel;
   FLAGS_minloglevel = 2;