[5/9] log: improve performance of qemu_log and qemu_log_mask if disabled
diff mbox

Message ID 1457954501-26528-6-git-send-email-den@openvz.org
State New
Headers show

Commit Message

Denis V. Lunev March 14, 2016, 11:21 a.m. UTC
The patch is intended to avoid to perform any operation including
calculation of log function arguments when the log is not enabled due to
various reasons.

Functions qemu_log and qemu_log_mask are replaced with variadic macros.

Format checking performed by compiler will not suffer by this patch. It
will be done inside in fprintf arguments checking.

Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/log.h | 15 ++++++++++++---
 util/log.c         | 21 ---------------------
 2 files changed, 12 insertions(+), 24 deletions(-)

Comments

Paolo Bonzini March 14, 2016, 2:30 p.m. UTC | #1
On 14/03/2016 12:21, Denis V. Lunev wrote:
> The patch is intended to avoid to perform any operation including
> calculation of log function arguments when the log is not enabled due to
> various reasons.
> 
> Functions qemu_log and qemu_log_mask are replaced with variadic macros.
> 
> Format checking performed by compiler will not suffer by this patch. It
> will be done inside in fprintf arguments checking.

Have you encountered a place that was calling them so hard that it
caused performance problem?  If so, the logging should probably be
replaced by a tracepoint.

Paolo

> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qemu/log.h | 15 ++++++++++++---
>  util/log.c         | 21 ---------------------
>  2 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/include/qemu/log.h b/include/qemu/log.h
> index 2c2c220..a05c7dc 100644
> --- a/include/qemu/log.h
> +++ b/include/qemu/log.h
> @@ -54,7 +54,12 @@ static inline bool qemu_loglevel_mask(int mask)
>  
>  /* main logging function
>   */
> -void GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
> +#define qemu_log(...)                          \
> +    do {                                       \
> +        if (qemu_log_enabled()) {              \
> +           fprintf(qemu_logfile, __VA_ARGS__); \
> +        }                                      \
> +    } while (0)
>  
>  /* vfprintf-like logging function
>   */
> @@ -68,8 +73,12 @@ qemu_log_vprintf(const char *fmt, va_list va)
>  
>  /* log only if a bit is set on the current loglevel mask
>   */
> -void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...);
> -
> +#define qemu_log_mask(mask, ...)                                \
> +    do {                                                        \
> +        if ((qemu_loglevel & (mask)) && qemu_log_enabled()) {   \
> +           fprintf(qemu_logfile, __VA_ARGS__);                  \
> +        }                                                       \
> +    } while (0)
>  
>  /* Maintenance: */
>  
> diff --git a/util/log.c b/util/log.c
> index df672cc..65d46e2 100644
> --- a/util/log.c
> +++ b/util/log.c
> @@ -27,27 +27,6 @@ FILE *qemu_logfile;
>  int qemu_loglevel;
>  static int log_append = 0;
>  
> -void qemu_log(const char *fmt, ...)
> -{
> -    va_list ap;
> -
> -    va_start(ap, fmt);
> -    if (qemu_logfile) {
> -        vfprintf(qemu_logfile, fmt, ap);
> -    }
> -    va_end(ap);
> -}
> -
> -void qemu_log_mask(int mask, const char *fmt, ...)
> -{
> -    va_list ap;
> -
> -    va_start(ap, fmt);
> -    if ((qemu_loglevel & mask) && qemu_logfile) {
> -        vfprintf(qemu_logfile, fmt, ap);
> -    }
> -    va_end(ap);
> -}
>  
>  /* enable or disable low levels log */
>  void do_qemu_set_log(int log_flags, bool use_own_buffers)
>
Denis V. Lunev March 14, 2016, 3:07 p.m. UTC | #2
On 03/14/2016 05:30 PM, Paolo Bonzini wrote:
>
> On 14/03/2016 12:21, Denis V. Lunev wrote:
>> The patch is intended to avoid to perform any operation including
>> calculation of log function arguments when the log is not enabled due to
>> various reasons.
>>
>> Functions qemu_log and qemu_log_mask are replaced with variadic macros.
>>
>> Format checking performed by compiler will not suffer by this patch. It
>> will be done inside in fprintf arguments checking.
> Have you encountered a place that was calling them so hard that it
> caused performance problem?  If so, the logging should probably be
> replaced by a tracepoint.
>
> Paolo
>

inline functions can have side-effects, i.e. the arguments of them
can be evaluated depending on the compiler. with this approach
we are sure that this will not happen, ever.

Den

Patch
diff mbox

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 2c2c220..a05c7dc 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -54,7 +54,12 @@  static inline bool qemu_loglevel_mask(int mask)
 
 /* main logging function
  */
-void GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
+#define qemu_log(...)                          \
+    do {                                       \
+        if (qemu_log_enabled()) {              \
+           fprintf(qemu_logfile, __VA_ARGS__); \
+        }                                      \
+    } while (0)
 
 /* vfprintf-like logging function
  */
@@ -68,8 +73,12 @@  qemu_log_vprintf(const char *fmt, va_list va)
 
 /* log only if a bit is set on the current loglevel mask
  */
-void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...);
-
+#define qemu_log_mask(mask, ...)                                \
+    do {                                                        \
+        if ((qemu_loglevel & (mask)) && qemu_log_enabled()) {   \
+           fprintf(qemu_logfile, __VA_ARGS__);                  \
+        }                                                       \
+    } while (0)
 
 /* Maintenance: */
 
diff --git a/util/log.c b/util/log.c
index df672cc..65d46e2 100644
--- a/util/log.c
+++ b/util/log.c
@@ -27,27 +27,6 @@  FILE *qemu_logfile;
 int qemu_loglevel;
 static int log_append = 0;
 
-void qemu_log(const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    if (qemu_logfile) {
-        vfprintf(qemu_logfile, fmt, ap);
-    }
-    va_end(ap);
-}
-
-void qemu_log_mask(int mask, const char *fmt, ...)
-{
-    va_list ap;
-
-    va_start(ap, fmt);
-    if ((qemu_loglevel & mask) && qemu_logfile) {
-        vfprintf(qemu_logfile, fmt, ap);
-    }
-    va_end(ap);
-}
 
 /* enable or disable low levels log */
 void do_qemu_set_log(int log_flags, bool use_own_buffers)