From a70966f31ac5d0c100d670232a93a639c4727507 Mon Sep 17 00:00:00 2001
From: Thierry Bordaz <tbordaz@redhat.com>
Date: Mon, 14 Nov 2016 16:48:11 +0100
Subject: [PATCH 1/3] Ticket 49031: Improve memberof with a cache of group
 parents

---
 ldap/servers/plugins/memberof/memberof.c | 605 ++++++++++++++++++++++++++++++-
 1 file changed, 593 insertions(+), 12 deletions(-)

diff --git a/ldap/servers/plugins/memberof/memberof.c b/ldap/servers/plugins/memberof/memberof.c
index aad300f..3fc2d30 100644
--- a/ldap/servers/plugins/memberof/memberof.c
+++ b/ldap/servers/plugins/memberof/memberof.c
@@ -34,10 +34,13 @@
 #  include <config.h>
 #endif
 
+#include <ctype.h>
 #include "slapi-plugin.h"
 #include "string.h"
 #include "nspr.h"
+#include "plhash.h"
 #include "memberof.h"
+#include <time.h>
 
 static Slapi_PluginDesc pdesc = { "memberof", VENDOR,
 	DS_PACKAGE_VERSION, "memberof plugin" };
@@ -47,6 +50,8 @@ static Slapi_DN* _ConfigAreaDN = NULL;
 static Slapi_RWLock *config_rwlock = NULL;
 static Slapi_DN* _pluginDN = NULL;
 static PRMonitor *memberof_operation_lock = 0;
+#define MEMBEROF_HASHTABLE_SIZE 1000
+static PLHashTable *group_ancestors_hashtable = NULL; /* global hash table protected by memberof_lock (memberof_operation_lock) */
 MemberOfConfig *qsortConfig = 0;
 static int usetxn = 0;
 static int premodfn = 0;
@@ -65,6 +70,33 @@ typedef struct _memberof_get_groups_data
         Slapi_ValueSet **group_norm_vals;
 } memberof_get_groups_data;
 
+/* The key to access the hash table is the normalized DN
+ * The normalized DN is stored in the value because:
+ *  - It is used in slapi_valueset_find
+ *  - It is used to fill the memberof_get_groups_data.group_norm_vals
+ */
+typedef struct _memberof_cached_value
+{
+	char *key;
+	char *group_dn_val;
+        char *group_ndn_val;
+	int valid;
+} memberof_cached_value;
+struct cache_stat
+{
+	int total_lookup;
+	int successfull_lookup;
+	int total_add;
+	int total_remove;
+	int total_enumerate;
+	int cumul_duration_lookup;
+	int cumul_duration_add;
+	int cumul_duration_remove;
+	int cumul_duration_enumerate;
+};
+static struct cache_stat cache_stat;
+#define MEMBEROF_CACHE_DEBUG	0
+
 /*** function prototypes ***/
 
 /* exported functions */
@@ -117,7 +149,7 @@ static int memberof_qsort_compare(const void *a, const void *b);
 static void memberof_load_array(Slapi_Value **array, Slapi_Attr *attr);
 static int memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *sdn);
 static int memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn, MemberOfConfig *config,
-	char **types, plugin_search_entry_callback callback,  void *callback_data);
+	char **types, plugin_search_entry_callback callback,  void *callback_data, int *cached);
 static int memberof_is_direct_member(MemberOfConfig *config, Slapi_Value *groupdn,
 	Slapi_Value *memberdn);
 static int memberof_is_grouping_attr(char *type, MemberOfConfig *config);
@@ -147,6 +179,12 @@ static int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data);
 static int memberof_entry_in_scope(MemberOfConfig *config, Slapi_DN *sdn);
 static int memberof_add_objectclass(char *auto_add_oc, const char *dn);
 static int memberof_add_memberof_attr(LDAPMod **mods, const char *dn, char *add_oc);
+static PLHashTable *hashtable_new();
+static void ancestor_hashtable_empty(char *msg);
+static void ancestor_hashtable_entry_free(memberof_cached_value *entry);
+static memberof_cached_value *ancestors_cache_lookup(const char *ndn);
+static PRBool ancestors_cache_remove(const char *ndn);
+static PLHashEntry *ancestors_cache_add(const void *key, void *value);
 
 /*** implementation ***/
 
