diff --git a/CHANGES b/CHANGES index c1d5439158..a2dead8cd1 100644 --- a/CHANGES +++ b/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] diff --git a/lib/bind9/check.c b/lib/bind9/check.c index 188234f823..b2ec4499c0 100644 --- a/lib/bind9/check.c +++ b/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); diff --git a/lib/isc/include/isc/symtab.h b/lib/isc/include/isc/symtab.h index 886d3b2d3a..5d4e163782 100644 --- a/lib/isc/include/isc/symtab.h +++ b/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. * diff --git a/lib/isc/symtab.c b/lib/isc/symtab.c index 9f8e798df3..52726fef7b 100644 --- a/lib/isc/symtab.c +++ b/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); } diff --git a/lib/isc/tests/Makefile.in b/lib/isc/tests/Makefile.in index 0ffe74b3c7..87a6167e6b 100644 --- a/lib/isc/tests/Makefile.in +++ b/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 diff --git a/lib/isc/tests/symtab_test.c b/lib/isc/tests/symtab_test.c new file mode 100644 index 0000000000..d94e2d9a69 --- /dev/null +++ b/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 + +#include + +#include + +#include + +#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()); +} +