Remove not-very-useful early checks of __pg_log_level in logging.h.

Enforce __pg_log_level message filtering centrally in logging.c,
instead of relying on the calling macros to do it.  This is more
reliable (e.g. it works correctly for direct calls to pg_log_generic)
and it saves a percent or so of total code size because we get rid of
so many duplicate checks of __pg_log_level.

This does mean that argument expressions in a logging macro will be
evaluated even if we end up not printing anything.  That seems of
little concern for INFO and higher levels as those messages are printed
by default, and most of our frontend programs don't even offer a way to
turn them off.  I left the unlikely() checks in place for DEBUG
messages, though.

Discussion: https://postgr.es/m/3993549.1649449609@sss.pgh.pa.us

diff --git a/src/bin/pg_dump/pg_backup_utils.h b/src/bin/pg_dump/pg_backup_utils.h
index 5b1c51554da0b991d2deaddeff9b8473f5f1fa28..8173bb93cfc0aa28a0d85430d8288c3840e1b404 100644 (file)
--- a/src/bin/pg_dump/pg_backup_utils.h
+++ b/src/bin/pg_dump/pg_backup_utils.h
@@ -34,8 +34,7 @@ extern void exit_nicely(int code) pg_attribute_noreturn();
 /* In pg_dump, we modify pg_fatal to call exit_nicely instead of exit */
 #undef pg_fatal
 #define pg_fatal(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_ERROR)) \
-                       pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
+               pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
                exit_nicely(1); \
        } while(0)
 

diff --git a/src/common/logging.c b/src/common/logging.c
index 2933cab85c8bdc7c705cba85462a64561fcb90a0..0b5bcb1a17256948a727f92a072fc1448d1b85f4 100644 (file)
--- a/src/common/logging.c
+++ b/src/common/logging.c
@@ -228,6 +228,10 @@ pg_log_generic_v(enum pg_log_level level, enum pg_log_part part,
        Assert(fmt);
        Assert(fmt[strlen(fmt) - 1] != '\n');
 
+       /* Do nothing if log level is too low. */
+       if (level < __pg_log_level)
+               return;
+
        /*
         * Flush stdout before output to stderr, to ensure sync even when stdout
         * is buffered.

diff --git a/src/include/common/logging.h b/src/include/common/logging.h
index e213bb70d0a68cad39d268a1f57a32a934d7da12..9f426c32d61259332e1a67599d3c6feb50df456b 100644 (file)
--- a/src/include/common/logging.h
+++ b/src/include/common/logging.h
@@ -103,50 +103,32 @@ void              pg_log_generic_v(enum pg_log_level level, enum pg_log_part part,
  * Preferred style is to use these macros to perform logging; don't call
  * pg_log_generic[_v] directly, except perhaps in error interface code.
  */
-#define pg_log_error(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_ERROR)) \
-                       pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
-       } while(0)
+#define pg_log_error(...) \
+       pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__)
 
-#define pg_log_error_detail(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_ERROR)) \
-                       pg_log_generic(PG_LOG_ERROR, PG_LOG_DETAIL, __VA_ARGS__); \
-       } while(0)
+#define pg_log_error_detail(...) \
+       pg_log_generic(PG_LOG_ERROR, PG_LOG_DETAIL, __VA_ARGS__)
 
-#define pg_log_error_hint(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_ERROR)) \
-                       pg_log_generic(PG_LOG_ERROR, PG_LOG_HINT, __VA_ARGS__); \
-       } while(0)
+#define pg_log_error_hint(...) \
+       pg_log_generic(PG_LOG_ERROR, PG_LOG_HINT, __VA_ARGS__)
 
-#define pg_log_warning(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_WARNING)) \
-                       pg_log_generic(PG_LOG_WARNING, PG_LOG_PRIMARY, __VA_ARGS__); \
-       } while(0)
+#define pg_log_warning(...) \
+       pg_log_generic(PG_LOG_WARNING, PG_LOG_PRIMARY, __VA_ARGS__)
 
-#define pg_log_warning_detail(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_WARNING)) \
-                       pg_log_generic(PG_LOG_WARNING, PG_LOG_DETAIL, __VA_ARGS__); \
-       } while(0)
+#define pg_log_warning_detail(...) \
+       pg_log_generic(PG_LOG_WARNING, PG_LOG_DETAIL, __VA_ARGS__)
 
-#define pg_log_warning_hint(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_WARNING)) \
-                       pg_log_generic(PG_LOG_WARNING, PG_LOG_HINT, __VA_ARGS__); \
-       } while(0)
+#define pg_log_warning_hint(...) \
+       pg_log_generic(PG_LOG_WARNING, PG_LOG_HINT, __VA_ARGS__)
 
-#define pg_log_info(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_INFO)) \
-                       pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__); \
-       } while(0)
+#define pg_log_info(...) \
+       pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__)
 
-#define pg_log_info_detail(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_INFO)) \
-                       pg_log_generic(PG_LOG_INFO, PG_LOG_DETAIL, __VA_ARGS__); \
-       } while(0)
+#define pg_log_info_detail(...) \
+       pg_log_generic(PG_LOG_INFO, PG_LOG_DETAIL, __VA_ARGS__)
 
-#define pg_log_info_hint(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_INFO)) \
-                       pg_log_generic(PG_LOG_INFO, PG_LOG_HINT, __VA_ARGS__); \
-       } while(0)
+#define pg_log_info_hint(...) \
+       pg_log_generic(PG_LOG_INFO, PG_LOG_HINT, __VA_ARGS__)
 
 #define pg_log_debug(...) do { \
                if (unlikely(__pg_log_level <= PG_LOG_DEBUG)) \
@@ -167,8 +149,7 @@ void                pg_log_generic_v(enum pg_log_level level, enum pg_log_part part,
  * A common shortcut: pg_log_error() and immediately exit(1).
  */
 #define pg_fatal(...) do { \
-               if (likely(__pg_log_level <= PG_LOG_ERROR)) \
-                       pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
+               pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
                exit(1); \
        } while(0)