@@ -337,6 +375,7 @@ int memberof_postop_start(Slapi_PBlock *pb)
 			goto bail;
 		}
 	}
+	group_ancestors_hashtable = hashtable_new();
 
 	/* Set the alternate config area if one is defined. */
 	slapi_pblock_get(pb, SLAPI_PLUGIN_CONFIG_AREA, &config_area);
@@ -432,6 +471,10 @@ int memberof_postop_close(Slapi_PBlock *pb)
 	PR_DestroyMonitor(memberof_operation_lock);
 	memberof_operation_lock = NULL;
 
+	if (group_ancestors_hashtable) {
+		ancestor_hashtable_empty("memberof_postop_close empty group_ancestors_hashtable");
+		PL_HashTableDestroy(group_ancestors_hashtable);
+	}
 	slapi_log_error( SLAPI_LOG_TRACE, MEMBEROF_PLUGIN_SUBSYSTEM,
 		     "<-- memberof_postop_close\n" );
 	return 0;
@@ -583,6 +626,7 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *
 	int i = 0;
 	char *groupattrs[2] = {0, 0};
 	int rc = LDAP_SUCCESS;
+	int cached = 0;
 
 	/* Loop through each grouping attribute to find groups that have
 	 * dn as a member.  For any matches, delete the dn value from the
@@ -593,9 +637,9 @@ memberof_del_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config, Slapi_DN *
 		                             config->groupattrs[i]};
 
 		groupattrs[0] = config->groupattrs[i];
-
+		slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_del_dn_from_groups: Ancestors of %s\n", slapi_sdn_get_dn(sdn));
 		rc = memberof_call_foreach_dn(pb, sdn, config, groupattrs,
-		                              memberof_del_dn_type_callback, &data);
+		                              memberof_del_dn_type_callback, &data, &cached);
 	}
 
 	return rc;
@@ -658,6 +702,55 @@ memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn)
 	}
 	return NULL;
 }
+
+
+
+static void
+add_ancestors_cbdata(memberof_cached_value *ancestors, void *callback_data)
+{
+	Slapi_Value *sval;
+	int val_index;
+	Slapi_ValueSet *group_norm_vals_cbdata = *((memberof_get_groups_data*) callback_data)->group_norm_vals;
+	Slapi_ValueSet *group_vals_cbdata = *((memberof_get_groups_data*) callback_data)->groupvals;
+	Slapi_Value *memberdn_val = ((memberof_get_groups_data*) callback_data)->memberdn_val;
+	int added_group;
+	PRBool empty_ancestor = PR_FALSE;
+
+	for (val_index = 0, added_group = 0; ancestors[val_index].valid; val_index++) {
+		/* For each of its ancestor (not already present) add it to callback_data */
+		
+		if (ancestors[val_index].group_ndn_val == NULL) {
+			/* This is a node with no ancestor
+			 * ancestors should only contains this empty valid value
+			 * but just in case let the loop continue instead of a break
+			 */
+			empty_ancestor = PR_TRUE;
+			continue;
+		}
+		
+		sval = slapi_value_new_string(ancestors[val_index].group_ndn_val);
+		if (sval) {
+			if (!slapi_valueset_find(
+				((memberof_get_groups_data*) callback_data)->config->group_slapiattrs[0], group_norm_vals_cbdata, sval)) {
+				/* This ancestor was not already present in the callback data
+				 * => Add it to the callback_data
+				 * Using slapi_valueset_add_value it consumes sval
+				 * so do not free sval
+				 */
+				slapi_valueset_add_value_ext(group_norm_vals_cbdata, sval, SLAPI_VALUE_FLAG_PASSIN);
+				sval = slapi_value_new_string(ancestors[val_index].group_dn_val);
+				slapi_valueset_add_value_ext(group_vals_cbdata, sval, SLAPI_VALUE_FLAG_PASSIN);
+				added_group++;
+			} else {
+				/* This ancestor was already present, free sval that will not be consumed */
+				slapi_value_free(&sval);
+			}
+		}
+	}
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "add_ancestors_cbdata: Ancestors of %s contained %d groups. %d added. %s\n", 
+		slapi_value_get_string(memberdn_val), val_index, added_group, empty_ancestor ? "no ancestors" : "");
+}
+	
 /*
  * Does a callback search of "type=dn" under the db suffix that "dn" is in,
  * unless all_backends is set, then we look at all the backends.  If "dn"
@@ -666,7 +759,7 @@ memberof_scope_is_child_of_dn(MemberOfConfig *config, Slapi_DN *sdn)
  */
 int
 memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,
