ldb: Avoid multiple tiny allocations during full DB scan

LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC allows us to consolidate some of these allocations

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>

Autobuild-User(master): Douglas Bagnall <dbagnall@samba.org>
Autobuild-Date(master): Wed Aug 31 10:53:09 CEST 2016 on sn-devel-144

5 files changed, 59 insertions, 7 deletions

diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 7e6dd2d55d3..a63dd5840ef 100644
--- a/lib/ldb/common/ldb_pack.c
+++ b/lib/ldb/common/ldb_pack.c
@@ -233,6 +233,7 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 	unsigned int nelem = 0;
 	size_t len;
 	unsigned int found = 0;
+	struct ldb_val *ldb_val_single_array = NULL;
 
 	if (list == NULL) {
 		list_size = 0;
@@ -313,6 +314,26 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 		goto failed;
 	}
 
+	/*
+	 * In typical use, most values are single-valued.  This makes
+	 * it quite expensive to allocate an array of ldb_val for each
+	 * of these, just to then hold the pointer to the data buffer
+	 * (in the LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC we don't
+	 * allocate the data).  So with
+	 * LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC we allocate this ahead
+	 * of time and use it for the single values where possible.
+	 * (This is used the the normal search case, but not in the
+	 * index case because of caller requirements).
+	 */
+	if (flags & LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC) {
+		ldb_val_single_array = talloc_array(message->elements, struct ldb_val,
+						    message->num_elements);
+		if (ldb_val_single_array == NULL) {
+			errno = ENOMEM;
+			goto failed;
+		}
+	}
+
 	for (i=0;i<message->num_elements;i++) {
 		const char *attr = NULL;
 		size_t attr_len;
@@ -396,7 +417,9 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 		p += attr_len + 1;
 		element->num_values = pull_uint32(p, 0);
 		element->values = NULL;
-		if (element->num_values != 0) {
+		if ((flags & LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC) && element->num_values == 1) {
+			element->values = &ldb_val_single_array[nelem];
+		} else if (element->num_values != 0) {
 			element->values = talloc_array(message->elements,
 						       struct ldb_val,
 						       element->num_values);
diff --git a/lib/ldb/include/ldb_module.h b/lib/ldb/include/ldb_module.h
index 1c48590a381..833d5a8f3d0 100644
--- a/lib/ldb/include/ldb_module.h
+++ b/lib/ldb/include/ldb_module.h
@@ -412,8 +412,15 @@ int ldb_unpack_data(struct ldb_context *ldb,
  * Giving a NULL list (or a list_size of 0) unpacks all the attributes.
  *
  * Flags allow control of allocation, so that if
- * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC is specified, then values are
- * not allocate, instead they point into the supplier constant buffer.
+ * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC is specified, then data in values are
+ * not allocated, instead they point into the supplier constant buffer.
+ *
+ * If LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC is specified, then values
+ * array are not allocated individually (for single-valued
+ * attributes), instead they point into a single buffer per message.
+ *
+ * LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC is only valid when
+ * LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC is also specified.
  *
  * Likewise if LDB_UNPACK_DATA_FLAG_NO_DN is specified, the DN is omitted.
  */
@@ -425,7 +432,8 @@ int ldb_unpack_data_only_attr_list_flags(struct ldb_context *ldb,
 					 unsigned int flags,
 					 unsigned int *nb_elements_in_db);
 
-#define LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC 0x0001
-#define LDB_UNPACK_DATA_FLAG_NO_DN         0x0002
+#define LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC   0x0001
+#define LDB_UNPACK_DATA_FLAG_NO_DN           0x0002
+#define LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC 0x0004
 
 #endif
diff --git a/lib/ldb/ldb_tdb/ldb_index.c b/lib/ldb/ldb_tdb/ldb_index.c
index 6c238562970..79241721c9f 100644
--- a/lib/ldb/ldb_tdb/ldb_index.c
+++ b/lib/ldb/ldb_tdb/ldb_index.c
@@ -952,7 +952,9 @@ static int ltdb_index_filter(const struct dn_list *dn_list,
 			return LDB_ERR_OPERATIONS_ERROR;
 		}
 
-		ret = ltdb_search_dn1(ac->module, dn, msg, LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC);
+		ret = ltdb_search_dn1(ac->module, dn, msg,
+				      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
+				      LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC);
 		talloc_free(dn);
 		if (ret == LDB_ERR_NO_SUCH_OBJECT) {
 			/* the record has disappeared? yes, this can happen */
diff --git a/lib/ldb/ldb_tdb/ldb_search.c b/lib/ldb/ldb_tdb/ldb_search.c
index 8dc93d68ace..373855fd428 100644
--- a/lib/ldb/ldb_tdb/ldb_search.c
+++ b/lib/ldb/ldb_tdb/ldb_search.c
@@ -559,7 +559,8 @@ static int search_func(struct tdb_context *tdb, TDB_DATA key, TDB_DATA data, voi
 	ret = ldb_unpack_data_only_attr_list_flags(ldb, &val,
 						   msg,
 						   NULL, 0,
-						   LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC,
+						   LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
+						   LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC,
 						   &nb_elements_in_db);
 	if (ret == -1) {
 		talloc_free(msg);
diff --git a/source4/torture/ldb/ldb.c b/source4/torture/ldb/ldb.c
index f7f04db0242..dcbe919d5bf 100644
--- a/source4/torture/ldb/ldb.c
+++ b/source4/torture/ldb/ldb.c
@@ -1138,6 +1138,24 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture)
 				 ldb_unpack_data_only_attr_list_flags(ldb, &data,
 								      msg,
 								      NULL, 0,
+								      LDB_UNPACK_DATA_FLAG_NO_DATA_ALLOC|
+								      LDB_UNPACK_DATA_FLAG_NO_VALUES_ALLOC,
+								      &nb_elements_in_db),
+				 0,
+				 "ldb_unpack_data failed");
+
+	ldif.changetype = LDB_CHANGETYPE_NONE;
+	ldif.msg = msg;
+	ldif_text = ldb_ldif_write_string(ldb, mem_ctx, &ldif);
+
+	torture_assert_int_equal(torture,
+				 strcmp(ldif_text, dda1d01d_ldif), 0,
+				 "ldif form differs from binary form");
+
+	torture_assert_int_equal(torture,
+				 ldb_unpack_data_only_attr_list_flags(ldb, &data,
+								      msg,
+								      NULL, 0,
 								      LDB_UNPACK_DATA_FLAG_NO_DN,
 								      &nb_elements_in_db),
 				 0,