[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: FW: Modify performance improvement for back-ldbm and back-bdb (ITS#1526)



Okay,

I've taken Howard's first alternative and implemented that in the attached patch. The patch still has the same effect on the modify performance.

Please review and include in the source-base if review is satisfactory.

P.S. Source-patch against release 2.0.19 is available if anyone is interested.

Best regards,

	Gertjan.

Howard Chu wrote:

I think this is a good idea as well, but I have a couple alternate ideas.
First, we could introduce a "flags" field in the Attribute structure, with
#define	SLAPD_ATTR_IXADD	0x01
#define	SLAPD_ATTR_IXDEL	0x02
and set these flags on the attrs as we find them in the modify. Seems pretty
clean and simple.

As another possibility, we can simply break the attr lists onto separate lists
as the modify proceeds. save_attrs would be broken out onto save_attr_del for
deleted indexed attrs, and likewise e->e_attrs would be broken out onto
e_attr_add for added indexed attrs. The separated lists can be easily rejoined
at the end
of the operation, but their order would not be preserved. Of course we claim no
guarantees on attribute order as it is, but the current code tends to preserve
it.

  -- Howard Chu
  Chief Architect, Symas Corp.       Director, Highland Sun
  http://www.symas.com               http://highlandsun.com/hyc
  Symas: Premier OpenSource Development and Support

-----Original Message-----
From: owner-openldap-bugs@OpenLDAP.org
[mailto:owner-openldap-bugs@OpenLDAP.org] On Behalf Of
masarati@aero.polimi.it


Full_Name: Gertjan van Wingerde


I have improved the modify performance of OpenLDAP by only reindexing
attributes
that are actually changed in the modify operation instead of all the

attributes

of
an entry. This change improves the performance of a modify operation on a
non-indexed
attribute from about 4 modifications per second to about 38 modifications per
second.
If an indexed attribute is being modified, the results are a bit less

dramatic,

but
still an improvement of about 200% is observed then.
Note: The changes are only applicable to the LDBM backend and the BDB

backend.

As far
as I can see no other backends can use this optimisation.


Your idea sounds definitely good; I'm not very comfortable with the fixed size array of index add/remove attrs. This may require a little reworking. I understand the improvement can be significant, though.

Pierangelo






-- MvG,

		Gertjan

----------

Gertjan van Wingerde
Geessinkweg 177
7544 TX Enschede
The Netherlands
E-mail: gwingerde@home.nl
Index: servers/slapd/attr.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/attr.c,v
retrieving revision 1.68
diff -u -r1.68 attr.c
--- servers/slapd/attr.c	2002/01/04 20:17:45	1.68
+++ servers/slapd/attr.c	2002/01/13 17:26:59
@@ -74,6 +74,7 @@
 
 	tmp->a_desc = a->a_desc;
 	tmp->a_next = NULL;
+	tmp->a_flags = 0;
 
 	return tmp;
 }
@@ -125,6 +126,7 @@
 		(*a)->a_desc = desc;
 		(*a)->a_vals = NULL;
 		(*a)->a_next = NULL;
+		(*a)->a_flags = 0;
 	}
 
 	return( value_add( &(*a)->a_vals, vals ) );
Index: servers/slapd/operational.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/operational.c,v
retrieving revision 1.4
diff -u -r1.4 operational.c
--- servers/slapd/operational.c	2002/01/13 04:00:59	1.4
+++ servers/slapd/operational.c	2002/01/13 17:27:00
@@ -27,6 +27,7 @@
 	a->a_vals[1].bv_val = NULL;
 
 	a->a_next = NULL;
+	a->a_flags = 0;
 
 	return a;
 }
@@ -47,6 +48,7 @@
 	a->a_vals[1].bv_val = NULL;
 
 	a->a_next = NULL;
+	a->a_flags = 0;
 
 	return a;
 }
Index: servers/slapd/slap.h
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/slap.h,v
retrieving revision 1.300
diff -u -r1.300 slap.h
--- servers/slapd/slap.h	2002/01/13 04:00:59	1.300
+++ servers/slapd/slap.h	2002/01/13 17:27:01
@@ -709,6 +709,9 @@
 	AttributeDescription *a_desc;
 	BVarray	a_vals;
 	struct slap_attr	*a_next;
