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

diff --git a/lib/ldb/common/ldb_pack.c b/lib/ldb/common/ldb_pack.c
index 7e6dd2d55d330bd275bd704fce243a79c2d7c0ce..a63dd5840ef840f70d91cb69ef38ae9b6baea229 100644 (file)
--- 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 1c48590a3812c2ff93aebffc350c4671310e572b..833d5a8f3d07d380ff030e0943b8884f070aa468 100644 (file)
--- 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 6c238562970a6cbdf3ecb01a8e5e7dac694795ca..79241721c9f1562ce925fcd7eed61332b94778de 100644 (file)
--- 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 8dc93d68acea4489dbd6d7b41cf640e646cd4a28..373855fd428bcddf930bcfd41084c10cd77e7f71 100644 (file)
--- 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 f7f04db0242f24c1ae85ae8e3cc3aac651a2d77d..dcbe919d5bfe8da157632269a9ecf525115432d2 100644 (file)
--- a/source4/torture/ldb/ldb.c
+++ b/source4/torture/ldb/ldb.c
@@ -1130,6 +1130,24 @@ static bool torture_ldb_unpack_flags(struct torture_context *torture)
        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_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");