Author: brane
Date: Tue Feb 3 01:52:26 2015
New Revision: 1656616
URL: http://svn.apache.org/r1656616
Log:
On the reuse-ra-session branch: Split the RA session cache into a set
of active sessions and a list of inactive, open sessions.
This split has two benefits:
- it speeds up the search for an inactive session to reuse, since
the search will no longer have to iterate over active sessions;
- it lays the groundwork needet for limiting the number of
cached idle sessions and closing the least recently used idle
session when releasing an active session onto the idle list.
* BRANCH-README: Update status.
* subversion/libsvn_client/client.h: Include apr_ring.h.
(svn_client__ra_session_t): Forward declare session cache entry.
(svn_client__ra_cache_t): Rename member 'cached_session' to 'active'.
Add new member 'freelist' which contains the idle sessions.
* subversion/libsvn_client/ra_cache.c:
Include assert.h and remove obsolete include of stddef.h.
(svn_client__ra_session_t): Renamed from client_ra_session_t;
all uses updated. Added members 'ra_cache' and 'freelist' and
moved the 'id' member to the end of the structure.
(release_session): Renamed from cleanup_session; all callers updated.
Removes the session from the active set and pushes it onto the idle list.
(get_private_ra_cache): Moved elsewhere in the same file.
(cleanup_ra_cache): New; cleanup handler for the whole RA session cache.
(svn_client__ra_cache_init): Initialize the idle session list and
register the cache cleanup handler.
(find_session_by_url): Search the idle session list instead.
(svn_client__ra_cache_open_session): Initialize the cached session's
link into the idle session list.
(svn_client__ra_cache_release_session): Double-check that we really
did get the right session from the active set.
Modified:
subversion/branches/reuse-ra-session/BRANCH-README
subversion/branches/reuse-ra-session/subversion/libsvn_client/client.h
subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
Modified: subversion/branches/reuse-ra-session/BRANCH-README
URL:
http://svn.apache.org/viewvc/subversion/branches/reuse-ra-session/BRANCH-README?rev=1656616&r1=1656615&r2=1656616&view=diff
==============================================================================
--- subversion/branches/reuse-ra-session/BRANCH-README (original)
+++ subversion/branches/reuse-ra-session/BRANCH-README Tue Feb 3 01:52:26 2015
@@ -8,12 +8,19 @@ all changes made in the branch.
STATUS
======
-+ Initial implementation
-- Do not call svn_ra_* methods in find_session_by_url() because callback
- table may be destroyed at that time.
+
+done:
+- Initial implementation.
+- Separate active and inactive session lists.
+
+todo:
+- Fix timeout in davautocheck tests at log_tests.py::log_diff_moved.
+- Limit the number of unused open sessions.
+- Run performance comparisons between trunk and branch to prove that
+ the RA session cache does in fact speed things up.
- Add new RA capability to signal if RA session is stateless and reusable.
-- Implement svn_client_close_all_sessions()
-- Merge to trunk and start switching code to use new functions.
+- Implement svn_client_close_all_sessions().
+- Switch code to use new functions.
PROBLEM
=======
@@ -21,7 +28,7 @@ PROBLEM
Currently Subversion client layer creates new RA session for every
svn_client_* call. Even more: for some operations like svn_client_merge() it
creates 10-15 RA sessions. Each session creation takes significant amount of
-time: TCP connection, SSL handshake, authentication and initial handshake. It
+time: TCP connection, SSL handshake, authentication and initial handshake. It
easily could take several seconds over WAN.
PROPOSED SOLUTION
Modified: subversion/branches/reuse-ra-session/subversion/libsvn_client/client.h
URL:
http://svn.apache.org/viewvc/subversion/branches/reuse-ra-session/subversion/libsvn_client/client.h?rev=1656616&r1=1656615&r2=1656616&view=diff
==============================================================================
--- subversion/branches/reuse-ra-session/subversion/libsvn_client/client.h
(original)
+++ subversion/branches/reuse-ra-session/subversion/libsvn_client/client.h Tue
Feb 3 01:52:26 2015
@@ -28,6 +28,7 @@
#include <apr_pools.h>
+#include <apr_ring.h>
#include "svn_types.h"
#include "svn_opt.h"
@@ -46,15 +47,25 @@ extern "C" {
#endif /* __cplusplus */
+/* Forward declaration of the cached RA session structure. */
+struct svn_client__ra_session_t;
+
/* RA session cache */
typedef struct svn_client__ra_cache_t
{
- /* Hashtable of cached RA sessions. Keys are RA_SESSION_T and values
- * are CLIENT_RA_SESION_T pointers. */
- apr_hash_t *cached_session;
+ /* The pool that defines the lifetime of the cache. */
apr_pool_t *pool;
+
+ /* The config hash used to create new sessions. */
apr_hash_t *config;
+ /* Cached active RA sessions.
+ Keys are RA_SESSION_T and values are CLIENT_RA_SESION_T pointers. */
+ apr_hash_t *active;
+
+ /* List of inactive sessions available for reuse. */
+ APR_RING_HEAD(, svn_client__ra_session_t) freelist;
+
/* Next ID for RA sessions. Used only for diagnostics purpose. */
int next_id;
} svn_client__ra_cache_t;
Modified:
subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
URL:
http://svn.apache.org/viewvc/subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c?rev=1656616&r1=1656615&r2=1656616&view=diff
==============================================================================
--- subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
(original)
+++ subversion/branches/reuse-ra-session/subversion/libsvn_client/ra_cache.c
Tue Feb 3 01:52:26 2015
@@ -21,7 +21,7 @@
* ====================================================================
*/
-#include <stddef.h>
+#include <assert.h>
#include <apr_pools.h>
#include "svn_dirent_uri.h"
@@ -36,8 +36,17 @@
#define RCTX_DBG(x)
#endif
-typedef struct client_ra_session_t
+typedef struct svn_client__ra_session_t
{
+ /* The free-list link for this session. */
+ APR_RING_ENTRY(svn_client__ra_session_t) freelist;
+
+ /* The cache that owns this session. Will be set to NULL in the
+ cache cleanup handler to prevent access through dangling pointers
+ during session release. */
+ svn_client__ra_cache_t* ra_cache;
+
+ /* The actual RA session. */
svn_ra_session_t *session;
/* POOL that owns this session. NULL if session is not used. */
@@ -52,21 +61,38 @@ typedef struct client_ra_session_t
/* Repository root URL. */
const char *root_url;
- /* ID of RA session. Used only for diagnostics purpose. */
- int id;
-
/* Last progress reported by this session. */
apr_off_t last_progress;
/* Accumulated progress since last session open. */
apr_off_t progress;
-} client_ra_session_t;
+
+ /* ID of RA session. Used only for diagnostics. */
+ int id;
+} svn_client__ra_session_t;
static apr_status_t
-cleanup_session(void *data)
+release_session(void *data)
{
- client_ra_session_t *cache_entry = data;
+ svn_client__ra_session_t *cache_entry = data;
+ svn_client__ra_cache_t *ra_cache = cache_entry->ra_cache;
+
+ /* Remove the session from the active table and insert it into the
+ inactive list. */
+ if (ra_cache)
+ {
+#ifdef SVN_DEBUG
+ /* Double-check that this entry is not part of the freelist. */
+ assert(cache_entry == APR_RING_NEXT(cache_entry, freelist));
+ assert(cache_entry == APR_RING_PREV(cache_entry, freelist));
+#endif /* SVN_DEBUG */
+
+ apr_hash_set(ra_cache->active, &cache_entry->session,
+ sizeof(cache_entry->session), NULL);
+ APR_RING_INSERT_HEAD(&ra_cache->freelist, cache_entry,
+ svn_client__ra_session_t, freelist);
+ }
cache_entry->owner_pool = NULL;
cache_entry->cb_table = NULL;
@@ -77,14 +103,28 @@ cleanup_session(void *data)
return APR_SUCCESS;
}
-/* Convert a public client context pointer to a private client context
- pointer. */
-static svn_client__ra_cache_t *
-get_private_ra_cache(svn_client_ctx_t *public_ctx)
+static apr_status_t
+cleanup_ra_cache(void *data)
{
- svn_client__private_ctx_t *const private_ctx =
- svn_client__get_private_ctx(public_ctx);
- return &private_ctx->ra_cache;
+ svn_client__ra_cache_t *ra_cache = data;
+ svn_client__ra_session_t *cache_entry;
+ apr_hash_index_t *hi;
+
+ /* Reset the cache owner pointers on all the cached sessions. */
+ for (hi = apr_hash_first(ra_cache->pool, ra_cache->active);
+ hi; hi = apr_hash_next(hi))
+ {
+ cache_entry = apr_hash_this_val(hi);
+ cache_entry->ra_cache = NULL;
+ }
+
+ APR_RING_FOREACH(cache_entry, &ra_cache->freelist,
+ svn_client__ra_session_t, freelist)
+ cache_entry->ra_cache = NULL;
+
+ RCTX_DBG(("RA_CACHE: Cleanup\n"));
+
+ return APR_SUCCESS;
}
void
@@ -92,9 +132,19 @@ svn_client__ra_cache_init(svn_client__pr
apr_hash_t *config,
apr_pool_t *pool)
{
+ RCTX_DBG(("RA_CACHE: Init\n"));
+
private_ctx->ra_cache.pool = pool;
- private_ctx->ra_cache.cached_session = apr_hash_make(pool);
private_ctx->ra_cache.config = config;
+ private_ctx->ra_cache.active = apr_hash_make(pool);
+ APR_RING_INIT(&private_ctx->ra_cache.freelist,
+ svn_client__ra_session_t, freelist);
+
+ /* This cleanup must always be registered last (i.e., after the hash
+ table of active sessions is created), so that all the bits of the
+ cache remain valid when it is run. */
+ apr_pool_cleanup_register(pool, &private_ctx->ra_cache, cleanup_ra_cache,
+ apr_pool_cleanup_null);
}
static svn_error_t *
@@ -103,7 +153,7 @@ get_wc_contents(void *baton,
const svn_checksum_t *checksum,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (!b->cb_table->get_wc_contents)
{
@@ -119,7 +169,7 @@ open_tmp_file(apr_file_t **fp,
void *baton,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
return svn_error_trace(b->cb_table->open_tmp_file(fp, b->cb_baton, pool));
}
@@ -131,7 +181,7 @@ get_wc_prop(void *baton,
const svn_string_t **value,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->get_wc_prop)
{
@@ -154,7 +204,7 @@ push_wc_prop(void *baton,
const svn_string_t *value,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->push_wc_prop)
{
@@ -177,7 +227,7 @@ set_wc_prop(void *baton,
const svn_string_t *value,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->set_wc_prop)
{
return svn_error_trace(
@@ -197,7 +247,7 @@ invalidate_wc_props(void *baton,
const char *prop_name,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->invalidate_wc_props)
{
@@ -216,7 +266,7 @@ get_client_string(void *baton,
const char **name,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->get_client_string)
{
@@ -233,7 +283,7 @@ get_client_string(void *baton,
static svn_error_t *
cancel_callback(void *baton)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
if (b->cb_table->cancel_func)
{
@@ -251,7 +301,7 @@ progress_func(apr_off_t progress,
void *baton,
apr_pool_t *pool)
{
- client_ra_session_t *b = baton;
+ svn_client__ra_session_t *b = baton;
b->progress += (progress - b->last_progress);
b->last_progress = progress;
@@ -262,30 +312,39 @@ progress_func(apr_off_t progress,
}
static svn_error_t *
-find_session_by_url(client_ra_session_t **cache_entry_p,
+find_session_by_url(svn_client__ra_session_t **cache_entry_p,
svn_client__ra_cache_t *ra_cache,
const char *url,
apr_pool_t *scratch_pool)
{
- apr_hash_index_t *hi;
+ svn_client__ra_session_t *cache_entry;
+
+ APR_RING_FOREACH(cache_entry, &ra_cache->freelist,
+ svn_client__ra_session_t, freelist)
+ {
+ SVN_ERR_ASSERT(cache_entry->owner_pool == NULL);
- for (hi = apr_hash_first(scratch_pool, ra_cache->cached_session);
- hi; hi = apr_hash_next(hi))
- {
- client_ra_session_t *cache_entry = apr_hash_this_val(hi);
-
- if (cache_entry->owner_pool == NULL &&
- svn_uri__is_ancestor(cache_entry->root_url, url))
- {
- *cache_entry_p = cache_entry;
- return SVN_NO_ERROR;
- }
- }
+ if (svn_uri__is_ancestor(cache_entry->root_url, url))
+ {
+ *cache_entry_p = cache_entry;
+ return SVN_NO_ERROR;
+ }
+ }
*cache_entry_p = NULL;
return SVN_NO_ERROR;
}
+/* Convert a public client context pointer to a pointer to the RA
+ session cache in the private client context. */
+static svn_client__ra_cache_t *
+get_private_ra_cache(svn_client_ctx_t *public_ctx)
+{
+ svn_client__private_ctx_t *const private_ctx =
+ svn_client__get_private_ctx(public_ctx);
+ return &private_ctx->ra_cache;
+}
+
svn_error_t *
svn_client__ra_cache_open_session(svn_ra_session_t **session_p,
const char **corrected_p,
@@ -298,7 +357,7 @@ svn_client__ra_cache_open_session(svn_ra
apr_pool_t *scratch_pool)
{
svn_client__ra_cache_t *const ra_cache = get_private_ra_cache(ctx);
- client_ra_session_t *cache_entry;
+ svn_client__ra_session_t *cache_entry;
if (corrected_p)
*corrected_p = NULL;
@@ -339,6 +398,10 @@ svn_client__ra_cache_open_session(svn_ra
}
}
+ /* Remove the session from the freelist. */
+ APR_RING_REMOVE(cache_entry, freelist);
+ APR_RING_ELEM_INIT(cache_entry, freelist);
+
RCTX_DBG(("SESSION(%d): Reused\n", cache_entry->id));
}
else
@@ -348,6 +411,7 @@ svn_client__ra_cache_open_session(svn_ra
svn_ra_session_t *session;
cache_entry = apr_pcalloc(ra_cache->pool, sizeof(*cache_entry));
+ APR_RING_ELEM_INIT(cache_entry, freelist);
SVN_ERR(svn_ra_create_callbacks(&cbtable_sink, ra_cache->pool));
cbtable_sink->open_tmp_file = open_tmp_file;
@@ -384,8 +448,9 @@ svn_client__ra_cache_open_session(svn_ra
RCTX_DBG(("SESSION(%d): Open('%s')\n", cache_entry->id, base_url));
- apr_hash_set(ra_cache->cached_session, &cache_entry->session,
+ apr_hash_set(ra_cache->active, &cache_entry->session,
sizeof(cache_entry->session), cache_entry);
+ cache_entry->ra_cache = ra_cache;
++ra_cache->next_id;
}
@@ -393,7 +458,7 @@ svn_client__ra_cache_open_session(svn_ra
cache_entry->cb_table = cbtable;
cache_entry->cb_baton = callback_baton;
cache_entry->progress = 0;
- apr_pool_cleanup_register(result_pool, cache_entry, cleanup_session,
+ apr_pool_cleanup_register(result_pool, cache_entry, release_session,
apr_pool_cleanup_null);
*session_p = cache_entry->session;
@@ -406,12 +471,13 @@ svn_client__ra_cache_release_session(svn
svn_ra_session_t *session)
{
svn_client__ra_cache_t *const ra_cache = get_private_ra_cache(ctx);
- client_ra_session_t *cache_entry = apr_hash_get(ra_cache->cached_session,
- &session, sizeof(session));
+ svn_client__ra_session_t *cache_entry =
+ apr_hash_get(ra_cache->active, &session, sizeof(session));
SVN_ERR_ASSERT_NO_RETURN(cache_entry != NULL);
+ SVN_ERR_ASSERT_NO_RETURN(cache_entry->session == session);
SVN_ERR_ASSERT_NO_RETURN(cache_entry->owner_pool != NULL);
apr_pool_cleanup_run(cache_entry->owner_pool, cache_entry,
- cleanup_session);
+ release_session);
}