+	unsigned a_flags;
+#define SLAP_ATTR_IXADD		0x1U
+#define SLAP_ATTR_IXDEL		0x2U
 } Attribute;
 
 
Index: servers/slapd/back-bdb/index.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb/index.c,v
retrieving revision 1.16
diff -u -r1.16 index.c
--- servers/slapd/back-bdb/index.c	2002/01/04 20:17:49	1.16
+++ servers/slapd/back-bdb/index.c	2002/01/13 17:27:01
@@ -75,6 +75,24 @@
 	return 0;
 }
 
+int bdb_index_is_indexed(
+	Backend *be,
+	AttributeDescription *desc )
+{
+	int rc;
+	slap_mask_t mask;
+	char *dbname;
+	struct berval prefix;
+
+	mask = index_mask( be, desc, &dbname, &prefix );
+
+	if( mask == 0 ) {
+		return LDAP_INAPPROPRIATE_MATCHING;
+	}
+
+    	return LDAP_SUCCESS;
+}
+
 int bdb_index_param(
 	Backend *be,
 	AttributeDescription *desc,
Index: servers/slapd/back-bdb/modify.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb/modify.c,v
retrieving revision 1.31
diff -u -r1.31 modify.c
--- servers/slapd/back-bdb/modify.c	2002/01/10 18:37:03	1.31
+++ servers/slapd/back-bdb/modify.c	2002/01/13 17:27:01
@@ -33,6 +33,7 @@
 	Modification	*mod;
 	Modifications	*ml;
 	Attribute	*save_attrs;
+	Attribute 	*ap;
 
 	Debug( LDAP_DEBUG_TRACE, "bdb_modify_internal: 0x%08lx: %s\n",
 		e->e_id, e->e_dn, 0);
@@ -115,6 +116,16 @@
 			/* unlock entry, delete from cache */
 			return err; 
 		}
+
+		/* check if modified attribute was indexed */
+		err = bdb_index_is_indexed( be, mod->sm_desc );
+		if ( err == LDAP_SUCCESS ) {
+			ap = attr_find( save_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXDEL;
+
+			ap = attr_find( e->e_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXADD;
+		}
 	}
 
 	/* check that the entry still obeys the schema */
@@ -127,24 +138,40 @@
 		return rc;
 	}
 
