Browse Source

Ticket #490 - Slow role performance when using a lot of roles

Bug description: Role uses the virtual attribute framework.
When the search with a filter including nsrole or a return
attribute list containing nsrole is being processed, the
virtual attribute code checks the entry if the vattr values
are valid or not by examining the watermark.  If it is valid,
the values are used as if they are static.  If it is not
valid, the entry is evaluated against the role definitions
and dynamically generated virtual attributes are set to the
list (e_virtual_attrs) with the proper watermark.

The current code additionally checks e_virtual_attrs to determine
the entry is already evaluated or not.  If it is NULL, it
considers the entry is not yet evaluated and it returns SLAPI_
ENTRY_VATTR_NOT_RESOLVED even if the watermark is valid.  That
is, all the entries which do not have virtual attributes are
unnecessarily evaluated every time search with nsrole is executed.

Fix description: This patch does not return SLAPI_ENTRY_VATTR_NOT_
RESOLVED but does SLAPI_ENTRY_VATTR_RESOLVED_ABSENT if e_virtual_
attrs is NULL AND the watermark is valid.  By skipping the not-
needed nsrole evaluation, it speeds up the virtual search once
virutual attribute values are placed in the entries in memory.

https://fedorahosted.org/389/ticket/490

Reviewed by Rich (Thank you!!)
Noriko Hosoi 12 years ago
parent
commit
ae7e811d00
1 changed files with 33 additions and 34 deletions
  1. 33 34
      ldap/servers/slapd/entry.c

+ 33 - 34
ldap/servers/slapd/entry.c

@@ -2429,9 +2429,9 @@ void slapi_entrycache_vattrcache_watermark_invalidate()
 
 int
 slapi_entry_vattrcache_findAndTest( const Slapi_Entry *e, const char *type,
-								Slapi_Filter *f,
-								filter_type_t filter_type,
-								int *rc )
+                                    Slapi_Filter *f,
+                                    filter_type_t filter_type,
+                                    int *rc )
 {
 	Slapi_Attr *tmp_attr = NULL;
 
@@ -2439,44 +2439,43 @@ slapi_entry_vattrcache_findAndTest( const Slapi_Entry *e, const char *type,
 	*rc = -1;
 
 	if( slapi_vattrcache_iscacheable(type) &&
-		slapi_entry_vattrcache_watermark_isvalid(e) && e->e_virtual_attrs)
-	{
+	    slapi_entry_vattrcache_watermark_isvalid(e) ) {
 
 		if(e->e_virtual_lock == NULL) {
-            return r;
+			return r;
 		}
 
 		vattrcache_entry_READ_LOCK(e);
-		tmp_attr = attrlist_find( e->e_virtual_attrs, type );
-		if (tmp_attr != NULL)
-		{
-			if(valueset_isempty(&(tmp_attr->a_present_values)))
-			{
-				/*
-				 * this is a vattr that has been
-				 * cached already but does not exist
-				 */
-				r= SLAPI_ENTRY_VATTR_RESOLVED_ABSENT; /* hard coded for prototype */				
-			}
-			else
-			{
-				/*
-				 * this is a cached vattr--test the filter on it.
-				 * 
-				*/
-				r= SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
-				if ( filter_type == FILTER_TYPE_AVA ) {
-					*rc = plugin_call_syntax_filter_ava( tmp_attr,
-														f->f_choice,
-														&f->f_ava );
-				} else if ( filter_type == FILTER_TYPE_SUBSTRING) {
-					*rc = plugin_call_syntax_filter_sub( NULL, tmp_attr,
-															&f->f_sub);
-				} else if ( filter_type == FILTER_TYPE_PRES ) {
-					/* type is there, that's all we need to know. */
-					*rc = 0;
+		if (e->e_virtual_attrs) {
+			tmp_attr = attrlist_find( e->e_virtual_attrs, type );
+			if (tmp_attr != NULL) {
+				if(valueset_isempty(&(tmp_attr->a_present_values))) {
+					/*
+					 * this is a vattr that has been
+					 * cached already but does not exist
+					 */
+					/* hard coded for prototype */
+					r= SLAPI_ENTRY_VATTR_RESOLVED_ABSENT; 
+				} else {
+					/*
+					 * this is a cached vattr--test the filter on it.
+					 */
+					r= SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
+					if ( filter_type == FILTER_TYPE_AVA ) {
+						*rc = plugin_call_syntax_filter_ava(tmp_attr,
+						                                    f->f_choice,
+						                                    &f->f_ava);
+					} else if ( filter_type == FILTER_TYPE_SUBSTRING) {
+						*rc = plugin_call_syntax_filter_sub(NULL, tmp_attr,
+						                                    &f->f_sub);
+					} else if ( filter_type == FILTER_TYPE_PRES ) {
+						/* type is there, that's all we need to know. */
+						*rc = 0;
+					}
 				}
 			}
+		} else {
+			r= SLAPI_ENTRY_VATTR_RESOLVED_ABSENT; 
 		}
 		vattrcache_entry_READ_UNLOCK(e);
 	}