-	MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void *callback_data)
+	MemberOfConfig *config, char **types, plugin_search_entry_callback callback, void *callback_data, int *cached)
 {
 	Slapi_PBlock *search_pb = NULL;
 	Slapi_DN *base_sdn = NULL;
@@ -681,11 +774,33 @@ memberof_call_foreach_dn(Slapi_PBlock *pb, Slapi_DN *sdn,
 	int free_it = 0;
 	int rc = 0;
 	int i = 0;
+	memberof_cached_value *ht_grp = NULL;
+	const char *ndn = slapi_sdn_get_ndn(sdn);
+	
+	*cached = 0;
 
 	if (!memberof_entry_in_scope(config, sdn)) {
 		return (rc);
 	}
-
+	
+	/* Here we will retrieve the ancestor of sdn.
+	 * The key access is the normalized sdn
+	 * This is done through recursive internal searches of parents
+	 * If the ancestors of sdn are already cached, just use
+	 * this value
+	 */
+	ht_grp = ancestors_cache_lookup((const void *) ndn);
+	if (ht_grp) {
+#if MEMBEROF_CACHE_DEBUG
+		slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s already cached (%x)\n", ndn, ht_grp);
+#endif
+		add_ancestors_cbdata(ht_grp, callback_data);
+		*cached = 1;
+		return (rc);
+	}
+#if MEMBEROF_CACHE_DEBUG
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_call_foreach_dn: Ancestors of %s not cached\n", ndn);
+#endif
 	/* Count the number of types. */
 	for (num_types = 0; types && types[num_types]; num_types++)
 	{
@@ -952,6 +1067,7 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,
 	int i = 0;
 	char *groupattrs[2] = {0, 0};
 	int ret = LDAP_SUCCESS;
+	int cached = 0;
 
 	/* Loop through each grouping attribute to find groups that have
 	 * pre_dn as a member.  For any matches, replace pre_dn with post_dn
@@ -965,10 +1081,10 @@ memberof_replace_dn_from_groups(Slapi_PBlock *pb, MemberOfConfig *config,
 		};
 
 		groupattrs[0] = config->groupattrs[i];
-
+		slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_replace_dn_from_groups: Ancestors of %s\n", slapi_sdn_get_dn(post_sdn));
 		if((ret = memberof_call_foreach_dn(pb, pre_sdn, config, groupattrs,
 		                                   memberof_replace_dn_type_callback,
-		                                   &data)))
+		                                   &data, &cached)))
 		{
 			break;
 		}
@@ -2029,26 +2145,230 @@ memberof_get_groups(MemberOfConfig *config, Slapi_DN *member_sdn)
 	Slapi_ValueSet *group_norm_vals = slapi_valueset_new();
 	Slapi_Value *memberdn_val = 
 	                      slapi_value_new_string(slapi_sdn_get_ndn(member_sdn));
+#if MEMBEROF_CACHE_DEBUG
+	memberof_cached_value *ht_grp;
+#endif
 	slapi_value_set_flags(memberdn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
 
 	memberof_get_groups_data data = {config, memberdn_val, &groupvals, &group_norm_vals};
 
 	memberof_get_groups_r(config, member_sdn, &data);
-
+#if MEMBEROF_CACHE_DEBUG
+	ht_grp = ancestors_cache_lookup((const void *) slapi_sdn_get_ndn(member_sdn));
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups: Is it a group %lx\n", ht_grp);
+#endif
 	slapi_value_free(&memberdn_val);
 	slapi_valueset_free(group_norm_vals);
 
 	return groupvals;
 }
 
+void
+dump_cache_entry(memberof_cached_value *double_check, char *msg)
+{
+	int i;
+	for(i = 0; double_check[i].valid; i++) {
+		slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "dump_cache_entry: %s -> %s (%lx)\n", 
+			msg ? msg : "<no key>", 
+			double_check[i].group_dn_val ? double_check[i].group_dn_val : "NULL", double_check);
+	}
+}
+/* 
+ * A cache value consist of an array of all dn/ndn of the groups member_ndn_val belongs to
+ * The last element of the array has 'valid=0'
+ * the firsts elements of the array has 'valid=1' and the dn/ndn of group it belong to
+ */
+static void
+cache_ancestors(Slapi_Value *member_ndn_val, memberof_get_groups_data *groups)
+{
+	Slapi_ValueSet *groupvals = *((memberof_get_groups_data*)groups)->groupvals;
+	Slapi_Value *sval;
+	Slapi_DN *sdn = 0;
+	const char *dn;
+	const char *ndn;
+	char *key;
+	char *key_copy;
+	int hint = 0;
+	int count;
+	int index;
+	memberof_cached_value *cache_entry;
+	memberof_cached_value *double_check;
+	
+	if (!member_ndn_val) {
+		slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "Fail to cache groups ancestor of unknown member\n");
+		return;
+	}
+	
+	/* Allocate the cache entry and fill it */
+	count = slapi_valueset_count(groupvals);
+	if (count == 0) {
+		/* There is no group containing member_ndn_val
+		 * so cache the NULL value
+		 */
+		cache_entry = (memberof_cached_value *) slapi_ch_calloc(2, sizeof(memberof_cached_value));
+		if (!cache_entry) {
+			slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "Fail to cache no group are ancestor of %s\n", slapi_value_get_string(member_ndn_val));
+			return;
+		}
+		index = 0;
+		cache_entry[index].key = NULL; /* only the last element (valid=0) contains the key */
+		cache_entry[index].group_dn_val  = NULL;
+		cache_entry[index].group_ndn_val = NULL;
+		cache_entry[index].valid = 1; /* this entry is valid and indicate no group contain member_ndn_val */
+#if MEMBEROF_CACHE_DEBUG
+		slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: For %s cache %s\n",
+				slapi_value_get_string(member_ndn_val), cache_entry[index].group_dn_val ? cache_entry[index].group_dn_val : "<empty>");
+#endif
+		index++;
+		
+	} else {
+		cache_entry = (memberof_cached_value *) slapi_ch_calloc(count + 1, sizeof (memberof_cached_value));
+		if (!cache_entry) {
+			slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "Fail to cache groups ancestor of %s\n", slapi_value_get_string(member_ndn_val));
+			return;
+		}
+
+		/* Store the dn/ndn into the cache_entry */
+		index = 0;
+		hint = slapi_valueset_first_value(groupvals, &sval);
+		while (sval) {
+			/* add this dn/ndn even if it is NULL
+			 * in fact a node belonging to no group needs to be cached
+			 */
+			dn = slapi_value_get_string(sval);
+			sdn = slapi_sdn_new_dn_byval(dn);
+			ndn = slapi_sdn_get_ndn(sdn);
+
+			cache_entry[index].key = NULL; /* only the last element (valid=0) contains the key */
+			cache_entry[index].group_dn_val = slapi_ch_strdup(dn);
+			cache_entry[index].group_ndn_val = slapi_ch_strdup(ndn);
+			cache_entry[index].valid = 1;
+#if MEMBEROF_CACHE_DEBUG
+			slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "cache_ancestors: For %s cache %s\n",
+				slapi_value_get_string(member_ndn_val), cache_entry[index].group_dn_val ? cache_entry[index].group_dn_val : "<empty>");
+#endif
+			index++;
+			slapi_sdn_free(&sdn);
+
+			hint = slapi_valueset_next_value(groupvals, hint, &sval);
+		}
+	}
+	/* This is marking the end of the cache_entry */
+	key = slapi_value_get_string(member_ndn_val);
+	key_copy = slapi_ch_strdup(key);
+	cache_entry[index].key = key_copy;
+	cache_entry[index].group_dn_val  = NULL;
+	cache_entry[index].group_ndn_val = NULL;
+	cache_entry[index].valid = 0;
+	
+	/* Cache the ancestor of member_ndn_val using the
+	 * normalized DN key
+	 */
+#if MEMBEROF_CACHE_DEBUG
+	dump_cache_entry(cache_entry, key);
+#endif
+	if (ancestors_cache_add((const void*) key_copy, (void *) cache_entry) == NULL) {
+		slapi_log_error( SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "Failed to cache ancestor of %s\n", key);
+		ancestor_hashtable_entry_free(cache_entry);
+		slapi_ch_free ((void**)&cache_entry);
+		return;
+	}
+#if MEMBEROF_CACHE_DEBUG
+	if (double_check = ancestors_cache_lookup((const void*) key)) {
+		dump_cache_entry(double_check, "read back");
+	}
+#endif
+}
+/*
+ * Add in v2 the values that are in v1
+ * If the values are already present in v2, they are skipped
+ * It does not consum the values in v1
+ */
+static void
+merge_ancestors(memberof_get_groups_data *v1, memberof_get_groups_data *v2)
+{
+	Slapi_Value *sval_dn = 0;
+	Slapi_Value *sval_ndn = 0;
+	Slapi_Value *sval;
+	Slapi_DN *val_sdn = 0;
+	int hint = 0;
+	Slapi_ValueSet *v1_groupvals = *((memberof_get_groups_data*)v1)->groupvals;
+	Slapi_ValueSet *v2_groupvals = *((memberof_get_groups_data*)v2)->groupvals;
+	Slapi_ValueSet *v2_group_norm_vals = *((memberof_get_groups_data*)v2)->group_norm_vals;
+	int merged_cnt = 0;
+	
+	hint = slapi_valueset_first_value(v1_groupvals, &sval);
+	while (sval) {
+		sval_dn = slapi_value_new_string(slapi_value_get_string(sval));
+		if (sval_dn) {
+			/* Use the normalized dn from v1 to search it
+			 * in v2
+			 */
+			val_sdn = slapi_sdn_new_dn_byval(slapi_value_get_string(sval_dn));
+			sval_ndn = slapi_value_new_string(slapi_sdn_get_ndn(val_sdn));
+			if (!slapi_valueset_find(
+				((memberof_get_groups_data*) v2)->config->group_slapiattrs[0], v2_group_norm_vals, sval_ndn)) {
+				/* This ancestor was not already present in v2 => Add it
+				 * Using slapi_valueset_add_value it consumes val
+				 * so do not free sval
+				 */
+#if MEMBEROF_CACHE_DEBUG
+				slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "merge_ancestors: add %s\n", slapi_value_get_string(sval_ndn));
+#endif
+				slapi_valueset_add_value_ext(v2_groupvals, sval_dn, SLAPI_VALUE_FLAG_PASSIN);
+				slapi_valueset_add_value_ext(v2_group_norm_vals, sval_ndn, SLAPI_VALUE_FLAG_PASSIN);
+				merged_cnt++;
+			} else {
+				/* This ancestor was already present, free sval_ndn/sval_dn that will not be consumed */
+				slapi_value_free(&sval_dn);
+				slapi_value_free(&sval_ndn);
+			}
+			slapi_sdn_free(&val_sdn);
+		}
+		hint = slapi_valueset_next_value(v1_groupvals, hint, &sval);
+	}
+}
 int
 memberof_get_groups_r(MemberOfConfig *config, Slapi_DN *member_sdn,
                       memberof_get_groups_data *data)
 {
+	Slapi_ValueSet *groupvals = slapi_valueset_new();
+	Slapi_ValueSet *group_norm_vals = slapi_valueset_new();
+	Slapi_Value *member_ndn_val = 
+	                      slapi_value_new_string(slapi_sdn_get_ndn(member_sdn));
+	const char *ndn;
+	int rc;
+	int cached = 0;
+#if MEMBEROF_CACHE_DEBUG
+	memberof_cached_value *ht_grp;
+#endif
+	slapi_value_set_flags(member_ndn_val, SLAPI_ATTR_FLAG_NORMALIZED_CIS);
+	
+	memberof_get_groups_data member_data = {config, member_ndn_val, &groupvals, &group_norm_vals};
+	
 	/* Search for any grouping attributes that point to memberdn.
 	 * For each match, add it to the list, recurse and do same search */
-	return memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs,
-		memberof_get_groups_callback, data);
+#if MEMBEROF_CACHE_DEBUG
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups_r: Ancestors of %s\n", slapi_sdn_get_dn(member_sdn));
+#endif
+	rc = memberof_call_foreach_dn(NULL, member_sdn, config, config->groupattrs,
+		memberof_get_groups_callback, &member_data, &cached);
+	
+	merge_ancestors(&member_data, data);
+	if (!cached)
+		cache_ancestors(member_ndn_val, &member_data);
+	ndn = slapi_sdn_get_ndn(member_sdn);
+#if MEMBEROF_CACHE_DEBUG
+	ht_grp = ancestors_cache_lookup(ndn);
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups_r: Retrieve %s\n", ndn ? ndn : "NULL");
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_get_groups_r: Is it a group %lx\n", ht_grp);
+#endif
+
+	slapi_value_free(&member_ndn_val);
+	slapi_valueset_free(groupvals);
+	slapi_valueset_free(group_norm_vals);
+
+	return rc;
 }
 
 /* memberof_get_groups_callback()
@@ -2127,7 +2447,7 @@ int memberof_get_groups_callback(Slapi_Entry *e, void *callback_data)
 			slapi_valueset_add_value_ext(group_norm_vals, group_ndn_val, SLAPI_VALUE_FLAG_PASSIN);
 	}
 	if(!config->skip_nested || config->fixup_task){
-		/* now recurse to find parent groups of e */
+		/* now recurse to find ancestors groups of e */
 		memberof_get_groups_r(((memberof_get_groups_data*)callback_data)->config,
 			group_sdn, callback_data);
 	}
