Switch symtab to use fxhash hashing

This merge request resolves some performance regressions introduced
with the change from isc_symtab_t to isc_hashmap_t.

The key improvements are:

1. Using a faster hash function than both isc_hashmap_t and
   isc_symtab_t. The previous implementation used SipHash, but the
   hashflood resistance properties of SipHash are unneeded for config
   parsing.
2. Shrinking the initial size of the isc_hashmap_t used inside
   isc_symtab_t. Symtab is mainly used for config parsing, and the
   when used that way it will have between 1 and ~50 keys, but the
   previous implementation initialized a map with 128 slots.
   By initializing a smaller map, we speed up mallocs and optimize for
   the typical case of few config keys.
3. Slight optimization of the string matching in the hashmap, so that
   the tail is handled in a single load + comparison, instead of byte
   by byte.
   Of the three improvements, this is the least important.

diff --git a/lib/isc/Makefile.am b/lib/isc/Makefile.am
index 3ccee9a520466ae39bcc95ec6ac86e9fea0660f8..27cfe92f3e2e1135e702bebb67a9274fbf71000d 100644 (file)
--- a/lib/isc/Makefile.am
+++ b/lib/isc/Makefile.am
@@ -26,6 +26,7 @@ libisc_la_HEADERS =                   \
        include/isc/file.h              \
        include/isc/formatcheck.h       \
        include/isc/fuzz.h              \
+       include/isc/fxhash.h            \
        include/isc/getaddresses.h      \
        include/isc/hash.h              \
        include/isc/hashmap.h           \

