From d1dbf6b20fdcfa95acd75cdb96fcd57067a31144 Mon Sep 17 00:00:00 2001 From: Mukund Sivaraman Date: Mon, 1 Feb 2016 08:59:49 +0530 Subject: [PATCH] Use __built_expect() where available (#41411) --- CHANGES | 3 +++ config.h.in | 3 +++ configure | 39 ++++++++++++++++++++++++++++++++ configure.in | 17 ++++++++++++++ lib/dns/name.c | 16 ++++++------- lib/dns/rbt.c | 18 +++++++-------- lib/isc/hash.c | 2 +- lib/isc/include/isc/assertions.h | 8 +++---- lib/isc/include/isc/error.h | 2 +- lib/isc/include/isc/magic.h | 6 +++-- lib/isc/include/isc/util.h | 11 +++++++++ 11 files changed, 100 insertions(+), 25 deletions(-) diff --git a/CHANGES b/CHANGES index 019d5479f7..276391de97 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4310. [performance] Use __builtin_expect() where available to annotate + conditions with known behavior. [RT #41411] + 4309. [cleanup] Remove the spurious "none" filename from log messages when processing built-in configuration. [RT #41594] diff --git a/config.h.in b/config.h.in index 3836450b08..2fbdba73de 100644 --- a/config.h.in +++ b/config.h.in @@ -191,6 +191,9 @@ int sigwait(const unsigned int *set, int *sig); MSVC and with C++ compilers. */ #undef FLEXIBLE_ARRAY_MEMBER +/* Define to 1 if the compiler supports __builtin_expect. */ +#undef HAVE_BUILTIN_EXPECT + /* Define to 1 if you have the `chroot' function. */ #undef HAVE_CHROOT diff --git a/configure b/configure index 15871119d5..38787be92b 100755 --- a/configure +++ b/configure @@ -19924,6 +19924,45 @@ fi ISC_ARCH_DIR=$arch +# +# Check for __builtin_expect +# +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking compiler support for __builtin_expect" >&5 +$as_echo_n "checking compiler support for __builtin_expect... " >&6; } +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + +int +main () +{ + + return (__builtin_expect(1, 1) ? 1 : 0); + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link "$LINENO"; then : + + have_builtin_expect=yes + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +$as_echo "yes" >&6; } + +else + + have_builtin_expect=no + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5 +$as_echo "no" >&6; } + +fi +rm -f core conftest.err conftest.$ac_objext \ + conftest$ac_exeext conftest.$ac_ext +if test "$have_builtin_expect" = "yes"; then + +$as_echo "#define HAVE_BUILTIN_EXPECT 1" >>confdefs.h + +fi + # # Activate "rrset-order fixed" or not? # diff --git a/configure.in b/configure.in index 05b27b0b1e..b0523bb4e2 100644 --- a/configure.in +++ b/configure.in @@ -3892,6 +3892,23 @@ AC_SUBST(ISC_PLATFORM_USEMACASM) ISC_ARCH_DIR=$arch AC_SUBST(ISC_ARCH_DIR) +# +# Check for __builtin_expect +# +AC_MSG_CHECKING([compiler support for __builtin_expect]) +AC_TRY_LINK(, [ + return (__builtin_expect(1, 1) ? 1 : 0); +], [ + have_builtin_expect=yes + AC_MSG_RESULT(yes) +], [ + have_builtin_expect=no + AC_MSG_RESULT(no) +]) +if test "$have_builtin_expect" = "yes"; then + AC_DEFINE(HAVE_BUILTIN_EXPECT, 1, [Define to 1 if the compiler supports __builtin_expect.]) +fi + # # Activate "rrset-order fixed" or not? # diff --git a/lib/dns/name.c b/lib/dns/name.c index 68f3180edd..d8d07f9c62 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -591,7 +591,7 @@ dns_name_fullcompare(const dns_name_t *name1, const dns_name_t *name2, REQUIRE((name1->attributes & DNS_NAMEATTR_ABSOLUTE) == (name2->attributes & DNS_NAMEATTR_ABSOLUTE)); - if (name1 == name2) { + if (ISC_UNLIKELY(name1 == name2)) { *orderp = 0; *nlabelsp = name1->labels; return (dns_namereln_equal); @@ -614,7 +614,7 @@ dns_name_fullcompare(const dns_name_t *name1, const dns_name_t *name2, offsets1 += l1; offsets2 += l2; - while (l > 0) { + while (ISC_LIKELY(l > 0)) { l--; offsets1--; offsets2--; @@ -636,7 +636,7 @@ dns_name_fullcompare(const dns_name_t *name1, const dns_name_t *name2, count = count2; /* Loop unrolled for performance */ - while (count > 3) { + while (ISC_LIKELY(count > 3)) { chdiff = (int)maptolower[label1[0]] - (int)maptolower[label2[0]]; if (chdiff != 0) { @@ -665,7 +665,7 @@ dns_name_fullcompare(const dns_name_t *name1, const dns_name_t *name2, label1 += 4; label2 += 4; } - while (count-- > 0) { + while (ISC_LIKELY(count-- > 0)) { chdiff = (int)maptolower[*label1++] - (int)maptolower[*label2++]; if (chdiff != 0) { @@ -741,7 +741,7 @@ dns_name_equal(const dns_name_t *name1, const dns_name_t *name2) { REQUIRE((name1->attributes & DNS_NAMEATTR_ABSOLUTE) == (name2->attributes & DNS_NAMEATTR_ABSOLUTE)); - if (name1 == name2) + if (ISC_UNLIKELY(name1 == name2)) return (ISC_TRUE); if (name1->length != name2->length) @@ -754,7 +754,7 @@ dns_name_equal(const dns_name_t *name1, const dns_name_t *name2) { label1 = name1->ndata; label2 = name2->ndata; - while (l-- > 0) { + while (ISC_LIKELY(l-- > 0)) { count = *label1++; if (count != *label2++) return (ISC_FALSE); @@ -762,7 +762,7 @@ dns_name_equal(const dns_name_t *name1, const dns_name_t *name2) { INSIST(count <= 63); /* no bitstring support */ /* Loop unrolled for performance */ - while (count > 3) { + while (ISC_LIKELY(count > 3)) { c = maptolower[label1[0]]; if (c != maptolower[label2[0]]) return (ISC_FALSE); @@ -779,7 +779,7 @@ dns_name_equal(const dns_name_t *name1, const dns_name_t *name2) { label1 += 4; label2 += 4; } - while (count-- > 0) { + while (ISC_LIKELY(count-- > 0)) { c = maptolower[*label1++]; if (c != maptolower[*label2++]) return (ISC_FALSE); diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 86b5183f60..9fb08075f0 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -1136,7 +1136,7 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { add_name = dns_fixedname_name(&fixedcopy); dns_name_clone(name, add_name); - if (rbt->root == NULL) { + if (ISC_UNLIKELY(rbt->root == NULL)) { result = create_node(rbt->mctx, add_name, &new_current); if (result == ISC_R_SUCCESS) { rbt->nodecount++; @@ -1377,12 +1377,12 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { } - } while (child != NULL); + } while (ISC_LIKELY(child != NULL)); - if (result == ISC_R_SUCCESS) + if (ISC_LIKELY(result == ISC_R_SUCCESS)) result = create_node(rbt->mctx, add_name, &new_current); - if (result == ISC_R_SUCCESS) { + if (ISC_LIKELY(result == ISC_R_SUCCESS)) { #ifdef DNS_RBT_USEHASH if (*root == NULL) UPPERNODE(new_current) = current; @@ -1465,7 +1465,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, } else dns_rbtnodechain_reset(chain); - if (rbt->root == NULL) + if (ISC_UNLIKELY(rbt->root == NULL)) return (ISC_R_NOTFOUND); /* @@ -1496,7 +1496,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, current = rbt->root; current_root = rbt->root; - while (current != NULL) { + while (ISC_LIKELY(current != NULL)) { NODENAME(current, ¤t_name); compared = dns_name_fullcompare(search_name, ¤t_name, &order, &common_labels); @@ -1577,7 +1577,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, { dns_name_t hnode_name; - if (hash != HASHVAL(hnode)) + if (ISC_LIKELY(hash != HASHVAL(hnode))) continue; /* * This checks that the hashed label @@ -1586,12 +1586,12 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * match a labelsequence from some other * subdomain. */ - if (get_upper_node(hnode) != up_current) + if (ISC_LIKELY(get_upper_node(hnode) != up_current)) continue; dns_name_init(&hnode_name, NULL); NODENAME(hnode, &hnode_name); - if (dns_name_equal(&hnode_name, &hash_name)) + if (ISC_LIKELY(dns_name_equal(&hnode_name, &hash_name))) break; } diff --git a/lib/isc/hash.c b/lib/isc/hash.c index 95b573db10..9fc993c951 100644 --- a/lib/isc/hash.c +++ b/lib/isc/hash.c @@ -492,7 +492,7 @@ isc_hash_function_reverse(const void *data, size_t length, RUNTIME_CHECK(isc_once_do(&fnv_once, fnv_initialize) == ISC_R_SUCCESS); - hval = previous_hashp != NULL ? *previous_hashp : fnv_offset_basis; + hval = ISC_UNLIKELY(previous_hashp != NULL) ? *previous_hashp : fnv_offset_basis; bp = (const unsigned char *) data; be = bp + length; diff --git a/lib/isc/include/isc/assertions.h b/lib/isc/include/isc/assertions.h index 2c81b1ae98..5f264619ff 100644 --- a/lib/isc/include/isc/assertions.h +++ b/lib/isc/include/isc/assertions.h @@ -83,7 +83,7 @@ isc_assertion_typetotext(isc_assertiontype_t type); #if ISC_CHECK_REQUIRE != 0 #define ISC_REQUIRE(cond) \ - ((void) ((cond) || \ + ((void) (ISC_LIKELY(cond) || \ ((isc_assertion_failed)(__FILE__, __LINE__, \ isc_assertiontype_require, \ #cond), 0))) @@ -93,7 +93,7 @@ isc_assertion_typetotext(isc_assertiontype_t type); #if ISC_CHECK_ENSURE != 0 #define ISC_ENSURE(cond) \ - ((void) ((cond) || \ + ((void) (ISC_LIKELY(cond) || \ ((isc_assertion_failed)(__FILE__, __LINE__, \ isc_assertiontype_ensure, \ #cond), 0))) @@ -103,7 +103,7 @@ isc_assertion_typetotext(isc_assertiontype_t type); #if ISC_CHECK_INSIST != 0 #define ISC_INSIST(cond) \ - ((void) ((cond) || \ + ((void) (ISC_LIKELY(cond) || \ ((isc_assertion_failed)(__FILE__, __LINE__, \ isc_assertiontype_insist, \ #cond), 0))) @@ -113,7 +113,7 @@ isc_assertion_typetotext(isc_assertiontype_t type); #if ISC_CHECK_INVARIANT != 0 #define ISC_INVARIANT(cond) \ - ((void) ((cond) || \ + ((void) (ISC_LIKELY(cond) || \ ((isc_assertion_failed)(__FILE__, __LINE__, \ isc_assertiontype_invariant, \ #cond), 0))) diff --git a/lib/isc/include/isc/error.h b/lib/isc/include/isc/error.h index e0cdfa83e7..5f13d5ed41 100644 --- a/lib/isc/include/isc/error.h +++ b/lib/isc/include/isc/error.h @@ -55,7 +55,7 @@ void isc_error_runtimecheck(const char *, int, const char *); #define ISC_ERROR_RUNTIMECHECK(cond) \ - ((void) ((cond) || \ + ((void) (ISC_LIKELY(cond) || \ ((isc_error_runtimecheck)(__FILE__, __LINE__, #cond), 0))) ISC_LANG_ENDDECLS diff --git a/lib/isc/include/isc/magic.h b/lib/isc/include/isc/magic.h index 073de90dcc..635d70f774 100644 --- a/lib/isc/include/isc/magic.h +++ b/lib/isc/include/isc/magic.h @@ -20,6 +20,8 @@ #ifndef ISC_MAGIC_H #define ISC_MAGIC_H 1 +#include + /*! \file isc/magic.h */ typedef struct { @@ -33,8 +35,8 @@ typedef struct { * The intent of this is to allow magic numbers to be checked even though * the object is otherwise opaque. */ -#define ISC_MAGIC_VALID(a,b) (((a) != NULL) && \ - (((const isc__magic_t *)(a))->magic == (b))) +#define ISC_MAGIC_VALID(a,b) (ISC_LIKELY((a) != NULL) && \ + ISC_LIKELY(((const isc__magic_t *)(a))->magic == (b))) #define ISC_MAGIC(a, b, c, d) ((a) << 24 | (b) << 16 | (c) << 8 | (d)) diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 6baf786bde..2f4fd3b7f4 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -206,6 +206,17 @@ #define INSERTAFTER(li, a, e, ln) ISC_LIST_INSERTAFTER(li, a, e, ln) #define APPENDLIST(list1, list2, link) ISC_LIST_APPENDLIST(list1, list2, link) +/*% + * Performance + */ +#ifdef HAVE_BUILTIN_EXPECT +#define ISC_LIKELY(x) __builtin_expect(!!(x), 1) +#define ISC_UNLIKELY(x) __builtin_expect(!!(x), 0) +#else +#define ISC_LIKELY(x) (x) +#define ISC_UNLIKELY(x) (x) +#endif + /* * Assertions */