@@ -2220,9 +2540,10 @@ memberof_test_membership(Slapi_PBlock *pb, MemberOfConfig *config,
                          Slapi_DN *group_sdn)
 {
 	char *attrs[2] = {config->memberof_attr, 0};
+	int cached = 0;
 
 	return memberof_call_foreach_dn(pb, group_sdn, config, attrs,
-		memberof_test_membership_callback, config);
+		memberof_test_membership_callback, config, &cached);
 }
 
 /*
@@ -2616,10 +2937,23 @@ void memberof_lock()
 	if (usetxn) {
 		PR_EnterMonitor(memberof_operation_lock);
 	}
+	if (group_ancestors_hashtable) {
+		ancestor_hashtable_empty("memberof_lock empty group_ancestors_hashtable");
+		memset(&cache_stat, 0, sizeof(cache_stat));
+	}
 }
 
 void memberof_unlock()
 {
+	if (group_ancestors_hashtable) {
+		ancestor_hashtable_empty("memberof_unlock empty group_ancestors_hashtable");
+		slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics: total lookup %d (success %d), add %d, remove %d, enum %d\n",
+			cache_stat.total_lookup, cache_stat.successfull_lookup, 
+			cache_stat.total_add, cache_stat.total_remove, cache_stat.total_enumerate);
+		slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "cache statistics duration: lookup %ld, add %ld, remove %ld, enum %ld\n",
+			cache_stat.cumul_duration_lookup, cache_stat.cumul_duration_add, 
+			cache_stat.cumul_duration_remove, cache_stat.cumul_duration_enumerate);
+	}
 	if (usetxn) {
 		PR_ExitMonitor(memberof_operation_lock);
 	}
@@ -2849,6 +3183,101 @@ int memberof_fix_memberof(MemberOfConfig *config, char *dn, char *filter_str)
 
 	return rc;
 }
+static memberof_cached_value *
+ancestors_cache_lookup(const char *ndn)
+{
+	memberof_cached_value *e;
+#ifdef HAVE_CLOCK_GETTIME
+	long int start;
+	struct timespec tsnow;
+#endif
+	
+	cache_stat.total_lookup++;
+	
+#ifdef HAVE_CLOCK_GETTIME
+        if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
+		start = 0;
+        } else {
+		start = tsnow.tv_nsec;
+	}
+#endif
+
+	e = (memberof_cached_value *) PL_HashTableLookupConst(group_ancestors_hashtable, (const void *) ndn);
+
+#ifdef HAVE_CLOCK_GETTIME
+	if (start) {
+		if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
+			cache_stat.cumul_duration_lookup += (tsnow.tv_nsec - start);
+		}
+	}
+#endif
+	
+	if (e)
+		cache_stat.successfull_lookup++;
+	return e;
+	
+}
+static PRBool
+ancestors_cache_remove(const char *ndn)
+{
+	PRBool rc;
+#ifdef HAVE_CLOCK_GETTIME
+	long int start;
+	struct timespec tsnow;
+#endif	
+
+	cache_stat.total_remove++;
+
+#ifdef HAVE_CLOCK_GETTIME
+        if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
+		start = 0;
+        } else {
+		start = tsnow.tv_nsec;
+	}
+#endif
+
+	rc = PL_HashTableRemove(group_ancestors_hashtable, (const void *) ndn);
+	
+#ifdef HAVE_CLOCK_GETTIME
+	if (start) {
+		if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
+			cache_stat.cumul_duration_remove += (tsnow.tv_nsec - start);
+		}
+	}
+#endif
+	return rc;
+}
+
+static PLHashEntry *
+ancestors_cache_add(const void *key, void *value)
+{
+	PLHashEntry *e;
+#ifdef HAVE_CLOCK_GETTIME
+	long int start;
+	struct timespec tsnow;
+#endif
+	cache_stat.total_add++;
+	
+#ifdef HAVE_CLOCK_GETTIME
+        if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
+		start = 0;
+        } else {
+		start = tsnow.tv_nsec;
+	}
+#endif
+	
+	e = PL_HashTableAdd(group_ancestors_hashtable, key, value);
+
+#ifdef HAVE_CLOCK_GETTIME
+	if (start) {
+		if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
+			cache_stat.cumul_duration_add += (tsnow.tv_nsec - start);
+		}
+	}
+#endif
+	return e;
+}
+
 
 /* memberof_fix_memberof_callback()
  * Add initial and/or fix up broken group list in entry
@@ -2875,6 +3304,37 @@ int memberof_fix_memberof_callback(Slapi_Entry *e, void *callback_data)
 	/* get a list of all of the groups this user belongs to */
 	groups = memberof_get_groups(config, sdn);
 