diff --git a/lib/isc/include/isc/ascii.h b/lib/isc/include/isc/ascii.h
index 384c3a190d8dce599ba594eb6f6fd47c9f77500a..84cfb1159532940709f8005f9eb434390b9784f9 100644 (file)
--- a/lib/isc/include/isc/ascii.h
+++ b/lib/isc/include/isc/ascii.h
@@ -131,18 +131,28 @@ isc__ascii_load8(const uint8_t *ptr) {
  * Compare `len` bytes at `a` and `b` for case-insensitive equality
  */
 static inline bool
-isc_ascii_lowerequal(const uint8_t *a, const uint8_t *b, unsigned int len) {
+isc_ascii_lowerequal(const uint8_t *restrict a, const uint8_t *restrict b,
+                    unsigned int len) {
        uint64_t a8 = 0, b8 = 0;
-       while (len >= 8) {
-               a8 = isc_ascii_tolower8(isc__ascii_load8(a));
-               b8 = isc_ascii_tolower8(isc__ascii_load8(b));
-               if (a8 != b8) {
-                       return false;
+       if (len >= 8) {
+               const uint8_t *a_tail = a + len - 8;
+               const uint8_t *b_tail = b + len - 8;
+               while (len >= 8) {
+                       a8 = isc_ascii_tolower8(isc__ascii_load8(a));
+                       b8 = isc_ascii_tolower8(isc__ascii_load8(b));
+                       if (a8 != b8) {
+                               return false;
+                       }
+                       len -= 8;
+                       a += 8;
+                       b += 8;
                }
-               len -= 8;
-               a += 8;
-               b += 8;
+
+               a8 = isc_ascii_tolower8(isc__ascii_load8(a_tail));
+               b8 = isc_ascii_tolower8(isc__ascii_load8(b_tail));
+               return a8 == b8;
        }
+
        while (len-- > 0) {
                if (isc_ascii_tolower(*a++) != isc_ascii_tolower(*b++)) {
                        return false;

diff --git a/lib/isc/include/isc/fxhash.h b/lib/isc/include/isc/fxhash.h
new file mode 100644 (file)
index 0000000..b76cce4
--- /dev/null
+++ b/lib/isc/include/isc/fxhash.h
@@ -0,0 +1,89 @@
+#pragma once
+
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the “Software”), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <string.h>
+
+/* The constant K from Rust's fxhash */
+#define K 0x9e3779b97f4a7c15ull
+
+static inline size_t
+rotate_left(size_t x, unsigned int n) {
+       return (x << n) | (x >> (sizeof(size_t) * 8 - n));
+}
+
+static inline size_t
+fx_add_to_hash(size_t hash, size_t i) {
+       return rotate_left(hash, 5) ^ i * K;
+}
+
+/*
+ * Beware: this implementation will use an approximate conversion to lowercase.
+ * This is ok as fxhash is already not hash-flooding resistant and we use it
+ * only for config parsing.
+ */
+static inline size_t
+fx_hash_bytes(size_t initial_hash, const uint8_t *restrict bytes, size_t len,
+             bool case_sensitive) {
+       size_t hash = initial_hash;
+       size_t case_mask = case_sensitive
+                                  ? -1ull
+                                  : (0b11011111 * 0x0101010101010101ull);
+
+       while (len >= sizeof(size_t)) {
+               size_t value;
+               memmove(&value, bytes, sizeof(size_t));
+               hash = fx_add_to_hash(hash, value & case_mask);
+               bytes += sizeof(size_t);
+               len -= sizeof(size_t);
+       }
+
+       /* Will be ignored if sizeof(size_t) <= 4 */
+       if (len >= 4) {
+               uint32_t value;
+               memmove(&value, bytes, sizeof(uint32_t));
+               hash = fx_add_to_hash(hash, value & case_mask);
+               bytes += 4;
+               len -= 4;
+       }
+
+       /* Will be ignored if sizeof(size_t) <= 2 */
+       if (len >= 2) {
+               uint16_t value;
+               memmove(&value, bytes, sizeof(uint16_t));
+               hash = fx_add_to_hash(hash, value & case_mask);
+               bytes += 2;
+               len -= 2;
+       }
+
+       if (len >= 1) {
+               hash = fx_add_to_hash(hash, bytes[0] & case_mask);
+       }
+
+       return hash;
+}

diff --git a/lib/isc/symtab.c b/lib/isc/symtab.c
index d4a7c5c628813b839ccd7c0e2d7fb365b95874e0..fcd192be60fc70d6157c6a1dc67c613daca8fded 100644 (file)
--- a/lib/isc/symtab.c
+++ b/lib/isc/symtab.c
@@ -16,6 +16,7 @@
 #include <stdbool.h>
 
 #include <isc/ascii.h>
+#include <isc/fxhash.h>
 #include <isc/hash.h>
 #include <isc/hashmap.h>
 #include <isc/magic.h>
@@ -31,9 +32,9 @@ typedef struct elt {
        isc_symvalue_t value;
 } elt_t;
 
-/* 7 bits means 128 entries at creation, which matches the common use of
+/* 4 bits means 16 entries at creation, which matches the common use of
  * symtab */
-#define ISC_SYMTAB_INIT_HASH_BITS 7
+#define ISC_SYMTAB_INIT_HASH_BITS 4
 #define SYMTAB_MAGIC             ISC_MAGIC('S', 'y', 'm', 'T')
 #define VALID_SYMTAB(st)         ISC_MAGIC_VALID(st, SYMTAB_MAGIC)
 
@@ -122,7 +123,7 @@ elt__match(void *node, const void *key0, bool case_sensitive) {
        if (case_sensitive) {
                return memcmp(elt->key, key->key, key->size) == 0;
        } else {
-               return isc_ascii_lowercmp(elt->key, key->key, key->size) == 0;
+               return isc_ascii_lowerequal(elt->key, key->key, key->size);
        }
 }
 
@@ -136,14 +137,11 @@ elt_match_nocase(void *node, const void *key) {
        return elt__match(node, key, false);
 }
 
-static uint32_t
-elt_hash(elt_t *elt, bool case_sensitive) {
-       isc_hash32_t hash;
-
-       isc_hash32_init(&hash);
-       isc_hash32_hash(&hash, elt->key, elt->size, case_sensitive);
-       isc_hash32_hash(&hash, &elt->type, sizeof(elt->type), false);
-       return isc_hash32_finalize(&hash);
+static inline uint32_t
+elt_hash(elt_t *restrict elt, bool case_sensitive) {
+       const uint8_t *ptr = elt->key;
+       size_t len = elt->size;
+       return fx_hash_bytes(0, ptr, len, case_sensitive);
 }
 
 isc_result_t