Hi,
Currently ra_serf caches *all* DAV properties retrieved using PROPFIND
in session pool. This was attempt to reduce number of PROPFIND
requests. But current implementation has several problems:
1. Unlimited memory usage: current implementation stores all
properties, so svn ls -R command can easily eat 150 mb of memory.
2. Very low cache hit due the fact that cache doesn't support caching
"allprop" requests. For some operations there is no hits at all.
3. Current implementation caches properties that may change between
requests, like URL to youngest revision.
Of course these problems can be fixed, but I'm not sure that we need
this code. Since in svn 1.7 we have HTTPv2 which doesn't use so many
PROPFIND requests that we tried to reduce using DAV properties cache.
So I'm propose just to remove DAV properties cache from ra_serf.
Objections? Comments?
PS: See attached patch in case you'd like test performance.
--
Ivan Zhakov
Index: subversion/libsvn_ra_serf/property.c
===================================================================
--- subversion/libsvn_ra_serf/property.c (revision 1070442)
+++ subversion/libsvn_ra_serf/property.c (working copy)
@@ -87,9 +87,6 @@
/* the list of requested properties */
const svn_ra_serf__dav_props_t *find_props;
- /* should we cache the values of this propfind in our session? */
- svn_boolean_t cache_props;
-
/* hash table that will be updated with the properties
*
* This can be shared between multiple svn_ra_serf__propfind_context_t
@@ -385,19 +382,7 @@
ctx->current_path, ctx->rev,
ns, pname, val_str,
ctx->pool);
- if (ctx->cache_props)
- {
- ns = apr_pstrdup(ctx->sess->pool, info->ns);
- pname = apr_pstrdup(ctx->sess->pool, info->name);
- val = apr_pmemdup(ctx->sess->pool, info->val, info->val_len);
- val_str = svn_string_ncreate(val, info->val_len, ctx->sess->pool);
- svn_ra_serf__set_ver_prop(ctx->sess->cached_props,
- ctx->current_path, ctx->rev,
- ns, pname, val_str,
- ctx->sess->pool);
- }
-
svn_ra_serf__xml_pop_state(parser);
}
@@ -533,40 +518,6 @@
return SVN_NO_ERROR;
}
-static svn_boolean_t
-check_cache(apr_hash_t *ret_props,
- svn_ra_serf__session_t *sess,
- const char *path,
- svn_revnum_t rev,
- const svn_ra_serf__dav_props_t *find_props,
- apr_pool_t *pool)
-{
- svn_boolean_t cache_hit = TRUE;
- const svn_ra_serf__dav_props_t *prop;
-
- /* check to see if we have any of this information cached */
- prop = find_props;
- while (prop && prop->namespace)
- {
- const svn_string_t *val;
-
- val = svn_ra_serf__get_ver_prop_string(sess->cached_props, path, rev,
- prop->namespace, prop->name);
- if (val)
- {
- svn_ra_serf__set_ver_prop(ret_props, path, rev,
- prop->namespace, prop->name, val, pool);
- }
- else
- {
- cache_hit = FALSE;
- }
- prop++;
- }
-
- return cache_hit;
-}
-
/*
* This function will deliver a PROP_CTX PROPFIND request in the SESS
* serf context for the properties listed in LOOKUP_PROPS at URL for
@@ -597,25 +548,10 @@
svn_ra_serf__handler_t *handler;
svn_ra_serf__xml_parser_t *parser_ctx;
- if (cache_props)
- {
- svn_boolean_t cache_satisfy;
-
- cache_satisfy = check_cache(ret_props, sess, path, rev, find_props,
- pool);
-
- if (cache_satisfy)
- {
- *prop_ctx = NULL;
- return SVN_NO_ERROR;
- }
- }
-
new_prop_ctx = apr_pcalloc(pool, sizeof(*new_prop_ctx));
new_prop_ctx->pool = apr_hash_pool_get(ret_props);
new_prop_ctx->path = path;
- new_prop_ctx->cache_props = cache_props;
new_prop_ctx->find_props = find_props;
new_prop_ctx->ret_props = ret_props;
new_prop_ctx->depth = depth;
Index: subversion/libsvn_ra_serf/ra_serf.h
===================================================================
--- subversion/libsvn_ra_serf/ra_serf.h (revision 1070543)
+++ subversion/libsvn_ra_serf/ra_serf.h (working copy)
@@ -132,9 +132,6 @@
/* Our Version-Controlled-Configuration; may be NULL until we know it. */
const char *vcc_url;
- /* Cached properties */
- apr_hash_t *cached_props;
-
/* Authentication related properties. */
svn_auth_iterstate_t *auth_state;
int auth_attempts;
Index: subversion/libsvn_ra_serf/serf.c
===================================================================
--- subversion/libsvn_ra_serf/serf.c (revision 1070442)
+++ subversion/libsvn_ra_serf/serf.c (working copy)
@@ -354,7 +354,6 @@
serf_sess->pool = svn_pool_create(pool);
serf_sess->bkt_alloc = serf_bucket_allocator_create(serf_sess->pool, NULL,
NULL);
- serf_sess->cached_props = apr_hash_make(serf_sess->pool);
serf_sess->wc_callbacks = callbacks;
serf_sess->wc_callback_baton = callback_baton;
serf_sess->wc_progress_baton = callbacks->progress_baton;