+	if(config->group_filter) {
+		if (slapi_filter_test_simple(e, config->group_filter)) {
+			const char *ndn;
+			memberof_cached_value *ht_grp;
+			
+			/* This entry is not a group
+			 * if (likely) we cached its ancestor it is useless 
+			 * so free this memory
+			 */
+			ndn = slapi_sdn_get_ndn(sdn);
+#if MEMBEROF_CACHE_DEBUG
+			slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: This is NOT a group %s\n", ndn);
+#endif
+			ht_grp = ancestors_cache_lookup((const void *) ndn);
+			if (ht_grp) {
+				if (ancestors_cache_remove((const void *) ndn)) {
+					slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: free cached values for %s\n", ndn);
+					ancestor_hashtable_entry_free(ht_grp);
+					slapi_ch_free((void **) &ht_grp);
+				} else {
+					slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Fail to remove that leaf node %s\n", ndn);
+				}
+			} else {
+				/* This is quite unexpected, after a call to memberof_get_groups
+				 * ndn ancestors should be in the cache
+				 */
+				slapi_log_error(SLAPI_LOG_FATAL, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_fix_memberof_callback: Weird, %s is not in the cache\n", ndn);
+			}
+		}
+		
+	}
 	/* If we found some groups, replace the existing memberOf attribute
 	 * with the found values.  */
 	if (groups && slapi_valueset_count(groups))
@@ -3000,3 +3460,124 @@ memberof_add_objectclass(char *auto_add_oc, const char *dn)
 
 	return rc;
 }
