From b65f2ab14abb4b6ef906d7d02064fba158f07b1e Mon Sep 17 00:00:00 2001 From: David Lawrence Date: Wed, 25 Oct 2000 07:21:31 +0000 Subject: [PATCH] 534. [func] Ancestors have been removed from RBT chains. Ancestor information can be discerned via node parent pointers. 533. [func] Incorporated name hashing into the RBT database to improve search speed. There is still evidence of a bug with regard to bitstring labels. It shows up in bin/test/rbt/t_rbt -x -t 4 when the assertion at lib/dns/rbt.c:1631 is uncommented -- essentially a bitstring node's location in the hashtable is not getting properly updated at some point. This shouldn't affect searching, because a bitstring label as the parent of a new level will generally cause the standard old binary search to be done. I will be looking at this more closely in the very near future. --- CHANGES | 10 +- lib/dns/include/dns/rbt.h | 70 ++--- lib/dns/rbt.c | 601 ++++++++++++++++++++++++++------------ 3 files changed, 439 insertions(+), 242 deletions(-) diff --git a/CHANGES b/CHANGES index ac73004aba..81690c1dcd 100644 --- a/CHANGES +++ b/CHANGES @@ -1,8 +1,14 @@ + 534. [func] Ancestors have been removed from RBT chains. Ancestor + information can be discerned via node parent pointers. + + 533. [func] Incorporated name hashing into the RBT database to + improve search speed. + 532. [func] Implement DNS UPDATE pseudo records using DNS_RDATA_UPDATE flag. - 531. [func] Rdata really should be initalized before being - assigned to (dns_rdata_fromwire(), dns_rdata_fromtext(), + 531. [func] Rdata really should be initalized before being assigned + to (dns_rdata_fromwire(), dns_rdata_fromtext(), dns_rdata_clone(), dns_rdata_fromregion()), check that it is. diff --git a/lib/dns/include/dns/rbt.h b/lib/dns/include/dns/rbt.h index 136a766d24..dacc599aa5 100644 --- a/lib/dns/include/dns/rbt.h +++ b/lib/dns/include/dns/rbt.h @@ -15,7 +15,7 @@ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: rbt.h,v 1.49 2000/08/24 01:19:58 gson Exp $ */ +/* $Id: rbt.h,v 1.50 2000/10/25 07:21:31 tale Exp $ */ #ifndef DNS_RBT_H #define DNS_RBT_H 1 @@ -53,6 +53,7 @@ typedef struct dns_rbtnode { struct dns_rbtnode *left; struct dns_rbtnode *right; struct dns_rbtnode *down; + struct dns_rbtnode *hashnext; /* * The following bitfields add up to a total bitwidth of 32. * The range of values necessary for each item is indicated, @@ -72,6 +73,9 @@ typedef struct dns_rbtnode { unsigned int namelen : 8; /* range is 1..255 */ unsigned int offsetlen : 8; /* range is 1..128 */ unsigned int padbytes : 9; /* range is 0..380 */ + + unsigned int hashval; + /* * These values are used in the RBT DB implementation. The appropriate * node lock must be held before accessing them. @@ -129,24 +133,14 @@ typedef isc_result_t (*dns_rbtfindcallback_t)(dns_rbtnode_t *node, * dns_rbtnodechain_current is similar to the _first, _last, _prev and _next * functions but additionally can provide the node to which the chain points. */ -/* - * For use in allocating space for the chain of ancestor nodes. - * - * The maximum number of ancestors is theoretically not limited by the - * data tree. This initial value of 24 ancestors would be able to scan - * the full height of a single level of 16,777,216 nodes, more than double - * the current size of .com. - */ -#define DNS_RBT_ANCESTORBLOCK 24 - /* * The number of level blocks to allocate at a time. Currently the maximum * number of levels is allocated directly in the structure, but future - * revisions of this code might treat levels like ancestors -- that is, have - * a static initial block with dynamic growth. Allocating space for 256 - * levels when the tree is almost never that deep is wasteful, but it's not - * clear that it matters, since the waste is only 2MB for 1000 concurrently - * active chains on a system with 64-bit pointers. + * revisions of this code might have a static initial block with dynamic + * growth. Allocating space for 256 levels when the tree is almost never that + * deep is wasteful, but it's not clear that it matters, since the waste is + * only 2MB for 1000 concurrently active chains on a system with 64-bit + * pointers. */ #define DNS_RBT_LEVELBLOCK 254 @@ -154,20 +148,12 @@ typedef struct dns_rbtnodechain { unsigned int magic; isc_mem_t * mctx; /* - * The terminal node of the chain. It is not in levels[] or - * ancestors[]. This is ostensibly private ... but in a pinch - * it could be used tell that the chain points nowhere without - * needing to call dns_rbtnodechain_current(). + * The terminal node of the chain. It is not in levels[]. + * This is ostensibly private ... but in a pinch it could be + * used tell that the chain points nowhere without needing to + * call dns_rbtnodechain_current(). */ dns_rbtnode_t * end; - dns_rbtnode_t ** ancestors; - /* - * ancestor_block avoids doing any memory allocation (a MP - * bottleneck) in 99%+ of the real-world cases. - */ - dns_rbtnode_t * ancestor_block[DNS_RBT_ANCESTORBLOCK]; - unsigned int ancestor_count; - unsigned int ancestor_maxitems; /* * The maximum number of labels in a name is 128; bitstrings mean * a conceptually very large number (which I have not bothered to @@ -372,16 +358,8 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * everything. But you can certainly construct a trivial tree and a * search for it that has no predecessor. * - * Within the chain structure, 'ancestors' will point - * to each successive node encountered in the search, with the root - * of each level searched indicated by a NULL. ancestor_count - * indicates how many node pointers are in the ancestor list. The - * 'levels' member of the structure holds the root node of each level - * except the first; it corresponds with the NULL pointers in - * 'ancestors' (except the first). That is, for the [n+1]'th NULL - * 'ancestors' pointer, the [n]'th 'levels' pointer is the node with - * the down pointer to the next level. That node is not stored - * at all in the 'ancestors' list. + * Within the chain structure, the 'levels' member of the structure holds + * the root node of each level except the first. * * The 'level_count' of the chain indicates how deep the chain to the * predecessor name is, as an index into the 'levels[]' array. It does @@ -396,12 +374,6 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * the node is found in the top level tree, or no node is found at all, * level_matches is 0. * - * If any space was allocated to hold 'ancestors' in the chain, - * the 'ancestor_maxitems' member will be greater than - * DNS_RBT_ANCESTORBLOCK and will indicate how many ancestors - * could have been stored; the amount to be freed from the rbt->mctx - * is ancestor_maxitems * sizeof(dns_rbtnode_t *). - * * When DNS_RBTFIND_NOEXACT is set, the closest matching superdomain is * returned (also subject to DNS_RBTFIND_EMPTYDATA), even when * there is an exact match in the tree. In this case, the chain @@ -452,19 +424,11 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * * chain->level_matches is 0. * - * If result is ISC_R_NOMEMORY: - * The function could not complete because memory could not - * be allocated to maintain the chain. However, it - * is possible that some memory was allocated; - * the chain's ancestor_maxitems will be greater than - * DNS_RBT_ANCESTORBLOCK if so. - * * Returns: * ISC_R_SUCCESS Success * DNS_R_PARTIALMATCH Superdomain found with data * ISC_R_NOTFOUND No match, or superdomain with no data - * ISC_R_NOMEMORY Resource Limit: Out of Memory building chain - * ISC_R_NOSPACE Concatenating nodes to form foundname failed + * ISC_R_NOSPACE Concatenating nodes to form foundname failed */ isc_result_t diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 5f5cb4b8dc..5c9b83cc5a 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -15,7 +15,7 @@ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: rbt.c,v 1.92 2000/08/03 21:34:27 bwelling Exp $ */ +/* $Id: rbt.c,v 1.93 2000/10/25 07:21:29 tale Exp $ */ /* Principal Authors: DCL */ @@ -46,6 +46,13 @@ #define CHAIN_MAGIC 0x302d302dU /* 0-0-. */ #define VALID_CHAIN(chain) ISC_MAGIC_VALID(chain, CHAIN_MAGIC) +#define RBT_HASH_SIZE 64 + +#ifdef RBT_MEM_TEST +#undef RBT_HASH_SIZE +#define RBT_HASH_SIZE 1 /* To give the reallocation code a workout. */ +#endif + struct dns_rbt { unsigned int magic; isc_mem_t * mctx; @@ -53,6 +60,8 @@ struct dns_rbt { void (*data_deleter)(void *, void *); void * deleter_arg; unsigned int nodecount; + unsigned int hashsize; + dns_rbtnode_t ** hashtable; }; #define RED 0 @@ -66,6 +75,8 @@ struct dns_rbt { #define LEFT(node) ((node)->left) #define RIGHT(node) ((node)->right) #define DOWN(node) ((node)->down) +#define HASHNEXT(node) ((node)->hashnext) +#define HASHVAL(node) ((node)->hashval) #define DATA(node) ((node)->data) #define FINDCALLBACK(node) ((node)->find_callback) #define COLOR(node) ((node)->color) @@ -102,17 +113,11 @@ struct dns_rbt { /* * Chain management. + * + * The "ancestors" member of chains were removed, with their job now + * being wholy handled by parent pointers (which didn't exist, because + * of memory concerns, when chains were first implemented). */ -#define ADD_ANCESTOR(chain, node) \ -do { \ - if ((chain)->ancestor_count == (chain)->ancestor_maxitems && \ - get_ancestor_mem(chain) != ISC_R_SUCCESS) { \ - dns_rbtnodechain_invalidate(chain); \ - return (ISC_R_NOMEMORY); \ - } \ - (chain)->ancestors[(chain)->ancestor_count++] = (node); \ -} while (0) - #define ADD_LEVEL(chain, node) \ (chain)->levels[(chain)->level_count++] = (node) @@ -152,10 +157,58 @@ Name(dns_rbtnode_t *node) { static void dns_rbt_printnodename(dns_rbtnode_t *node); #endif -#ifdef RBT_MEM_TEST -#undef DNS_RBT_ANCESTORBLOCK -#define DNS_RBT_ANCESTORBLOCK 1 /* To give the reallocation code a workout. */ -#endif +static inline dns_rbtnode_t * +find_up(dns_rbtnode_t *node) { + dns_rbtnode_t *root; + + /* + * Return the node in the level above the argument node that points + * to the level the argument node is in. If the argument node is in + * the top level, the return value is NULL. + */ + for (root = node; ! IS_ROOT(root); root = PARENT(root)) + ; /* Nothing. */ + + return(PARENT(root)); +} + +static inline unsigned int +name_hash(dns_name_t *name) { + unsigned int nlabels; + unsigned int hash; + dns_name_t tname; + + if (dns_name_countlabels(name) == 1) + return (dns_name_hash(name, ISC_FALSE)); + + dns_name_init(&tname, NULL); + nlabels = dns_name_countlabels(name); + hash = 0; + + for ( ; nlabels > 0; nlabels--) { + dns_name_getlabelsequence(name, nlabels - 1, 1, &tname); + hash += dns_name_hash(&tname, ISC_FALSE); + } + + return (hash); +} + +static inline void +compute_node_hash(dns_rbtnode_t *node) { + unsigned int hash; + dns_name_t name; + dns_rbtnode_t *up_node; + + dns_name_init(&name, NULL); + NODENAME(node, &name); + hash = name_hash(&name); + + up_node = find_up(node); + if (up_node != NULL) + hash += HASHVAL(up_node); + + HASHVAL(node) = hash; +} /* * Forward declarations. @@ -164,11 +217,10 @@ static isc_result_t create_node(isc_mem_t *mctx, dns_name_t *name, dns_rbtnode_t **nodep); static isc_result_t join_nodes(dns_rbt_t *rbt, dns_rbtnode_t *node); - static inline isc_result_t -get_ancestor_mem(dns_rbtnodechain_t *chain); +hash_node(dns_rbt_t *rbt, dns_rbtnode_t *node); static inline void -put_ancestor_mem(dns_rbtnodechain_t *chain); +unhash_node(dns_rbt_t *rbt, dns_rbtnode_t *node); static inline void rotate_left(dns_rbtnode_t *node, dns_rbtnode_t **rootp); @@ -176,8 +228,8 @@ static inline void rotate_right(dns_rbtnode_t *node, dns_rbtnode_t **rootp); static void -dns_rbt_addonlevel(dns_rbtnode_t *node, dns_rbtnode_t *current, int order, - dns_rbtnode_t **rootp, dns_rbtnodechain_t *chain); +dns_rbt_addonlevel(dns_rbtnode_t *node, dns_rbtnode_t *current, int order, + dns_rbtnode_t **rootp); static void dns_rbt_deletefromlevel(dns_rbtnode_t *delete, dns_rbtnode_t **rootp); @@ -207,6 +259,8 @@ dns_rbt_create(isc_mem_t *mctx, void (*deleter)(void *, void *), rbt->deleter_arg = deleter_arg; rbt->root = NULL; rbt->nodecount = 0; + rbt->hashtable = NULL; + rbt->hashsize = 0; rbt->magic = RBT_MAGIC; *rbtp = rbt; @@ -229,6 +283,10 @@ dns_rbt_destroy(dns_rbt_t **rbtp) { INSIST(rbt->nodecount == 0); + if (rbt->hashtable != NULL) + isc_mem_put(rbt->mctx, rbt->hashtable, + rbt->hashsize * sizeof(dns_rbtnode_t *)); + rbt->magic = 0; isc_mem_put(rbt->mctx, rbt, sizeof(*rbt)); @@ -242,54 +300,6 @@ dns_rbt_nodecount(dns_rbt_t *rbt) { return (rbt->nodecount); } -/* - * The next three functions for chains, get_ancestor_mem, put_ancestor_mem - * and chain_name, appear early in this file so they can be effectively - * inlined by the other rbt functions that use them. - */ -static inline isc_result_t -get_ancestor_mem(dns_rbtnodechain_t *chain) { - dns_rbtnode_t **ancestor_mem; - int oldsize, newsize; - - oldsize = chain->ancestor_maxitems * sizeof(dns_rbtnode_t *); - newsize = oldsize + DNS_RBT_ANCESTORBLOCK * sizeof(dns_rbtnode_t *); - - if (oldsize == 0) { - chain->ancestors = chain->ancestor_block; - } else { - ancestor_mem = isc_mem_get(chain->mctx, newsize); - - if (ancestor_mem == NULL) - return (ISC_R_NOMEMORY); - - memcpy(ancestor_mem, chain->ancestors, oldsize); - - if (chain->ancestor_maxitems > DNS_RBT_ANCESTORBLOCK) - isc_mem_put(chain->mctx, chain->ancestors, oldsize); - - chain->ancestors = ancestor_mem; - } - - chain->ancestor_maxitems += DNS_RBT_ANCESTORBLOCK; - - return (ISC_R_SUCCESS); -} - -/* - * This is used by functions that are popping the chain off their - * own stack, and so do not need to have ancestor_maxitems or the - * ancestors pointer reset. Functions that will be reusing a chain - * structure need to call dns_rbtnodechain_reset() instead. - */ -static inline void -put_ancestor_mem(dns_rbtnodechain_t *chain) { - if (chain->ancestor_maxitems > DNS_RBT_ANCESTORBLOCK) - isc_mem_put(chain->mctx, chain->ancestors, - chain->ancestor_maxitems - * sizeof(dns_rbtnode_t *)); -} - static inline isc_result_t chain_name(dns_rbtnodechain_t *chain, dns_name_t *name, isc_boolean_t include_chain_end) @@ -325,15 +335,12 @@ move_chain_to_last(dns_rbtnodechain_t *chain, dns_rbtnode_t *node) { * Go as far right and then down as much as possible, * as long as the rightmost node has a down pointer. */ - while (RIGHT(node) != NULL) { - ADD_ANCESTOR(chain, node); + while (RIGHT(node) != NULL) node = RIGHT(node); - } if (DOWN(node) == NULL) break; - ADD_ANCESTOR(chain, NULL); ADD_LEVEL(chain, node); node = DOWN(node); } while (1); @@ -382,12 +389,12 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { IS_ROOT(new_current) = ISC_TRUE; rbt->root = new_current; *nodep = new_current; + hash_node(rbt, new_current); } return (result); } dns_rbtnodechain_init(&chain, rbt->mctx); - ADD_ANCESTOR(&chain, NULL); dns_fixedname_init(&fixedprefix); dns_fixedname_init(&fixedsuffix); @@ -400,15 +407,10 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { current = NULL; child = *root; dns_name_init(¤t_name, current_offsets); + do { current = child; - INSIST((IS_ROOT(current) && - chain.ancestors[chain.ancestor_count - 1] == NULL) || - (! IS_ROOT(current) && - chain.ancestors[chain.ancestor_count - 1] == - PARENT(current))); - NODENAME(current, ¤t_name); compared = dns_name_fullcompare(add_name, ¤t_name, &order, @@ -422,15 +424,14 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { } if (compared == dns_namereln_none) { + if (order < 0) { parent = current; child = LEFT(current); - ADD_ANCESTOR(&chain, current); } else if (order > 0) { parent = current; child = RIGHT(current); - ADD_ANCESTOR(&chain, current); } @@ -446,7 +447,6 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { * the non-common parts of these two names should * start a new tree. */ - if (compared == dns_namereln_subdomain) { /* * All of the existing labels are in common, @@ -474,7 +474,6 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { parent = NULL; child = DOWN(current); - ADD_ANCESTOR(&chain, NULL); ADD_LEVEL(&chain, current); } else { @@ -494,7 +493,14 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { * does not exceed the number of logical * levels allowed by DNSSEC. * - * XXX DCL need a better error result? + * XXXDCL need a better error result? + * + * XXXDCL Since chain ancestors were removed, + * no longer used by dns_rbt_addonlevel(), + * this is the only real use of chains in the + * function. It could be done instead with + * a simple integer variable, but I am pressed + * for time. */ if (chain.level_count == (sizeof(chain.levels) / @@ -620,7 +626,6 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { DOWN(new_current) = current; root = &DOWN(new_current); - ADD_ANCESTOR(&chain, NULL); ADD_LEVEL(&chain, new_current); LEFT(current) = NULL; @@ -630,6 +635,7 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { ATTRS(current) &= ~DNS_NAMEATTR_ABSOLUTE; rbt->nodecount++; + hash_node(rbt, new_current); if (common_labels == dns_name_countlabels(add_name) && @@ -640,7 +646,6 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { * a new level. */ *nodep = new_current; - put_ancestor_mem(&chain); return (ISC_R_SUCCESS); } else { @@ -649,11 +654,8 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { * because it is just a placeholder. * Its data pointer is already NULL * from create_node()), so there's - * nothing more to do to but add it to - * the ancestor chain, because it will - * be the parent node of the new node. + * nothing more to do to it. */ - ADD_ANCESTOR(&chain, current); /* The not-in-common parts of the new * name will be inserted into the new @@ -681,13 +683,12 @@ dns_rbt_addnode(dns_rbt_t *rbt, dns_name_t *name, dns_rbtnode_t **nodep) { result = create_node(rbt->mctx, add_name, &new_current); if (result == ISC_R_SUCCESS) { - dns_rbt_addonlevel(new_current, current, order, root, &chain); + dns_rbt_addonlevel(new_current, current, order, root); rbt->nodecount++; + hash_node(rbt, new_current); *nodep = new_current; } - put_ancestor_mem(&chain); - return (result); } @@ -730,7 +731,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, unsigned int options, dns_rbtfindcallback_t callback, void *callback_arg) { - dns_rbtnode_t *current; + dns_rbtnode_t *current, *last_compared; dns_rbtnodechain_t localchain; dns_name_t *search_name, current_name, *callback_name; dns_fixedname_t fixedcallbackname, fixedsearchname; @@ -738,6 +739,7 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, isc_result_t result, saved_result; unsigned int common_labels, common_bits; int order; + unsigned int hash; REQUIRE(VALID_RBT(rbt)); REQUIRE(dns_name_isabsolute(name)); @@ -759,6 +761,14 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, if (rbt->root == NULL) return (ISC_R_NOTFOUND); + else { + /* + * Appease GCC about variables it incorrectly thinks are + * possibly used unitialized. + */ + compared = dns_namereln_none; + last_compared = NULL; + } dns_fixedname_init(&fixedcallbackname); callback_name = dns_fixedname_name(&fixedcallbackname); @@ -776,22 +786,92 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, dns_name_init(¤t_name, NULL); - ADD_ANCESTOR(chain, NULL); - saved_result = ISC_R_SUCCESS; current = rbt->root; + while (current != NULL) { NODENAME(current, ¤t_name); compared = dns_name_fullcompare(search_name, ¤t_name, &order, &common_labels, &common_bits); + last_compared = current; if (compared == dns_namereln_equal) break; if (compared == dns_namereln_none) { - ADD_ANCESTOR(chain, current); + dns_name_t hash_name; + dns_rbtnode_t *hnode; + dns_rbtnode_t *up_current; + unsigned int nlabels; + unsigned int tlabels = 1; + if (rbt->hashtable == NULL) + goto nohash; + + nlabels = dns_name_countlabels(search_name); + + /* + * In this case, PARENT == UP, since the only + * nodes that are ever examined at a level are the + * root node and the "correct" node. If + * compared == dns_namereln_none, this must not + * be the "correct" node. + */ + up_current = PARENT(current); + dns_name_init(&hash_name, NULL); + + hashagain: + dns_name_getlabelsequence(search_name, + nlabels - tlabels, + tlabels, &hash_name); + hash = HASHVAL(up_current) + name_hash(&hash_name); + + for (hnode = rbt->hashtable[hash % rbt->hashsize]; + hnode != NULL; + hnode = hnode->hashnext) + { + dns_name_t hnode_name; + + if (hash != HASHVAL(hnode)) + continue; + if (find_up(hnode) != up_current) + continue; + dns_name_init(&hnode_name, NULL); + NODENAME(hnode, &hnode_name); + if (dns_name_equal(&hnode_name, &hash_name)) + break; + } + + if (hnode != NULL) { + current = hnode; + continue; + } + + if (tlabels++ < nlabels) + goto hashagain; + + /* + * XXXDCL Bitstring labels complicate things, as usual. + * Checking for the situation could be done up by + * the dns_name_getlabelsequence so that they could + * still use the hashing code, but it would be messy + * to repeatedly try various bitstring lengths. + * Best to keep the issue out of the way down here for + * now by punting to the traditional binary search. + * + * Yes, accessing the name structure data directly + * is evil. This function gets used to much to slow + * it down with the name.h function call APIs. + */ + if (search_name->ndata[search_name->offsets[nlabels-1]] + != DNS_LABELTYPE_BITSTRING) { + current = NULL; + continue; + } + + /* FALLTHROUGH */ + nohash: /* * Standard binary search tree movement. */ @@ -840,7 +920,6 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * function needs the chain pointed to the * next level. */ - ADD_ANCESTOR(chain, NULL); ADD_LEVEL(chain, current); /* @@ -885,14 +964,10 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * entire name at this node is not common * with the search name so the search * name does not exist in the tree. - * Add this node to the ancestor chain - * to simplify things for the chain fixing - * logic below then end the loop. */ INSIST(compared == dns_namereln_commonancestor || compared == dns_namereln_contains); - ADD_ANCESTOR(chain, current); current = NULL; } } @@ -990,16 +1065,8 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * Since there was no exact match, the chain argument * needs to be pointed at the DNSSEC predecessor of * the search name. - * - * First, point current to the node that stopped the - * search, and remove that node from the ancestor - * history. */ - - INSIST(chain->ancestor_count > 0); - current = chain->ancestors[--chain->ancestor_count]; - - if (current == NULL) { + if (compared == dns_namereln_subdomain) { /* * Attempted to follow a down pointer that was * NULL, which means the searched for name was @@ -1017,6 +1084,51 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, } else { isc_result_t result2; + /* + * Point current to the node that stopped + * the search. + * + * With the hashing modification that has been + * added to the algorithm, the stop node of a + * standard binary search is not known. So it + * has to be found. There is probably a more + * clever way of doing this. + * + * The assignment of current to NULL when + * the relationship is *not* dns_namereln_none, + * even though it later gets set to the same + * last_compared anyway, is simply to not push + * the while loop in one more level of + * indentation. + */ + if (compared == dns_namereln_none) + current = last_compared; + else + current = NULL; + + while (current != NULL) { + NODENAME(current, ¤t_name); + compared = dns_name_fullcompare( + search_name, + ¤t_name, + &order, + &common_labels, + &common_bits); + + last_compared = current; + + /* + * Standard binary search movement. + */ + if (order < 0) + current = LEFT(current); + else + current = RIGHT(current); + + } + + current = last_compared; + /* * Reached a point within a level tree that * positively indicates the name is not @@ -1043,9 +1155,9 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, * no predecessor? */ + if (order > 0) { if (DOWN(current) != NULL) { - ADD_ANCESTOR(chain, NULL); ADD_LEVEL(chain, current); result2 = @@ -1086,9 +1198,6 @@ dns_rbt_findnode(dns_rbt_t *rbt, dns_name_t *name, dns_name_t *foundname, } } - if (chain == &localchain) - put_ancestor_mem(chain); - return (result); } @@ -1161,7 +1270,7 @@ dns_rbt_deletename(dns_rbt_t *rbt, dns_name_t *name, isc_boolean_t recurse) { isc_result_t dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, isc_boolean_t recurse) { - dns_rbtnode_t *parent, *root; + dns_rbtnode_t *parent; isc_result_t result; REQUIRE(VALID_RBT(rbt)); @@ -1204,10 +1313,7 @@ dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, isc_boolean_t recurse) * deleted. If the deleted node is the top level, parent will be set * to NULL. */ - for (root = node; ! IS_ROOT(root); root = PARENT(root)) - ; /* Nothing. */ - - parent = PARENT(root); + parent = find_up(node); /* * This node now has no down pointer (either because it didn't @@ -1219,6 +1325,8 @@ dns_rbt_deletenode(dns_rbt_t *rbt, dns_rbtnode_t *node, isc_boolean_t recurse) if (rbt->data_deleter != NULL) rbt->data_deleter(DATA(node), rbt->deleter_arg); + + unhash_node(rbt, node); isc_mem_put(rbt->mctx, node, NODE_SIZE(node)); rbt->nodecount--; @@ -1274,6 +1382,8 @@ create_node(isc_mem_t *mctx, dns_name_t *name, dns_rbtnode_t **nodep) { LEFT(node) = NULL; DOWN(node) = NULL; DATA(node) = NULL; + HASHNEXT(node) = NULL; + HASHVAL(node) = 0; LOCKNUM(node) = 0; REFS(node) = 0; @@ -1346,8 +1456,6 @@ join_nodes(dns_rbt_t *rbt, dns_rbtnode_t *node) { oldlength = NAMELEN(down) + OFFSETLEN(down); if (newlength > oldlength + PADBYTES(down)) { result = create_node(rbt->mctx, newname, &newnode); - if (result == ISC_R_SUCCESS) - rbt->nodecount++; } else { memcpy(NAME(down), newname->ndata, newname->length); PADBYTES(down) -= newlength - oldlength; @@ -1393,18 +1501,154 @@ join_nodes(dns_rbt_t *rbt, dns_rbtnode_t *node) { if (DOWN(down) != NULL) PARENT(DOWN(down)) = newnode; - isc_mem_put(rbt->mctx, node, NODE_SIZE(node)); rbt->nodecount--; + hash_node(rbt, newnode); + + unhash_node(rbt, node); + isc_mem_put(rbt->mctx, node, NODE_SIZE(node)); if (newnode != down) { + unhash_node(rbt, down); isc_mem_put(rbt->mctx, down, NODE_SIZE(down)); - rbt->nodecount--; } + } return (result); } +static inline void +hash_add_node(dns_rbt_t *rbt, dns_rbtnode_t *node) { + unsigned int hash; + + compute_node_hash(node); + + hash = HASHVAL(node) % rbt->hashsize; + HASHNEXT(node) = rbt->hashtable[hash]; + + rbt->hashtable[hash] = node; +} + +static void +inithash_internal(dns_rbt_t *rbt, dns_rbtnode_t *node) { + if (node == NULL) + return; + + hash_add_node(rbt, node); + + inithash_internal(rbt, DOWN(node)); + inithash_internal(rbt, LEFT(node)); + inithash_internal(rbt, RIGHT(node)); +} + +static isc_result_t +inithash(dns_rbt_t *rbt) { + unsigned int bytes; + + rbt->hashsize = RBT_HASH_SIZE * 2; + bytes = rbt->hashsize * sizeof(dns_rbtnode_t *); + rbt->hashtable = isc_mem_get(rbt->mctx, bytes); + + if (rbt->hashtable == NULL) + return (ISC_R_NOMEMORY); + + memset(rbt->hashtable, 0, bytes); + + inithash_internal(rbt, rbt->root); + + return (ISC_R_SUCCESS); +} + +static isc_result_t +rehash(dns_rbt_t *rbt) { + unsigned int oldsize; + dns_rbtnode_t **oldtable; + dns_rbtnode_t *node; + unsigned int hash; + unsigned int i; + + oldsize = rbt->hashsize; + rbt->hashsize *= 2; + oldtable = rbt->hashtable; + rbt->hashtable = isc_mem_get(rbt->mctx, + rbt->hashsize * sizeof(dns_rbtnode_t *)); + if (rbt->hashtable == NULL) + return (ISC_R_NOMEMORY); + + for (i = 0; i < rbt->hashsize; i++) + rbt->hashtable[i] = NULL; + + for (i = 0; i < oldsize; i++) { + node = oldtable[i]; + while (node != NULL) { + hash = HASHVAL(node) % rbt->hashsize; + oldtable[i] = HASHNEXT(node); + HASHNEXT(node) = rbt->hashtable[hash]; + rbt->hashtable[hash] = node; + node = oldtable[i]; + } + } + + isc_mem_put(rbt->mctx, oldtable, + rbt->hashsize * sizeof(dns_rbtnode_t *) / 2); + + return (ISC_R_SUCCESS); +} + +static inline isc_result_t +hash_node(dns_rbt_t *rbt, dns_rbtnode_t *node) { + isc_result_t result = ISC_R_SUCCESS; + + if (rbt->hashtable == NULL) { + if (rbt->nodecount == RBT_HASH_SIZE) + result = inithash(rbt); + return (result); + + } else if (rbt->nodecount >= rbt->hashsize) + result = rehash(rbt); + + if (result == ISC_R_SUCCESS) + hash_add_node(rbt, node); + + return (result); +} + +static inline void +unhash_node(dns_rbt_t *rbt, dns_rbtnode_t *node) { + unsigned int bucket; + dns_rbtnode_t *bucket_node; + + if (rbt->hashtable != NULL) { + bucket = HASHVAL(node) % rbt->hashsize; + bucket_node = rbt->hashtable[bucket]; + + if (bucket_node == node) + rbt->hashtable[bucket] = HASHNEXT(node); + else { + while (HASHNEXT(bucket_node) != node) { + /* + + INSIST(HASHNEXT(bucket_node) != NULL); + + * XXXDCL This is WRONG and I have not + * yet had the chance to figure out why. + * The assertion is triggered by + * bin/tests/t_rbt -x -t 4 + * It is, of course, a bitstring problem. + * I will figure it out next week after + * my move. + + */ + if (HASHNEXT(bucket_node) == NULL) + break; + + bucket_node = HASHNEXT(bucket_node); + } + HASHNEXT(bucket_node) = HASHNEXT(node); + } + } +} + static inline void rotate_left(dns_rbtnode_t *node, dns_rbtnode_t **rootp) { dns_rbtnode_t *child; @@ -1476,14 +1720,12 @@ rotate_right(dns_rbtnode_t *node, dns_rbtnode_t **rootp) { * true red/black tree on a single level. */ static void -dns_rbt_addonlevel(dns_rbtnode_t *node, - dns_rbtnode_t *current, int order, - dns_rbtnode_t **rootp, dns_rbtnodechain_t *chain) +dns_rbt_addonlevel(dns_rbtnode_t *node, dns_rbtnode_t *current, int order, + dns_rbtnode_t **rootp) { - dns_rbtnode_t *child, *root, *tmp, *parent, *grandparent; + dns_rbtnode_t *child, *root, *parent, *grandparent; dns_name_t add_name, current_name; dns_offsets_t add_offsets, current_offsets; - unsigned int depth; REQUIRE(rootp != NULL); REQUIRE(node != NULL && LEFT(node) == NULL && RIGHT(node) == NULL); @@ -1522,13 +1764,15 @@ dns_rbt_addonlevel(dns_rbtnode_t *node, MAKE_RED(node); - depth = chain->ancestor_count - 1; + while (node != root && IS_RED(PARENT(node))) { + /* + * XXXDCL could do away with separate parent and grandparent + * variables. They are vestiges of the days before parent + * pointers. However, they make the code a little clearer. + */ - while (node != root && IS_RED(chain->ancestors[depth])) { - INSIST(depth > 0); - - parent = chain->ancestors[depth]; - grandparent = chain->ancestors[depth - 1]; + parent = PARENT(node); + grandparent = PARENT(parent); if (parent == LEFT(grandparent)) { child = RIGHT(grandparent); @@ -1537,18 +1781,15 @@ dns_rbt_addonlevel(dns_rbtnode_t *node, MAKE_BLACK(child); MAKE_RED(grandparent); node = grandparent; - depth -= 2; } else { if (node == RIGHT(parent)) { rotate_left(parent, &root); - tmp = node; node = parent; - parent = tmp; - chain->ancestors[depth] = parent; + parent = PARENT(node); + grandparent = PARENT(parent); } MAKE_BLACK(parent); MAKE_RED(grandparent); - INSIST(depth > 1); rotate_right(grandparent, &root); } } else { @@ -1558,18 +1799,15 @@ dns_rbt_addonlevel(dns_rbtnode_t *node, MAKE_BLACK(child); MAKE_RED(grandparent); node = grandparent; - depth -= 2; } else { if (node == LEFT(parent)) { rotate_right(parent, &root); - tmp = node; node = parent; - parent = tmp; - chain->ancestors[depth] = parent; + parent = PARENT(node); + grandparent = PARENT(parent); } MAKE_BLACK(parent); MAKE_RED(grandparent); - INSIST(depth > 1); rotate_left(grandparent, &root); } } @@ -1831,7 +2069,6 @@ dns_rbt_deletefromlevel(dns_rbtnode_t *delete, dns_rbtnode_t **rootp) { */ static void dns_rbt_deletetree(dns_rbt_t *rbt, dns_rbtnode_t *node) { - REQUIRE(VALID_RBT(rbt)); if (node == NULL) @@ -1847,6 +2084,7 @@ dns_rbt_deletetree(dns_rbt_t *rbt, dns_rbtnode_t *node) { if (DATA(node) != NULL && rbt->data_deleter != NULL) rbt->data_deleter(DATA(node), rbt->deleter_arg); + unhash_node(rbt, node); isc_mem_put(rbt->mctx, node, NODE_SIZE(node)); rbt->nodecount--; } @@ -1957,9 +2195,6 @@ dns_rbtnodechain_init(dns_rbtnodechain_t *chain, isc_mem_t *mctx) { chain->mctx = mctx; chain->end = NULL; - chain->ancestors = chain->ancestor_block; - chain->ancestor_count = 0; - chain->ancestor_maxitems = DNS_RBT_ANCESTORBLOCK; chain->level_count = 0; chain->level_matches = 0; @@ -2025,21 +2260,27 @@ dns_rbtnodechain_prev(dns_rbtnodechain_t *chain, dns_name_t *name, current = chain->end; if (LEFT(current) != NULL) { - ADD_ANCESTOR(chain, current); + /* + * Moving left one then right as far as possible is the + * previous node, at least for this level. + */ current = LEFT(current); - while (RIGHT(current) != NULL) { - ADD_ANCESTOR(chain, current); + while (RIGHT(current) != NULL) current = RIGHT(current); - } predecessor = current; } else { - while (chain->ancestors[chain->ancestor_count - 1] != NULL) { - INSIST(chain->ancestor_count > 1); + /* + * No left links, so move toward the root. If at any point on + * the way there the link from parent to child is a right + * link, then the parent is the previous node, at least + * for this level. + */ + while (! IS_ROOT(current)) { previous = current; - current = chain->ancestors[--chain->ancestor_count]; + current = PARENT(current); if (RIGHT(current) == previous) { predecessor = current; @@ -2049,6 +2290,10 @@ dns_rbtnodechain_prev(dns_rbtnodechain_t *chain, dns_name_t *name, } if (predecessor != NULL) { + /* + * Found a predecessor node in this level. It might not + * really be the predecessor, however. + */ if (DOWN(predecessor) != NULL) { /* * The predecessor is really down at least one level. @@ -2062,17 +2307,14 @@ dns_rbtnodechain_prev(dns_rbtnodechain_t *chain, dns_name_t *name, * whether it is truly what Bob calls a * new origin. */ - ADD_ANCESTOR(chain, NULL); ADD_LEVEL(chain, predecessor); predecessor = DOWN(predecessor); /* XXX DCL duplicated from above; clever * way to unduplicate? */ - while (RIGHT(predecessor) != NULL) { - ADD_ANCESTOR(chain, predecessor); + while (RIGHT(predecessor) != NULL) predecessor = RIGHT(predecessor); - } } while (DOWN(predecessor) != NULL); /* XXX DCL probably needs work on the concept */ @@ -2082,13 +2324,13 @@ dns_rbtnodechain_prev(dns_rbtnodechain_t *chain, dns_name_t *name, } else if (chain->level_count > 0) { /* + * Dang, didn't find a predecessor in this level. * Got to the root of this level without having traversed - * any right links. Ascend the tree one level. + * any right links. Ascend the tree one level; the + * node that points to this tree is the predecessor. */ - INSIST(chain->level_count > 0 && - chain->ancestor_count > 0); + INSIST(chain->level_count > 0 && IS_ROOT(current)); predecessor = chain->levels[--chain->level_count]; - chain->ancestor_count--; /* XXX DCL probably needs work on the concept */ /* @@ -2134,6 +2376,10 @@ dns_rbtnodechain_next(dns_rbtnodechain_t *chain, dns_name_t *name, current = chain->end; + /* + * If there is a level below this node, the next node is the leftmost + * node of the next level. + */ if (DOWN(current) != NULL) { /* * Don't declare an origin change when the new origin is "." @@ -2144,14 +2390,11 @@ dns_rbtnodechain_next(dns_rbtnodechain_t *chain, dns_name_t *name, OFFSETLEN(current) > 1) new_origin = ISC_TRUE; - ADD_ANCESTOR(chain, NULL); ADD_LEVEL(chain, current); current = DOWN(current); - while (LEFT(current) != NULL) { - ADD_ANCESTOR(chain, current); + while (LEFT(current) != NULL) current = LEFT(current); - } successor = current; @@ -2167,12 +2410,9 @@ dns_rbtnodechain_next(dns_rbtnodechain_t *chain, dns_name_t *name, * ascents until either case is true. */ do { - while (chain->ancestors[chain->ancestor_count - 1] != - NULL) { - INSIST(chain->ancestor_count > 1); + while (! IS_ROOT(current)) { previous = current; - current = - chain->ancestors[--chain->ancestor_count]; + current = PARENT(current); if (LEFT(current) == previous) { successor = current; @@ -2188,9 +2428,7 @@ dns_rbtnodechain_next(dns_rbtnodechain_t *chain, dns_name_t *name, if (chain->level_count == 0) break; - INSIST(chain->ancestor_count > 0); current = chain->levels[--chain->level_count]; - chain->ancestor_count--; new_origin = ISC_TRUE; if (RIGHT(current) != NULL) @@ -2200,13 +2438,10 @@ dns_rbtnodechain_next(dns_rbtnodechain_t *chain, dns_name_t *name, } if (successor == NULL && RIGHT(current) != NULL) { - ADD_ANCESTOR(chain, current); current = RIGHT(current); - while (LEFT(current) != NULL) { - ADD_ANCESTOR(chain, current); + while (LEFT(current) != NULL) current = LEFT(current); - } successor = current; } @@ -2254,8 +2489,6 @@ dns_rbtnodechain_first(dns_rbtnodechain_t *chain, dns_rbt_t *rbt, dns_rbtnodechain_reset(chain); - ADD_ANCESTOR(chain, NULL); - chain->end = rbt->root; result = dns_rbtnodechain_current(chain, name, origin, NULL); @@ -2278,8 +2511,6 @@ dns_rbtnodechain_last(dns_rbtnodechain_t *chain, dns_rbt_t *rbt, dns_rbtnodechain_reset(chain); - ADD_ANCESTOR(chain, NULL); - result = move_chain_to_last(chain, rbt->root); if (result != ISC_R_SUCCESS) return (result); @@ -2302,11 +2533,7 @@ dns_rbtnodechain_reset(dns_rbtnodechain_t *chain) { REQUIRE(VALID_CHAIN(chain)); - put_ancestor_mem(chain); chain->end = NULL; - chain->ancestors = chain->ancestor_block; - chain->ancestor_count = 0; - chain->ancestor_maxitems = DNS_RBT_ANCESTORBLOCK; chain->level_count = 0; chain->level_matches = 0; }