-	/* delete indices for old attributes */
-	rc = bdb_index_entry_del( be, tid, e, save_attrs);
-	if ( rc != LDAP_SUCCESS ) {
-		attrs_free( e->e_attrs );
-		e->e_attrs = save_attrs;
-		Debug( LDAP_DEBUG_ANY, "entry index delete failed!\n",
-			0, 0, 0 );
-		return rc;
+	/* update the indices of the modified attributes */
+
+	/* start with deleting the old index entries */
+	for ( ap = save_attrs; ap != NULL; ap = ap->a_next ) {
+		if ( ap->a_flags & SLAP_ATTR_IXDEL ) {
+			rc = bdb_index_values( be, tid, ap->a_desc, ap->a_vals,
+					       e->e_id, SLAP_INDEX_DELETE_OP );
+			if ( rc != LDAP_SUCCESS ) {
+				attrs_free( e->e_attrs );
+				e->e_attrs = save_attrs;
+				Debug( LDAP_DEBUG_ANY,
+				       "Attribute index delete failure",
+				       0, 0, 0 );
+				return rc;
+			}
+			ap->a_flags &= ~SLAP_ATTR_IXDEL;
+		}
 	}
 
-	/* add indices for new attributes */
-	rc = bdb_index_entry_add( be, tid, e, e->e_attrs); 
-	if ( rc != LDAP_SUCCESS ) {
-		attrs_free( e->e_attrs );
-		e->e_attrs = save_attrs;
-		Debug( LDAP_DEBUG_ANY, "entry index add failed!\n",
-			0, 0, 0 );
-		return rc;
+	/* add the new index entries */
+	for ( ap = e->e_attrs; ap != NULL; ap = ap->a_next ) {
+		if (ap->a_flags & SLAP_ATTR_IXADD) {
+			rc = bdb_index_values( be, tid, ap->a_desc, ap->a_vals,
+					       e->e_id, SLAP_INDEX_ADD_OP );
+			if ( rc != LDAP_SUCCESS ) {
+				attrs_free( e->e_attrs );
+				e->e_attrs = save_attrs;
+				Debug( LDAP_DEBUG_ANY,
+				       "Attribute index add failure",
+				       0, 0, 0 );
+				return rc;
+			}
+			ap->a_flags &= ~SLAP_ATTR_IXADD;
+		}
 	}
 
 	return rc;
Index: servers/slapd/back-bdb/proto-bdb.h
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-bdb/proto-bdb.h,v
retrieving revision 1.40
diff -u -r1.40 proto-bdb.h
--- servers/slapd/back-bdb/proto-bdb.h	2002/01/04 20:17:49	1.40
+++ servers/slapd/back-bdb/proto-bdb.h	2002/01/13 17:27:01
@@ -207,6 +207,11 @@
  * index.c
  */
 extern int
+bdb_index_is_indexed LDAP_P((
+	Backend *be,
+	AttributeDescription *desc ));
+
+extern int
 bdb_index_param LDAP_P((
 	Backend *be,
 	AttributeDescription *desc,
Index: servers/slapd/back-ldap/search.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-ldap/search.c,v
retrieving revision 1.52
diff -u -r1.52 search.c
--- servers/slapd/back-ldap/search.c	2002/01/12 16:35:01	1.52
+++ servers/slapd/back-ldap/search.c	2002/01/13 17:27:02
@@ -426,6 +426,7 @@
 		attr = (Attribute *)ch_malloc( sizeof(Attribute) );
 		if (attr == NULL)
 			continue;
+		attr->a_flags = 0;
 		attr->a_next = 0;
 		attr->a_desc = NULL;
 		if (slap_bv2ad(&mapped, &attr->a_desc, &text) != LDAP_SUCCESS) {
Index: servers/slapd/back-ldbm/index.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-ldbm/index.c,v
retrieving revision 1.67
diff -u -r1.67 index.c
--- servers/slapd/back-ldbm/index.c	2002/01/04 20:17:52	1.67
+++ servers/slapd/back-ldbm/index.c	2002/01/13 17:27:02
@@ -71,6 +71,24 @@
 	return 0;
 }
 
+int index_is_indexed(
+	Backend *be,
+	AttributeDescription *desc )
+{
+	int rc;
+	slap_mask_t mask;
+	char *dbname;
+	struct berval prefix;
+
+	mask = index_mask( be, desc, &dbname, &prefix );
+
+	if( mask == 0 ) {
+		return LDAP_INAPPROPRIATE_MATCHING;
+	}
+
+    	return LDAP_SUCCESS;
+}
+
 int index_param(
 	Backend *be,
 	AttributeDescription *desc,
Index: servers/slapd/back-ldbm/modify.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-ldbm/modify.c,v
retrieving revision 1.81
diff -u -r1.81 modify.c
--- servers/slapd/back-ldbm/modify.c	2002/01/10 18:37:03	1.81
+++ servers/slapd/back-ldbm/modify.c	2002/01/13 17:27:02
@@ -26,7 +26,6 @@
  * and there and of course the likelihood of bugs increases.
  * Juan C. Gomez (gomez@engr.sgi.com) 05/18/99
  */ 
-
 int ldbm_modify_internal(
     Backend	*be,
     Connection	*conn,
@@ -43,6 +42,7 @@
 	Modification	*mod;
 	Modifications	*ml;
 	Attribute	*save_attrs;
+	Attribute 	*ap;
 
 #ifdef NEW_LOGGING
 	LDAP_LOG(( "backend", LDAP_LEVEL_ENTRY,
@@ -189,6 +189,16 @@
 			/* unlock entry, delete from cache */
 			return err; 
 		}
+
+		/* check if modified attribute was indexed */
+		err = index_is_indexed( be, mod->sm_desc );
+		if ( err == LDAP_SUCCESS ) {
+			ap = attr_find( save_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXDEL;
+
+			ap = attr_find( e->e_attrs, mod->sm_desc );
+			if ( ap ) ap->a_flags |= SLAP_ATTR_IXADD;
+		}
 	}
 
 	/* check for abandon */
@@ -228,17 +238,56 @@
 	}
 	ldap_pvt_thread_mutex_unlock( &op->o_abandonmutex );
 
-	/* delete indices for old attributes */
-	index_entry_del( be, e, save_attrs);
+	/* update the indices of the modified attributes */
+
+	/* start with deleting the old index entries */
+	for ( ap = save_attrs; ap != NULL; ap = ap->a_next ) {
+		if ( ap->a_flags & SLAP_ATTR_IXDEL ) {
+			rc = index_values( be, ap->a_desc, ap->a_vals, e->e_id,
+				  	   SLAP_INDEX_DELETE_OP );
+			if ( rc != LDAP_SUCCESS ) {
+				attrs_free( e->e_attrs );
+				e->e_attrs = save_attrs;
+#ifdef NEW_LOGGING
+				LDAP_LOG(( "backend", LDAP_LEVEL_ERR,
+				   	   "ldbm_modify_internal: Attribute index delete failure\n" ));
+#else
+				Debug( LDAP_DEBUG_ANY,
+				       "Attribute index delete failure",
+			               0, 0, 0 );
+#endif
+				return rc;
+			}
+			ap->a_flags &= ~SLAP_ATTR_IXDEL;
+		}
+	}
 
-	/* add indices for new attributes */
-	index_entry_add( be, e, e->e_attrs);
+	/* add the new index entries */
+	for ( ap = e->e_attrs; ap != NULL; ap = ap->a_next ) {
+		if ( ap->a_flags & SLAP_ATTR_IXADD ) {
+			rc = index_values( be, ap->a_desc, ap->a_vals, e->e_id,
+				  	   SLAP_INDEX_ADD_OP );
+			if ( rc != LDAP_SUCCESS ) {
+				attrs_free( e->e_attrs );
+				e->e_attrs = save_attrs;
+#ifdef NEW_LOGGING
+				LDAP_LOG(( "backend", LDAP_LEVEL_ERR,
+				   	   "ldbm_modify_internal: Attribute index add failure\n" ));
+#else
+				Debug( LDAP_DEBUG_ANY,
+				       "Attribute index add failure",
+			               0, 0, 0 );
+#endif
+				return rc;
+			}
+			ap->a_flags &= ~SLAP_ATTR_IXADD;
+		}
+	}
 
 	attrs_free( save_attrs );
 
 	return LDAP_SUCCESS;
 }
-
 
 int
 ldbm_back_modify(
Index: servers/slapd/back-ldbm/proto-back-ldbm.h
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-ldbm/proto-back-ldbm.h,v
retrieving revision 1.67
diff -u -r1.67 proto-back-ldbm.h
--- servers/slapd/back-ldbm/proto-back-ldbm.h	2002/01/04 20:17:53	1.67
+++ servers/slapd/back-ldbm/proto-back-ldbm.h	2002/01/13 17:27:02
@@ -139,6 +139,11 @@
  * index.c
  */
 extern int
+index_is_indexed LDAP_P((
+	Backend *be,
+	AttributeDescription *desc ));
+
+extern int
 index_param LDAP_P((
 	Backend *be,
 	AttributeDescription *desc,
Index: servers/slapd/back-meta/search.c
===================================================================
RCS file: /repo/OpenLDAP/pkg/ldap/servers/slapd/back-meta/search.c,v
retrieving revision 1.25
diff -u -r1.25 search.c
--- servers/slapd/back-meta/search.c	2002/01/06 05:11:02	1.25
+++ servers/slapd/back-meta/search.c	2002/01/13 17:27:03
@@ -646,6 +646,7 @@
 		if ( attr == NULL ) {
 			continue;
 		}
+		attr->a_flags = 0;
 		attr->a_next = 0;
 		attr->a_desc = NULL;
 		if ( slap_bv2ad( &mapped, &attr->a_desc, &text )