I wrote:
> One other loose end is bothering me: I stuck with logging.h's
> original choice to put "if (likely())" or "if (unlikely())"
> conditionals into the macros, but I rather suspect that that's
> just a waste. I think we should put a centralized level check
> into logging.c, and get rid of at least the "if (likely())"
> checks, because those are going to succeed approximately 100.0%
> of the time. Maybe there's an argument for keeping the unlikely()
> ones.
Concretely, something like the attached. As a simple check,
I looked at the compiled size of pg_dump. It went from
text data bss dec hex filename
380298 4008 1384 385690 5e29a /home/postgres/testversion/bin/pg_dump
to
text data bss dec hex filename
374954 4008 1384 380346 5cdba src/bin/pg_dump/pg_dump
for a savings of about 5K or 1.5%. Not a huge amount, but
not nothing either, especially considering that the existing
coding isn't buying us anything.
regards, tom lane
diff --git a/src/bin/pg_dump/pg_backup_utils.h b/src/bin/pg_dump/pg_backup_utils.h
index 5b1c51554d..8173bb93cf 100644
--- 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 18d6669f27..8a061f46b4 100644
--- a/src/common/logging.c
+++ b/src/common/logging.c
@@ -223,6 +223,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 e213bb70d0..35c7c7b976 100644
--- a/src/include/common/logging.h
+++ b/src/include/common/logging.h
@@ -104,48 +104,39 @@ void pg_log_generic_v(enum pg_log_level level, enum pg_log_part part,
* 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__); \
+ pg_log_generic(PG_LOG_ERROR, PG_LOG_PRIMARY, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_ERROR, PG_LOG_DETAIL, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_ERROR, PG_LOG_HINT, __VA_ARGS__); \
} while(0)
#define pg_log_warning(...) do { \
- if (likely(__pg_log_level <= PG_LOG_WARNING)) \
- pg_log_generic(PG_LOG_WARNING, PG_LOG_PRIMARY, __VA_ARGS__); \
+ pg_log_generic(PG_LOG_WARNING, PG_LOG_PRIMARY, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_WARNING, PG_LOG_DETAIL, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_WARNING, PG_LOG_HINT, __VA_ARGS__); \
} while(0)
#define pg_log_info(...) do { \
- if (likely(__pg_log_level <= PG_LOG_INFO)) \
- pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__); \
+ pg_log_generic(PG_LOG_INFO, PG_LOG_PRIMARY, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_INFO, PG_LOG_DETAIL, __VA_ARGS__); \
} while(0)
#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__); \
+ pg_log_generic(PG_LOG_INFO, PG_LOG_HINT, __VA_ARGS__); \
} while(0)
#define pg_log_debug(...) do { \
@@ -167,8 +158,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)
