From: Michael Adam <obnox@samba.org>
Date: Wed, 25 Feb 2009 08:53:16 +0000 (+0100)
Subject: s3:registry: hash the list of subkeys in the regsubkey_ctr
X-Git-Tag: build_3.3.2_ctdb_1~33
X-Git-Url: http://git.samba.org/?a=commitdiff_plain;h=d306b158231658458c4cdadc44afee79c648fba2;p=obnox%2Fsamba-ctdb.git

s3:registry: hash the list of subkeys in the regsubkey_ctr

This removes many loops over all the arrays
(from regsubkey_ctr_key_exists) and thus
reduces "net conf drop" from 1m55 to 48seconds
and "net conf import" from 1m55 to 58 seconds
for 2000 shares on my box.

Michael

Signed-off-by: Michael Adam <obnox@samba.org>
---

diff --git a/source/include/proto.h b/source/include/proto.h
index 5ed46d4ce2..1fa7f90595 100644
--- a/source/include/proto.h
+++ b/source/include/proto.h
@@ -6805,7 +6805,7 @@ WERROR regsubkey_ctr_init(TALLOC_CTX *mem_ctx, struct regsubkey_ctr **ctr);
 WERROR regsubkey_ctr_set_seqnum(struct regsubkey_ctr *ctr, int seqnum);
 int regsubkey_ctr_get_seqnum(struct regsubkey_ctr *ctr);
 WERROR regsubkey_ctr_addkey( struct regsubkey_ctr *ctr, const char *keyname );
-int regsubkey_ctr_delkey( struct regsubkey_ctr *ctr, const char *keyname );
+WERROR regsubkey_ctr_delkey( struct regsubkey_ctr *ctr, const char *keyname );
 bool regsubkey_ctr_key_exists( struct regsubkey_ctr *ctr, const char *keyname );
 int regsubkey_ctr_numkeys( struct regsubkey_ctr *ctr );
 char* regsubkey_ctr_specific_key( struct regsubkey_ctr *ctr, uint32 key_index );
diff --git a/source/registry/reg_api.c b/source/registry/reg_api.c
index 5e1e276392..64c34f93ac 100644
--- a/source/registry/reg_api.c
+++ b/source/registry/reg_api.c
@@ -552,7 +552,6 @@ WERROR reg_deletekey(struct registry_key *parent, const char *path)
 	WERROR err;
 	TALLOC_CTX *mem_ctx;
 	char *name, *end;
-	int num_subkeys;
 	struct registry_key *tmp_key, *key;
 
 	if (!(mem_ctx = talloc_init("reg_createkey"))) return WERR_NOMEM;
@@ -598,10 +597,8 @@ WERROR reg_deletekey(struct registry_key *parent, const char *path)
 		goto error;
 	}
 
