Libguestfs - Nov 2019 - [PATCH libnbd] lib: Use GCC hints to move debug and error handling code out of hot paths.

---
 generator/generator |  6 +++---
 lib/crypto.c        |  2 +-
 lib/internal.h      | 14 +++++++++++++-
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/generator/generator b/generator/generator
index c2ff0db..eb3408d 100755
--- a/generator/generator
+++ b/generator/generator
@@ -4512,7 +4512,7 @@ let generate_lib_api_c ()        let value = match errcode
with
         | Some value -> value
         | None -> assert false in
-      pr "  if (!%s_in_permitted_state (h)) {\n" name;
+      pr "  if (unlikely (!%s_in_permitted_state (h))) {\n" name;
       pr "    ret = %s;\n" value;
       pr "    goto out;\n";
       pr "  }\n";
@@ -4525,7 +4525,7 @@ let generate_lib_api_c ()          | Some value ->
value
         | None -> assert false in
       let mask = List.fold_left (lor) 0 (List.map snd flags) in
-      pr "  if ((%s & ~%d) != 0) {\n" n mask;
+      pr "  if (unlikely ((%s & ~%d) != 0)) {\n" n mask;
       pr "    set_error (EINVAL, \"%%s: invalid value for flag:
%%d\",\n";
       pr "               \"%s\", %s);\n" n n;
       pr "    ret = %s;\n" value;
@@ -4658,7 +4658,7 @@ let generate_lib_api_c ()      pr ");\n"
   (* Print the trace when we leave a call with debugging enabled. *)
   and print_trace_leave ret -    pr "  if (h->debug) {\n";
+    pr "  if_debug (h) {\n";
     let errcode = errcode_of_ret ret in
     (match errcode with
      | Some r -> 
diff --git a/lib/crypto.c b/lib/crypto.c
index 07d06c0..8d86911 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -676,7 +676,7 @@ nbd_internal_crypto_handshake (struct nbd_handle *h)
 void
 nbd_internal_crypto_debug_tls_enabled (struct nbd_handle *h)
 {
-  if (h->debug) {
+  if_debug (h) {
     const gnutls_session_t session = h->sock->u.tls.session;
     const gnutls_cipher_algorithm_t cipher = gnutls_cipher_get (session);
     const gnutls_kx_algorithm_t kx = gnutls_kx_get (session);
diff --git a/lib/internal.h b/lib/internal.h
index 50c0a9b..894e437 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -40,6 +40,18 @@
 #include "states.h"
 #include "unlocked.h"
 
+/* Define unlikely macro, but only for GCC.  These are used to move
+ * debug and error handling code out of hot paths, making the hot path
+ * into common functions use less instruction cache.
+ */
+#if defined(__GNUC__)
+#define unlikely(x) __builtin_expect (!!(x), 0)
+#define if_debug(h) if (unlikely ((h)->debug))
+#else
+#define unlikely(x) (x)
+#define if_debug(h) if ((h)->debug)
+#endif
+
 /* MSG_MORE is an optimization.  If not present, ignore it. */
 #ifndef MSG_MORE
 #define MSG_MORE 0
@@ -329,7 +341,7 @@ extern void nbd_internal_crypto_debug_tls_enabled (struct
nbd_handle *);
 extern void nbd_internal_debug (struct nbd_handle *h, const char *fs, ...);
 #define debug(h, fs, ...)                               \
   do {                                                  \
-    if ((h)->debug)                                     \
+    if_debug ((h))                                      \
       nbd_internal_debug ((h), (fs), ##__VA_ARGS__);    \
   } while (0)
 
-- 
2.23.0

On 11/2/19 4:24 PM, Richard W.M. Jones wrote:
> ---
>   generator/generator |  6 +++---
>   lib/crypto.c        |  2 +-
>   lib/internal.h      | 14 +++++++++++++-
>   3 files changed, 17 insertions(+), 5 deletions(-)
> 
> +++ b/lib/internal.h
> @@ -40,6 +40,18 @@
>   #include "states.h"
>   #include "unlocked.h"
>   
> +/* Define unlikely macro, but only for GCC.  These are used to move
> + * debug and error handling code out of hot paths, making the hot path
> + * into common functions use less instruction cache.
> + */
> +#if defined(__GNUC__)
> +#define unlikely(x) __builtin_expect (!!(x), 0)
> +#define if_debug(h) if (unlikely ((h)->debug))
> +#else
> +#define unlikely(x) (x)
> +#define if_debug(h) if ((h)->debug)
> +#endif
Same question about applicability to clang as for nbdkit, otherwise 
looks fine.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org