ldap/servers/plugins/cos/cos_cache.c | 2
ldap/servers/slapd/entry.c | 238 ++++++++++++++++-------------------
ldap/servers/slapd/slapi-plugin.h | 1
ldap/servers/slapd/slapi-private.h | 2
ldap/servers/slapd/vattr.c | 14 +-
5 files changed, 123 insertions(+), 134 deletions(-)
New commits:
commit 86f8b9ff6cc7aa25f5424aa4661f20f9aeea9aad
Author: Thierry bordaz (tbordaz) <tbordaz(a)redhat.com>
Date: Tue Apr 9 16:33:37 2013 +0200
Ticket 512 - improve performance of vattr code
Bug Description:
Description of virtual attribute evaluation for a search:
1 - create a list of candidate
2 - For each candidate entry the filter is evaluated
(slapi_vattr_filter_test).
For a given filter component, if the attribute is a virtual attribute (it
exists a Service Provider for it)
the computed valueset is cached into the entry "e_virtual_attrs"
(slapi_entry_vattrcache_merge_sv).
The condition to cache the value is that the attribute should be
cachable.
3 - The entries matching the filter are returned
attributes values are retrieved from "e_virtual_attrs"
(slapi_vattr_namespace_values_get_sp),
if they have been cached
In DS 389 implementation, only 'nsrole' attribute is cachable. So service
provide like 'COS'
are not cached. If a filter contains multiple times the same vattr, the vattr is
evaluated (cos evaluated)
several time. The same way if the set of returned attribute contains a vattr it is
evaluated even if
it was already evaluated during filter analyze.
Fix Description:
When a Service Provider evaluates a vattr, the fix consists to provide to the SP a
mechanism
to say if the returned valueset is or not cachable.
If the valueset is cachable, it is stored into the entry (e->e_virtual_attrs).
If the vattr is cached and appears several times in the filter or is a returned
attribute, then
its valueset is looked up from e_virtual_attrs.
In order to keep the same plugin interface, I added a new value to the buffer_flag
parameter.
This new value (SLAPI_VIRTUALATTRS_VALUES_CACHEABLE) is not related to buffer but to
the fact
that the returned valueset is cachable.
In term of performance, the search throughput is equivalent or better. If the search
does not
use vattr or use vattr that are evaluate only once, I still measure a gain 5-20%.
In the best condition (vattr in the filter/returned and the same vattr appears
several times (10))
the throughput is > 2times more (1800/s vs 700/s)
ticket:
https://fedorahosted.org/389/ticket/512
Reviewed by: Rich Megginson and Noriko Hosoi (thanks Rich, thanks Noriko !)
Platforms tested: Fedora 17
Flag Day: no
Doc impact: no
diff --git a/ldap/servers/plugins/cos/cos_cache.c b/ldap/servers/plugins/cos/cos_cache.c
index fed2aa9..4a3a23a 100644
--- a/ldap/servers/plugins/cos/cos_cache.c
+++ b/ldap/servers/plugins/cos/cos_cache.c
@@ -2096,7 +2096,7 @@ static int cos_cache_vattr_get(vattr_sp_handle *handle,
vattr_context *c, Slapi_
ret = cos_cache_query_attr(pCache, c, e, type, results, NULL, NULL, NULL);
if(ret == 0)
{
- *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES;
+ *free_flags = SLAPI_VIRTUALATTRS_RETURNED_COPIES |
SLAPI_VIRTUALATTRS_VALUES_CACHEABLE;
*actual_type_name = slapi_ch_strdup(type);
*type_name_disposition = SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_EXACTLY_OR_ALIAS;
}
diff --git a/ldap/servers/slapd/entry.c b/ldap/servers/slapd/entry.c
index d7dbf61..01446ff 100644
--- a/ldap/servers/slapd/entry.c
+++ b/ldap/servers/slapd/entry.c
@@ -2443,49 +2443,47 @@ slapi_entry_vattrcache_findAndTest( const Slapi_Entry *e, const
char *type,
int r= SLAPI_ENTRY_VATTR_NOT_RESOLVED; /* assume not resolved yet */
*rc = -1;
- if( slapi_vattrcache_iscacheable(type) &&
- slapi_entry_vattrcache_watermark_isvalid(e) ) {
-
- if(e->e_virtual_lock == NULL) {
- return r;
- }
+ if ((e->e_virtual_attrs == NULL) || !
slapi_entry_vattrcache_watermark_isvalid(e)) {
+ /* there is not virtual attribute cached or they are all invalid
+ * just return
+ */
+ return r;
+ }
- vattrcache_entry_READ_LOCK(e);
- 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);
- }
+ /* Check if the attribute is already cached */
+ vattrcache_entry_READ_LOCK(e);
+ 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;
+ }
+ }
+ } /* tmp_attr != NULL */
+ }
+ vattrcache_entry_READ_UNLOCK(e);
- return r;
+ return r;
}
/*
@@ -2512,49 +2510,46 @@ slapi_entry_vattrcache_find_values_and_type_ex( const Slapi_Entry
*e,
{
Slapi_Attr *tmp_attr = NULL;
- int r= SLAPI_ENTRY_VATTR_NOT_RESOLVED; /* assume not resolved yet */
+ int r = SLAPI_ENTRY_VATTR_NOT_RESOLVED; /* assume not resolved yet */
- if( slapi_vattrcache_iscacheable(type) &&
- slapi_entry_vattrcache_watermark_isvalid(e) && e->e_virtual_attrs)
- {
+ if ((e->e_virtual_attrs == NULL) || !
slapi_entry_vattrcache_watermark_isvalid(e)) {
+ /* there is not virtual attribute cached or they are all invalid
+ * just return
+ */
+ return r;
+ }
- if(e->e_virtual_lock == NULL) {
- return r;
- }
+ /* check if the attribute is not already cached */
+ vattrcache_entry_READ_LOCK(e);
+ 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
+ */
+ r = SLAPI_ENTRY_VATTR_RESOLVED_ABSENT; /* hard coded for
prototype */
+ } else {
+ /*
+ * this is a cached vattr
+ * return a duped copy of the values and type
+ */
+ char *vattr_type = NULL;
+
+ r = SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
+ *results = (Slapi_ValueSet**) slapi_ch_calloc(1, sizeof
(**results));
+ **results =
valueset_dup(&(tmp_attr->a_present_values));
+
+ *actual_type_name =
+ (char**) slapi_ch_malloc(sizeof
(**actual_type_name));
+ slapi_attr_get_type(tmp_attr, &vattr_type);
+ **actual_type_name = slapi_ch_strdup(vattr_type);
- 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
- * return a duped copy of the values and type
- */
- char *vattr_type=NULL;
-
- r= SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
- *results = (Slapi_ValueSet**)slapi_ch_calloc(1, sizeof(**results));
- **results = valueset_dup(&(tmp_attr->a_present_values));
-
- *actual_type_name =
- (char**)slapi_ch_malloc(sizeof(**actual_type_name));
- slapi_attr_get_type( tmp_attr, &vattr_type );
- **actual_type_name = slapi_ch_strdup(vattr_type);
-
- }
- }
- vattrcache_entry_READ_UNLOCK(e);
- }
+ }
+ } /* tmp_attr != NULL */
+ }
+ vattrcache_entry_READ_UNLOCK(e);
return r;
}
@@ -2573,45 +2568,42 @@ slapi_entry_vattrcache_find_values_and_type( const Slapi_Entry
*e,
int r= SLAPI_ENTRY_VATTR_NOT_RESOLVED; /* assume not resolved yet */
- if( slapi_vattrcache_iscacheable(type) &&
- slapi_entry_vattrcache_watermark_isvalid(e) && e->e_virtual_attrs)
- {
-
- if(e->e_virtual_lock == NULL) {
- 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
- * return a duped copy of the values and type
- */
- char *vattr_type=NULL;
-
- r= SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
- *results = valueset_dup(&(tmp_attr->a_present_values));
-
- slapi_attr_get_type( tmp_attr, &vattr_type );
- *actual_type_name = slapi_ch_strdup(vattr_type);
-
- }
- }
- vattrcache_entry_READ_UNLOCK(e);
- }
+ if ((e->e_virtual_attrs == NULL) || !
slapi_entry_vattrcache_watermark_isvalid(e)) {
+ /* there is not virtual attribute cached or they are all invalid
+ * just return
+ */
+ return r;
+ }
+
+ /* Check if the attribute is already cached */
+ vattrcache_entry_READ_LOCK(e);
+ 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
+ */
+ r = SLAPI_ENTRY_VATTR_RESOLVED_ABSENT; /* hard coded for
prototype */
+ } else {
+ /*
+ * this is a cached vattr
+ * return a duped copy of the values and type
+ */
+ char *vattr_type = NULL;
+
+ r = SLAPI_ENTRY_VATTR_RESOLVED_EXISTS;
+ *results =
valueset_dup(&(tmp_attr->a_present_values));
+
+ slapi_attr_get_type(tmp_attr, &vattr_type);
+ *actual_type_name = slapi_ch_strdup(vattr_type);
+ }
+ }
+ }
+ vattrcache_entry_READ_UNLOCK(e);
+
return r;
}
@@ -2642,16 +2634,12 @@ slapi_entry_attr_merge_sv(Slapi_Entry *e, const char *type,
Slapi_Value **vals )
int
slapi_entry_vattrcache_merge_sv(Slapi_Entry *e, const char *type,
- Slapi_ValueSet *valset)
+ Slapi_ValueSet *valset, int buffer_flags)
{
Slapi_Value **vals = NULL;
/* only attempt to merge if it's a cacheable attribute */
- if ( slapi_vattrcache_iscacheable(type) ) {
-
- if(e->e_virtual_lock == NULL) {
- return 0;
- }
+ if ( slapi_vattrcache_iscacheable(type) || (buffer_flags &
SLAPI_VIRTUALATTRS_VALUES_CACHEABLE)) {
vattrcache_entry_WRITE_LOCK(e);
diff --git a/ldap/servers/slapd/slapi-plugin.h b/ldap/servers/slapd/slapi-plugin.h
index 25ccb00..05b9742 100644
--- a/ldap/servers/slapd/slapi-plugin.h
+++ b/ldap/servers/slapd/slapi-plugin.h
@@ -6120,6 +6120,7 @@ int slapi_unregister_backend_state_change(void * handle);
#define SLAPI_VIRTUALATTRS_RETURNED_POINTERS 1
#define SLAPI_VIRTUALATTRS_RETURNED_COPIES 2
#define SLAPI_VIRTUALATTRS_REALATTRS_ONLY 4
+#define SLAPI_VIRTUALATTRS_VALUES_CACHEABLE 8
/* Attribute type name disposition values (type_name_disposition parameter) */
#define SLAPI_VIRTUALATTRS_TYPE_NAME_MATCHED_EXACTLY_OR_ALIAS 1
diff --git a/ldap/servers/slapd/slapi-private.h b/ldap/servers/slapd/slapi-private.h
index 375c33f..a77c9f4 100644
--- a/ldap/servers/slapd/slapi-private.h
+++ b/ldap/servers/slapd/slapi-private.h
@@ -495,7 +495,7 @@ typedef enum{
#define SLAPI_ENTRY_VATTR_RESOLVED_ABSENT -2
#define SLAPI_ENTRY_VATTR_RESOLVED_EXISTS 0
-int slapi_entry_vattrcache_merge_sv(Slapi_Entry *e, const char *type, Slapi_ValueSet
*vals);
+int slapi_entry_vattrcache_merge_sv(Slapi_Entry *e, const char *type, Slapi_ValueSet
*vals, int buffer_flags);
int slapi_entry_vattrcache_find_values_and_type_ex( const Slapi_Entry *e,
const char *type,
Slapi_ValueSet ***results,
diff --git a/ldap/servers/slapd/vattr.c b/ldap/servers/slapd/vattr.c
index 3360852..b04fca7 100644
--- a/ldap/servers/slapd/vattr.c
+++ b/ldap/servers/slapd/vattr.c
@@ -508,7 +508,7 @@ int vattr_test_filter( Slapi_PBlock *pb,
* entry itself.
* but first lets cache the no result
*/
- slapi_entry_vattrcache_merge_sv(e, type, NULL );
+ slapi_entry_vattrcache_merge_sv(e, type, NULL, buffer_flags);
}
else
{
@@ -548,7 +548,7 @@ int vattr_test_filter( Slapi_PBlock *pb,
* Cache stuff: dups results
*/
slapi_entry_vattrcache_merge_sv(e, actual_type_name[i],
- results[i] );
+ results[i], buffer_flags);
/*
* Free stuff, just in case we did not
* get pointers.
@@ -577,7 +577,7 @@ int vattr_test_filter( Slapi_PBlock *pb,
while(results[i])
{
slapi_entry_vattrcache_merge_sv(e, actual_type_name[i],
- results[i] );
+ results[i], buffer_flags);
/*
* Free stuff, just in case we did not
* get pointers.
@@ -730,7 +730,7 @@ slapi_vattr_values_get_sp(vattr_context *c,
* But first lets cache the no result
* Creates the type (if necessary).
*/
- slapi_entry_vattrcache_merge_sv(e, type, NULL );
+ slapi_entry_vattrcache_merge_sv(e, type, NULL, *buffer_flags);
}
else
@@ -741,7 +741,7 @@ slapi_vattr_values_get_sp(vattr_context *c,
* results.
*/
slapi_entry_vattrcache_merge_sv(e, *actual_type_name,
- *results );
+ *results, *buffer_flags);
}
break;
@@ -953,7 +953,7 @@ int slapi_vattr_namespace_values_get_sp(vattr_context *c,
* But first lets cache the no result
* dups the type (if necessary).
*/
- slapi_entry_vattrcache_merge_sv(e, type, NULL );
+ slapi_entry_vattrcache_merge_sv(e, type, NULL, *buffer_flags);
}
else
@@ -967,7 +967,7 @@ int slapi_vattr_namespace_values_get_sp(vattr_context *c,
* when we do real batched attributes
*/
slapi_entry_vattrcache_merge_sv(e, **actual_type_name,
- **results );
+ **results, *buffer_flags);
}
}
}