+
+static PRIntn memberof_hash_compare_keys(const void *v1, const void *v2)
+{
+	PRIntn rc;
+	rc = (0 == strcasecmp(v1, v2)) ? 1 : 0;
+#if MEMBEROF_CACHE_DEBUG
+	slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_hash_compare_values: %s -> %s (%d)\n", v1 ? v1 : "NULL", v2 ? v2 : "NULL", rc);
+#endif
+	return rc;
+}
+
+static PRIntn memberof_hash_compare_values(const void *v1, const void *v2)
+{
+	PRIntn rc;
+	rc = (((char *) v1 == (char *) v2) ? 1 : 0);
+	return rc;
+}
+
+/*
+ *  Hashing function using Bernstein's method
+ */
+static PLHashNumber memberof_hash_fn(const void *key)
+{
+    PLHashNumber hash = 5381;
+    unsigned char *x = (unsigned char *)key;
+    int c;
+
+    while ((c = *x++)){
+        hash = ((hash << 5) + hash) ^ c;
+    }
+#if MEMBEROF_CACHE_DEBUG
+    slapi_log_error(SLAPI_LOG_PLUGIN, MEMBEROF_PLUGIN_SUBSYSTEM, "memberof_hash_fn: %s -> %lx\n", (char *) key, hash);
+#endif
+    return hash;
+}
+
+/* allocates the plugin hashtable
+ * This hash table is used by operation and is protected from
+ * concurrent operations with the memberof_lock (if not usetxn, memberof_lock
+ * is not implemented and the hash table will be not used.
+ *
+ * The hash table contains all the DN of the entries for which the memberof
+ * attribute has been computed/updated during the current operation
+ *
+ * hash table should be empty at the beginning and end of the plugin callback
+ */
+PRIntn
+hashNocaseCompare(const void *v1, const void *v2)
+{
+	return (strcasecmp((char *)v1, (char *)v2) == 0);
+}
+static PLHashTable *hashtable_new()
+{
+	if (!usetxn)
+		return NULL;
+
+	return PL_NewHashTable(MEMBEROF_HASHTABLE_SIZE,
+		memberof_hash_fn,
+		memberof_hash_compare_keys,
+		memberof_hash_compare_values, NULL, NULL);
+}
+
+static
+void ancestor_hashtable_entry_free(memberof_cached_value *entry)
+{
+	int i;
+	for (i = 0; entry[i].valid; i++) {
+		slapi_ch_free(&entry[i].group_dn_val);
+		slapi_ch_free(&entry[i].group_ndn_val);
+	}
+	/* Here we are at the ending element containing the key */
+	slapi_ch_free(&entry[i].key);
+}
+/* this function called for each hash node during hash destruction */
+static PRIntn ancestor_hashtable_remove(PLHashEntry *he, PRIntn index, void *arg)
+{
+	memberof_cached_value *group_ancestor_array;
+	int i;
+	char *msg; /* used for debug purpose */
+
+	if (he == NULL)
+		return HT_ENUMERATE_NEXT;
+
+
+	group_ancestor_array = (memberof_cached_value *) he->value;
+	msg = (char *) arg;
+	ancestor_hashtable_entry_free(group_ancestor_array);
+	slapi_ch_free((void **)&group_ancestor_array);
+
+	return HT_ENUMERATE_REMOVE;
+}
+
+static void ancestor_hashtable_empty(char *msg)
+{
+#ifdef HAVE_CLOCK_GETTIME
+	long int start;
+	struct timespec tsnow;
+#endif
+
+	if (group_ancestors_hashtable) {
+		cache_stat.total_enumerate++;
+#ifdef HAVE_CLOCK_GETTIME
+		if (clock_gettime(CLOCK_REALTIME, &tsnow) != 0) {
+			start = 0;
+		} else {
+			start = tsnow.tv_nsec;
+		}
+#endif
+		PL_HashTableEnumerateEntries(group_ancestors_hashtable, ancestor_hashtable_remove, msg);
+		
+#ifdef HAVE_CLOCK_GETTIME
+		if (start) {
+			if (clock_gettime(CLOCK_REALTIME, &tsnow) == 0) {
+				cache_stat.cumul_duration_enumerate += (tsnow.tv_nsec - start);
+			}
+		}
+#endif
+	}
+	
+}
+
-- 
2.5.0

