[PATCH] Improve LDB modify performance by reducing allocation

[Andrew Bartlett]

This patch makes joining a large domain about 10% faster, by avoiding
individual allocation of the attributes that are about the be re-
packed.  Instead, pointers into the whole record are used. 

Please review/push!

Andrew Bartlett
-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba



-------------- next part --------------
From 2a1309f567711e68841b59bbf2f0250e0350ad8f Mon Sep 17 00:00:00 2001
From: Andrew Bartlett <abartlet at samba.org>
Date: Wed, 16 Aug 2017 12:51:09 +1200
Subject: [PATCH] ldb_tdb: Rework ltdb_modify_internal() to use
 ltdb_search_dn1() internally

This avoids duplicate code and allows us to use the allocation-avoiding
LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC flag.

We can not use LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC as el2->values
is talloc_realloc()ed in the routine.

This patch makes joining a large domain (with many group members)
about 10% faster, by avoiding individual allocation of the attributes
that are about the be re-packed or replaced.

Instead, pointers into the whole record are used.

Signed-off-by: Andrew Bartlett <abartlet at samba.org>
---
 lib/ldb/ldb_tdb/ldb_tdb.c | 40 +++++++++++-----------------------------
 1 file changed, 11 insertions(+), 29 deletions(-)

diff --git a/lib/ldb/ldb_tdb/ldb_tdb.c b/lib/ldb/ldb_tdb/ldb_tdb.c
index 2ac1967ee15..bc8780ad7ee 100644
--- a/lib/ldb/ldb_tdb/ldb_tdb.c
+++ b/lib/ldb/ldb_tdb/ldb_tdb.c
@@ -686,52 +686,34 @@ int ltdb_modify_internal(struct ldb_module *module,
 			 struct ldb_request *req)
 {
 	struct ldb_context *ldb = ldb_module_get_ctx(module);
-	void *data = ldb_module_get_private(module);
-	struct ltdb_private *ltdb = talloc_get_type(data, struct ltdb_private);
-	TDB_DATA tdb_key, tdb_data;
-	struct ldb_val ldb_data;
 	struct ldb_message *msg2;
 	unsigned int i, j;
 	int ret = LDB_SUCCESS, idx;
 	struct ldb_control *control_permissive = NULL;
+	TALLOC_CTX *mem_ctx = talloc_new(req);
 
+	if (mem_ctx == NULL) {
+		return ldb_module_oom(module);
+	}
+	
 	if (req) {
 		control_permissive = ldb_request_get_control(req,
 					LDB_CONTROL_PERMISSIVE_MODIFY_OID);
 	}
 
-	tdb_key = ltdb_key(module, msg->dn);
-	if (!tdb_key.dptr) {
-		return LDB_ERR_OTHER;
-	}
-
-	tdb_data = tdb_fetch(ltdb->tdb, tdb_key);
-	if (!tdb_data.dptr) {
-		talloc_free(tdb_key.dptr);
-		return ltdb_err_map(tdb_error(ltdb->tdb));
-	}
-
-	msg2 = ldb_msg_new(tdb_key.dptr);
+	msg2 = ldb_msg_new(mem_ctx);
 	if (msg2 == NULL) {
-		free(tdb_data.dptr);
 		ret = LDB_ERR_OTHER;
 		goto done;
 	}
 
-	ldb_data.data = tdb_data.dptr;
-	ldb_data.length = tdb_data.dsize;
-
-	ret = ldb_unpack_data(ldb_module_get_ctx(module), &ldb_data, msg2);
-	free(tdb_data.dptr);
-	if (ret == -1) {
-		ret = LDB_ERR_OTHER;
+	ret = ltdb_search_dn1(module, msg->dn,
+			      msg2,
+			      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC);
+	if (ret != LDB_SUCCESS) {
 		goto done;
 	}
 
-	if (!msg2->dn) {
-		msg2->dn = msg->dn;
-	}
-
 	for (i=0; i<msg->num_elements; i++) {
 		struct ldb_message_element *el = &msg->elements[i], *el2;
 		struct ldb_val *vals;
@@ -1018,7 +1000,7 @@ int ltdb_modify_internal(struct ldb_module *module,
 	}
 
 done:
-	talloc_free(tdb_key.dptr);
+	TALLOC_FREE(mem_ctx);
 	return ret;
 }
 
-- 
2.11.0