3228. [tuning] Dynamically grow symbol table to improve zone

loading performance. [RT #26523]

CHANGES

@@ -1,3 +1,6 @@
+3228.   [tuning]        Dynamically grow symbol table to improve zone
+                        loading performance. [RT #26523]
+
 3227.	[bug]		Interim fix to make WKS's use of getprotobyname()
 			and getservbyname() self thread safe. [RT #26232]
 

lib/bind9/check.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: check.c,v 1.138 2011/11/07 00:14:11 marka Exp $ */
+/* $Id: check.c,v 1.139 2011/11/30 04:27:17 each Exp $ */
 
 /*! \file */
 
@@ -2149,7 +2149,7 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions,
 	 * Check that all zone statements are syntactically correct and
 	 * there are no duplicate zones.
 	 */
-	tresult = isc_symtab_create(mctx, 100, freekey, mctx,
+	tresult = isc_symtab_create(mctx, 1000, freekey, mctx,
 				    ISC_FALSE, &symtab);
 	if (tresult != ISC_R_SUCCESS)
 		return (ISC_R_NOMEMORY);
@@ -2213,7 +2213,7 @@ check_viewconf(const cfg_obj_t *config, const cfg_obj_t *voptions,
 	 * Check that all key statements are syntactically correct and
 	 * there are no duplicate keys.
 	 */
-	tresult = isc_symtab_create(mctx, 100, freekey, mctx,
+	tresult = isc_symtab_create(mctx, 1000, freekey, mctx,
 				    ISC_FALSE, &symtab);
 	if (tresult != ISC_R_SUCCESS)
 		return (ISC_R_NOMEMORY);

lib/isc/include/isc/symtab.h

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: symtab.h,v 1.26 2009/01/18 23:48:14 tbox Exp $ */
+/* $Id: symtab.h,v 1.27 2011/11/30 04:27:17 each Exp $ */
 
 #ifndef ISC_SYMTAB_H
 #define ISC_SYMTAB_H 1
@@ -57,6 +57,14 @@
  * undefined.  It can be used to free memory associated with keys and/or
  * values.
  *
+ * A symbol table is implemented as a hash table of lists; the size of the
+ * hash table is set by the 'size' parameter to isc_symtbl_create().  When
+ * the number of entries in the symbol table reaches three quarters of this
+ * value, the hash table is reallocated with size doubled, in order to
+ * optimize lookup performance.  This has a negative effect on insertion
+ * performance, which can be mitigated by sizing the table appropriately
+ * when creating it.
+ *
  * \li MP:
  *	The callers of this module must ensure any required synchronization.
  *

lib/isc/symtab.c

@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: symtab.c,v 1.30 2007/06/19 23:47:17 tbox Exp $ */
+/* $Id: symtab.c,v 1.31 2011/11/30 04:27:17 each Exp $ */
 
 /*! \file */
 
@@ -46,6 +46,8 @@ struct isc_symtab {
 	unsigned int			magic;
 	isc_mem_t *			mctx;
 	unsigned int			size;
+	unsigned int			count;
+	unsigned int			maxload;
 	eltlist_t *			table;
 	isc_symtabaction_t		undefine_action;
 	void *				undefine_arg;
@@ -79,6 +81,8 @@ isc_symtab_create(isc_mem_t *mctx, unsigned int size,
 		INIT_LIST(symtab->table[i]);
 	symtab->mctx = mctx;
 	symtab->size = size;
+	symtab->count = 0;
+	symtab->maxload = size * 3 / 4;
 	symtab->undefine_action = undefine_action;
 	symtab->undefine_arg = undefine_arg;
 	symtab->case_sensitive = case_sensitive;
@@ -181,6 +185,44 @@ isc_symtab_lookup(isc_symtab_t *symtab, const char *key, unsigned int type,
 	return (ISC_R_SUCCESS);
 }
 
+static void
+grow_table(isc_symtab_t *symtab) {
+	eltlist_t *newtable;
+	unsigned int i, newsize, newmax;
+
+	REQUIRE(symtab != NULL);
+
+	newsize = symtab->size * 2;
+	newmax = newsize * 3 / 4;
+
+	newtable = isc_mem_get(symtab->mctx, newsize * sizeof(eltlist_t));
+	if (newtable == NULL)
+		return;
+
+	for (i = 0; i < newsize; i++)
+		INIT_LIST(newtable[i]);
+
+	for (i = 0; i < symtab->size; i++) {
+		elt_t *elt, *nelt;
+
+		for (elt = HEAD(symtab->table[i]); elt != NULL; elt = nelt) {
+			nelt = NEXT(elt, link);
+			unsigned int hv;
+
+			UNLINK(symtab->table[i], elt, link);
+			hv = hash(elt->key, symtab->case_sensitive);
+			APPEND(newtable[hv % newsize], elt, link);
+		}
+	}
+
+	isc_mem_put(symtab->mctx, symtab->table,
+		    symtab->size * sizeof(eltlist_t));
+
+	symtab->table = newtable;
+	symtab->size = newsize;
+	symtab->maxload = newmax;
+}
+
 isc_result_t
 isc_symtab_define(isc_symtab_t *symtab, const char *key, unsigned int type,
 		  isc_symvalue_t value, isc_symexists_t exists_policy)
@@ -208,6 +250,7 @@ isc_symtab_define(isc_symtab_t *symtab, const char *key, unsigned int type,
 		if (elt == NULL)
 			return (ISC_R_NOMEMORY);
 		ISC_LINK_INIT(elt, link);
+		symtab->count++;
 	}
 
 	/*
@@ -226,6 +269,9 @@ isc_symtab_define(isc_symtab_t *symtab, const char *key, unsigned int type,
 	 */
 	PREPEND(symtab->table[bucket], elt, link);
 
+	if (symtab->count > symtab->maxload)
+		grow_table(symtab);
+
 	return (ISC_R_SUCCESS);
 }
 
@@ -247,6 +293,7 @@ isc_symtab_undefine(isc_symtab_t *symtab, const char *key, unsigned int type) {
 					  elt->value, symtab->undefine_arg);
 	UNLINK(symtab->table[bucket], elt, link);
 	isc_mem_put(symtab->mctx, elt, sizeof(*elt));
+	symtab->count--;
 
 	return (ISC_R_SUCCESS);
 }

lib/isc/tests/Makefile.in

@@ -12,7 +12,7 @@
 # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 # PERFORMANCE OF THIS SOFTWARE.
 
-# $Id: Makefile.in,v 1.8 2011/10/10 22:57:13 each Exp $
+# $Id: Makefile.in,v 1.9 2011/11/30 04:27:17 each Exp $
 
 srcdir =	@srcdir@
 VPATH =		@srcdir@
@@ -36,11 +36,11 @@ LIBS =		@LIBS@ @ATFLIBS@
 
 OBJS =		isctest.@O@
 SRCS =		isctest.c taskpool_test.c socket_test.c hash_test.c \
-		task_test.c queue_test.c
+		symtab_test.c task_test.c queue_test.c
 
 SUBDIRS =
 TARGETS =	taskpool_test@EXEEXT@ socket_test@EXEEXT@ hash_test@EXEEXT@ \
-		task_test@EXEEXT@ queue_test@EXEEXT@
+		symtab_test@EXEEXT@ task_test@EXEEXT@ queue_test@EXEEXT@
 
 @BIND9_MAKE_RULES@
 
@@ -64,6 +64,10 @@ queue_test@EXEEXT@: queue_test.@O@ isctest.@O@ ${ISCDEPLIBS}
 	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} ${LDFLAGS} -o $@ \
 			queue_test.@O@ isctest.@O@ ${ISCLIBS} ${LIBS}
 
+symtab_test@EXEEXT@: symtab_test.@O@ isctest.@O@ ${ISCDEPLIBS}
+	${LIBTOOL_MODE_LINK} ${PURIFY} ${CC} ${CFLAGS} ${LDFLAGS} -o $@ \
+			symtab_test.@O@ isctest.@O@ ${ISCLIBS} ${LIBS}
+
 unit::
 	sh ${top_srcdir}/unit/unittest.sh
 

lib/isc/tests/symtab_test.c

@@ -0,0 +1,146 @@
+/*
+ * Copyright (C) 2011  Internet Systems Consortium, Inc. ("ISC")
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+ * REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+ * INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+ * LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+ * OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+ * PERFORMANCE OF THIS SOFTWARE.
+ */
+
+/* $Id: symtab_test.c,v 1.2 2011/11/30 04:27:17 each Exp $ */
+
+/*! \file */
+
+#include <config.h>
+
+#include <atf-c.h>
+
+#include <unistd.h>
+
+#include <isc/symtab.h>
+
+#include "isctest.h"
+
+static void
+undefine(char *key, unsigned int type, isc_symvalue_t value, void *arg) {
+	UNUSED(arg);
+
+	ATF_REQUIRE_EQ(type, 1);
+	isc_mem_free(mctx, key);
+	isc_mem_free(mctx, value.as_pointer);
+}
+
+/*
+ * Individual unit tests
+ */
+
+ATF_TC(symtab_grow);
+ATF_TC_HEAD(symtab_grow, tc) {
+	atf_tc_set_md_var(tc, "descr", "symbol table growth");
+}
+ATF_TC_BODY(symtab_grow, tc) {
+	isc_result_t result;
+	isc_symtab_t *st = NULL;
+	isc_symvalue_t value;
+	isc_symexists_t policy = isc_symexists_reject;
+	int i;
+
+	UNUSED(tc);
+
+	result = isc_test_begin(NULL, ISC_TRUE);
+	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+
+	result = isc_symtab_create(mctx, 3, undefine, NULL, ISC_FALSE, &st);
+	ATF_REQUIRE_EQ(result, ISC_R_SUCCESS);
+	ATF_REQUIRE(st != NULL);
+
+	/* Nothing should be in the table yet */
+
+	/*
+	 * Put 1024 entries in the table (this should necessate
+	 * regrowing the hash table several times
+	 */
+	for (i = 0; i < 1024; i++) {
+		char str[16], *key;
+
+		snprintf(str, sizeof(str), "%04x", i);
+		key = isc_mem_strdup(mctx, str);
+		ATF_REQUIRE(key != NULL);
+		value.as_pointer = isc_mem_strdup(mctx, str);
+		ATF_REQUIRE(value.as_pointer != NULL);
+		result = isc_symtab_define(st, key, 1, value, policy);
+		ATF_CHECK_EQ(result, ISC_R_SUCCESS);
+		if (result != ISC_R_SUCCESS)
+			undefine(key, 1, value, NULL);
+	}
+
+	/*
+	 * Try to put them in again; this should fail
+	 */
+	for (i = 0; i < 1024; i++) {
+		char str[16], *key;
+
+		snprintf(str, sizeof(str), "%04x", i);
+		key = isc_mem_strdup(mctx, str);
+		ATF_REQUIRE(key != NULL);
+		value.as_pointer = isc_mem_strdup(mctx, str);
+		ATF_REQUIRE(value.as_pointer != NULL);
+		result = isc_symtab_define(st, key, 1, value, policy);
+		ATF_CHECK_EQ(result, ISC_R_EXISTS);
+		undefine(key, 1, value, NULL);
+	}
+
+	/*
+	 * Retrieve them; this should succeed
+	 */
+	for (i = 0; i < 1024; i++) {
+		char str[16];
+
+		snprintf(str, sizeof(str), "%04x", i);
+		result = isc_symtab_lookup(st, str, 0, &value);
+		ATF_CHECK_EQ(result, ISC_R_SUCCESS);
+		ATF_CHECK_STREQ(str, value.as_pointer);
+	}
+
+	/*
+	 * Undefine them
+	 */
+	for (i = 0; i < 1024; i++) {
+		char str[16];
+
+		snprintf(str, sizeof(str), "%04x", i);
+		result = isc_symtab_undefine(st, str, 1);
+		ATF_CHECK_EQ(result, ISC_R_SUCCESS);
+	}
+
+	/*
+	 * Retrieve them again; this should fail
+	 */
+	for (i = 0; i < 1024; i++) {
+		char str[16];
+
+		snprintf(str, sizeof(str), "%04x", i);
+		result = isc_symtab_lookup(st, str, 0, &value);
+		ATF_CHECK_EQ(result, ISC_R_NOTFOUND);
+	}
+
+	isc_symtab_destroy(&st);
+	isc_test_end();
+}
+
+/*
+ * Main
+ */
+ATF_TP_ADD_TCS(tp) {
+	ATF_TP_ADD_TC(tp, symtab_grow);
+
+	return (atf_no_error());
+}
+