-	num_subkeys = regsubkey_ctr_numkeys(parent->subkeys);
-
-	if (regsubkey_ctr_delkey(parent->subkeys, name) == num_subkeys) {
-		err = WERR_BADFILE;
+	err = regsubkey_ctr_delkey(parent->subkeys, name);
+	if (!W_ERROR_IS_OK(err)) {
 		goto error;
 	}
 
@@ -1151,6 +1148,9 @@ static WERROR reg_deletekey_recursive_trans(TALLOC_CTX *ctx,
 	werr = reg_deletekey_recursive_internal(ctx, parent, path, del_key);
 
 	if (!W_ERROR_IS_OK(werr)) {
+		DEBUG(1, (__location__ " failed to delete key '%s' from key "
+			  "'%s': %s\n", path, parent->key->name,
+			  dos_errstr(werr)));
 		werr = regdb_transaction_cancel();
 		if (!W_ERROR_IS_OK(werr)) {
 			DEBUG(0, ("reg_deletekey_recursive_trans: "
diff --git a/source/registry/reg_objects.c b/source/registry/reg_objects.c
index c3d67a01ed..ec358a3ed5 100644
--- a/source/registry/reg_objects.c
+++ b/source/registry/reg_objects.c
@@ -25,8 +25,9 @@
 #define DBGC_CLASS DBGC_REGISTRY
 
 struct regsubkey_ctr {
-	uint32          num_subkeys;
+	uint32_t        num_subkeys;
 	char            **subkeys;
+	struct db_context *subkeys_hash;
 	int seqnum;
 };
 
@@ -53,6 +54,12 @@ WERROR regsubkey_ctr_init(TALLOC_CTX *mem_ctx, struct regsubkey_ctr **ctr)
 		return WERR_NOMEM;
 	}
 
+	(*ctr)->subkeys_hash = db_open_rbt(*ctr);
+	if ((*ctr)->subkeys_hash == NULL) {
+		talloc_free(*ctr);
+		return WERR_NOMEM;
+	}
+
 	return WERR_OK;
 }
 
@@ -76,6 +83,68 @@ int regsubkey_ctr_get_seqnum(struct regsubkey_ctr *ctr)
 	return ctr->seqnum;
 }
 
+static WERROR regsubkey_ctr_hash_keyname(struct regsubkey_ctr *ctr,
+					 const char *keyname,
+					 uint32 idx)
+{
+	WERROR werr;
+
+	werr = ntstatus_to_werror(dbwrap_store_bystring(ctr->subkeys_hash,
+						keyname,
+						make_tdb_data((uint8 *)&idx,
+							      sizeof(idx)),
+						TDB_REPLACE));
+	if (!W_ERROR_IS_OK(werr)) {
+		DEBUG(1, ("error hashing new key '%s' in container: %s\n",
+			  keyname, dos_errstr(werr)));
+	}
+
+	return werr;
+}
+
+static WERROR regsubkey_ctr_unhash_keyname(struct regsubkey_ctr *ctr,
+					   const char *keyname)
+{
+	WERROR werr;
+
+	werr = ntstatus_to_werror(dbwrap_delete_bystring(ctr->subkeys_hash,
+				  keyname));
+	if (!W_ERROR_IS_OK(werr)) {
+		DEBUG(1, ("error unhashing key '%s' in container: %s\n",
+			  keyname, dos_errstr(werr)));
+	}
+
+	return werr;
+}
+
+static WERROR regsubkey_ctr_index_for_keyname(struct regsubkey_ctr *ctr,
+					      const char *keyname,
+					      uint32 *idx)
+{
+	TDB_DATA data;
+
+	if ((ctr == NULL) || (keyname == NULL)) {
+		return WERR_INVALID_PARAM;
+	}
+
+	data = dbwrap_fetch_bystring(ctr->subkeys_hash, ctr, keyname);
+	if (data.dptr == NULL) {
+		return WERR_NOT_FOUND;
+	}
+
+	if (data.dsize != sizeof(*idx)) {
+		talloc_free(data.dptr);
+		return WERR_INVALID_DATATYPE;
+	}
+
+	if (idx != NULL) {
+		*idx = *(uint32 *)data.dptr;
+	}
+
+	talloc_free(data.dptr);
+	return WERR_OK;
+}
+
 /***********************************************************************
  Add a new key to the array
  **********************************************************************/
@@ -83,6 +152,7 @@ int regsubkey_ctr_get_seqnum(struct regsubkey_ctr *ctr)
 WERROR regsubkey_ctr_addkey( struct regsubkey_ctr *ctr, const char *keyname )
 {
 	char **newkeys;
+	WERROR werr;
 
 	if ( !keyname ) {
 		return WERR_OK;
@@ -108,6 +178,10 @@ WERROR regsubkey_ctr_addkey( struct regsubkey_ctr *ctr, const char *keyname )
 		 */
 		return WERR_NOMEM;
 	}
+
+	werr = regsubkey_ctr_hash_keyname(ctr, keyname, ctr->num_subkeys);
+	W_ERROR_NOT_OK_RETURN(werr);
+
 	ctr->num_subkeys++;
 
 	return WERR_OK;
@@ -117,30 +191,37 @@ WERROR regsubkey_ctr_addkey( struct regsubkey_ctr *ctr, const char *keyname )
  Delete a key from the array
  **********************************************************************/
 
-int regsubkey_ctr_delkey( struct regsubkey_ctr *ctr, const char *keyname )
+WERROR regsubkey_ctr_delkey( struct regsubkey_ctr *ctr, const char *keyname )
 {
-	int i;
+	WERROR werr;
+	uint32 idx, j;
 
-	if ( !keyname )
-		return ctr->num_subkeys;
+	if (keyname == NULL) {
+		return WERR_INVALID_PARAM;
+	}
 
 	/* make sure the keyname is actually already there */
 
-	for ( i=0; i<ctr->num_subkeys; i++ ) {
-		if ( strequal( ctr->subkeys[i], keyname ) )
-			break;
-	}
+	werr = regsubkey_ctr_index_for_keyname(ctr, keyname, &idx);
+	W_ERROR_NOT_OK_RETURN(werr);
 
-	if ( i == ctr->num_subkeys )
-		return ctr->num_subkeys;
+	werr = regsubkey_ctr_unhash_keyname(ctr, keyname);
+	W_ERROR_NOT_OK_RETURN(werr);
 
 	/* update if we have any keys left */
 	ctr->num_subkeys--;
-	if ( i < ctr->num_subkeys )
-		memmove(&ctr->subkeys[i], &ctr->subkeys[i+1],
-			sizeof(char*) * (ctr->num_subkeys-i));
+	if (idx < ctr->num_subkeys) {
+		memmove(&ctr->subkeys[idx], &ctr->subkeys[idx+1],
+			sizeof(char *) * (ctr->num_subkeys - idx));
+
+		/* we have to re-hash rest of the array...  :-( */
+		for (j = idx; j < ctr->num_subkeys; j++) {
+			werr = regsubkey_ctr_hash_keyname(ctr, ctr->subkeys[j], j);
+			W_ERROR_NOT_OK_RETURN(werr);
+		}
+	}
 
-	return ctr->num_subkeys;
+	return WERR_OK;
 }
 
 /***********************************************************************
@@ -149,18 +230,18 @@ int regsubkey_ctr_delkey( struct regsubkey_ctr *ctr, const char *keyname )
 
 bool regsubkey_ctr_key_exists( struct regsubkey_ctr *ctr, const char *keyname )
 {
-	int 	i;
+	WERROR werr;
 
 	if (!ctr->subkeys) {
 		return False;
 	}
 
-	for ( i=0; i<ctr->num_subkeys; i++ ) {
-		if ( strequal( ctr->subkeys[i],keyname ) )
-			return True;
+	werr = regsubkey_ctr_index_for_keyname(ctr, keyname, NULL);
+	if (!W_ERROR_IS_OK(werr)) {
+		return false;
 	}
 
-	return False;
+	return true;
 }
 
 /***********************************************************************
@@ -176,7 +257,7 @@ int regsubkey_ctr_numkeys( struct regsubkey_ctr *ctr )
  Retreive a specific key string
  **********************************************************************/
 
-char* regsubkey_ctr_specific_key( struct regsubkey_ctr *ctr, uint32 key_index )
+char* regsubkey_ctr_specific_key( struct regsubkey_ctr *ctr, uint32_t key_index )
 {
 	if ( ! (key_index < ctr->num_subkeys) )
 		return NULL;