Hi,
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 easily could take several
seconds over WAN.
To solve this problem I propose to introduce RA context abstraction
layer for managing set of RA sessions for different repositories and
reuse them if possible. In the first version I made this layer
libsvn_client private, but in we can make it part of ra-loader in
future.
The patch is attached. It passes all tests of course. I'm very
interested for feedback. Proposed solution should make our svn merge
code much easy to maintain and significantly faster.
[[[
Introduce repository access abstraction layer for managing RA session for
different repositories.
* subversion/include/private/svn_client_private.h
(svn_client__ra_session_release): New.
(svn_client__ra_session_from_path2): Document that created session will be
automatically returned back to RA session cache on pool cleanup.
* subversion/libsvn_client/ra.c
(): Include ra_ctx.h.
(svn_client__open_ra_session_internal): Use svn_client__ra_session_open().
(svn_client__ra_session_release): New. Wrapper around
svn_client__ra_ctx_release_session()
* subversion/include/svn_client.h
(svn_client_ctx_t): Add RA_CTX member.
* subversion/libsvn_client/ctx.c
(): Include ra_ctx.h.
(svn_client_create_context2): Initialize RA_CTX.
* subversion/libsvn_client/log.c
(svn_client_log5): Return RA session back to session cache.
* subversion/libsvn_client/ra_ctx.c
* subversion/libsvn_client/ra_ctx.h
(svn_client__ra_ctx_t, svn_client__ra_ctx_create,
svn_client__ra_session_open, svn_client__ra_ctx_release_session): New.
]]]
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Index: subversion/include/private/svn_client_private.h
===================================================================
--- subversion/include/private/svn_client_private.h (revision 1489316)
+++ subversion/include/private/svn_client_private.h (working copy)
@@ -136,6 +136,9 @@
Use authentication baton cached in CTX to authenticate against the
repository.
+ The *RA_SESSION_P will be automatically returned to session cache
+ on POOL cleanup or by explicit svn_client__ra_session_release() call
+
Use POOL for all allocations. */
svn_error_t *
svn_client__ra_session_from_path2(svn_ra_session_t **ra_session_p,
@@ -147,6 +150,11 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
+/* Release repository access SESSION back to CTX session cache. */
+svn_error_t *
+svn_client__ra_session_release(svn_client_ctx_t *ctx,
+ svn_ra_session_t *session);
+
/* Given PATH_OR_URL, which contains either a working copy path or an
absolute URL, a peg revision PEG_REVISION, and a desired revision
REVISION, find the path at which that object exists in REVISION,
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1489316)
+++ subversion/include/svn_client.h (working copy)
@@ -1014,6 +1014,12 @@
* @since New in 1.7. */
svn_wc_context_t *wc_ctx;
+ /** A repository access context for the client operation to use.
+ * this is initialized by svn_client_create_context() and should never
+ * be directly used or changed.
+ *
+ * @since New in 1.9. */
+ void *ra_ctx;
} svn_client_ctx_t;
/** Initialize a client context.
Index: subversion/libsvn_client/ctx.c
===================================================================
--- subversion/libsvn_client/ctx.c (revision 1489316)
+++ subversion/libsvn_client/ctx.c (working copy)
@@ -33,7 +33,7 @@
#include "svn_error.h"
#include "private/svn_wc_private.h"
-
+#include "ra_ctx.h"
/*** Code. ***/
@@ -101,6 +101,8 @@
SVN_ERR(svn_wc_context_create(&(*ctx)->wc_ctx, cfg_config, pool,
pool));
+ (*ctx)->ra_ctx = svn_client__ra_ctx_create(cfg_hash, pool);
+
return SVN_NO_ERROR;
}
Index: subversion/libsvn_client/log.c
===================================================================
--- subversion/libsvn_client/log.c (revision 1489316)
+++ subversion/libsvn_client/log.c (working copy)
@@ -863,6 +863,6 @@
discover_changed_paths, strict_node_history,
include_merged_revisions, revprops, real_receiver,
real_receiver_baton, ctx, pool));
-
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session));
return SVN_NO_ERROR;
}
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 1489316)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -38,6 +38,7 @@
#include "svn_mergeinfo.h"
#include "client.h"
#include "mergeinfo.h"
+#include "ra_ctx.h"
#include "svn_private_config.h"
#include "private/svn_wc_private.h"
@@ -387,10 +388,10 @@
/* Try to open the RA session. If this is our last attempt,
don't accept corrected URLs from the RA provider. */
- SVN_ERR(svn_ra_open4(ra_session,
- attempts_left == 0 ? NULL : &corrected,
- base_url, uuid, cbtable, cb, ctx->config,
- result_pool));
+ SVN_ERR(svn_client__ra_ctx_open_session(
+ ra_session, attempts_left == 0 ? NULL : &corrected,
+ ctx->ra_ctx, base_url, uuid, cbtable, cb,
+ result_pool, scratch_pool));
/* No error and no corrected URL? We're done here. */
if (! corrected)
@@ -422,8 +423,9 @@
}
else
{
- SVN_ERR(svn_ra_open4(ra_session, NULL, base_url,
- uuid, cbtable, cb, ctx->config, result_pool));
+ SVN_ERR(svn_client__ra_ctx_open_session(ra_session, NULL, ctx->ra_ctx,
+ base_url, uuid, cbtable,
+ cb, result_pool, scratch_pool));
}
return SVN_NO_ERROR;
@@ -1145,3 +1147,11 @@
return reb;
}
+
+svn_error_t *
+svn_client__ra_session_release(svn_client_ctx_t *ctx,
+ svn_ra_session_t *session)
+{
+ svn_client__ra_ctx_release_session(ctx->ra_ctx, session);
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_client/ra_ctx.c
===================================================================
--- subversion/libsvn_client/ra_ctx.c (revision 0)
+++ subversion/libsvn_client/ra_ctx.c (working copy)
@@ -0,0 +1,403 @@
+/*
+ * ra.c : RA session abstraction layer
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+#include <apr_pools.h>
+
+#include "svn_dirent_uri.h"
+#include "svn_private_config.h"
+
+#include "ra_ctx.h"
+#include "private/svn_debug.h"
+
+#if 0
+#define RCTX_DBG(x) SVN_DBG(x)
+#else
+#define RCTX_DBG(x) while(0)
+#endif
+
+typedef struct cached_session_s
+{
+ svn_ra_session_t *session;
+
+ /* POOL that owns this session. NULL if session is not used. */
+ apr_pool_t *owner_pool;
+
+ /* Current callbacks table. */
+ svn_ra_callbacks2_t *cb_table;
+
+ /* Current callbacks table. */
+ void *cb_baton;
+
+ /* ID of RA session. Used only for diagnostics purpose. */
+ int id;
+} cached_session_t;
+
+struct svn_client__ra_ctx_s
+{
+ /* Hashtable of cached RA sessions. Keys are RA_SESSION_T and values
+ * are CACHED_SESION_T pointers. */
+ apr_hash_t *cached_session;
+ apr_pool_t *pool;
+ apr_hash_t *config;
+
+ /* Next ID for RA sessions. Used only for diagnostics purpose. */
+ int next_id;
+};
+
+static apr_status_t
+cleanup_session(void *data)
+{
+ cached_session_t *cache_entry = data;
+
+ cache_entry->owner_pool = NULL;
+ cache_entry->cb_table = NULL;
+ cache_entry->cb_baton = NULL;
+
+ RCTX_DBG(("SESSION(%d): Released\r\n", cache_entry->id));
+
+ return APR_SUCCESS;
+}
+
+svn_client__ra_ctx_t *
+svn_client__ra_ctx_create(apr_hash_t *config,
+ apr_pool_t *pool)
+{
+ svn_client__ra_ctx_t *ctx = apr_pcalloc(pool, sizeof(*ctx));
+
+ ctx->pool = pool;
+ ctx->cached_session = apr_hash_make(pool);
+ ctx->config = config;
+
+ return ctx;
+}
+
+static svn_error_t *
+get_wc_contents(void *baton,
+ svn_stream_t **contents,
+ const svn_checksum_t *checksum,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (!b->cb_table->get_wc_contents)
+ {
+ *contents = NULL;
+ return SVN_NO_ERROR;
+ }
+
+ return b->cb_table->get_wc_contents(b->cb_baton, contents, checksum, pool);
+}
+
+static svn_error_t *
+open_tmp_file(apr_file_t **fp,
+ void *baton,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+ return svn_error_trace(b->cb_table->open_tmp_file(fp, b->cb_baton, pool));
+}
+
+/* This implements the 'svn_ra_get_wc_prop_func_t' interface. */
+static svn_error_t *
+get_wc_prop(void *baton,
+ const char *relpath,
+ const char *name,
+ const svn_string_t **value,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->get_wc_prop)
+ {
+ return svn_error_trace(
+ b->cb_table->get_wc_prop(b->cb_baton, relpath, name, value,
+ pool));
+ }
+ else
+ {
+ *value = NULL;
+ return SVN_NO_ERROR;
+ }
+}
+
+/* This implements the 'svn_ra_push_wc_prop_func_t' interface. */
+static svn_error_t *
+push_wc_prop(void *baton,
+ const char *relpath,
+ const char *name,
+ const svn_string_t *value,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->push_wc_prop)
+ {
+ return svn_error_trace(
+ b->cb_table->push_wc_prop(b->cb_baton, relpath, name, value,
+ pool));
+ }
+ else
+ {
+ return SVN_NO_ERROR;
+ }
+}
+
+
+/* This implements the 'svn_ra_set_wc_prop_func_t' interface. */
+static svn_error_t *
+set_wc_prop(void *baton,
+ const char *path,
+ const char *name,
+ const svn_string_t *value,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+ if (b->cb_table->set_wc_prop)
+ {
+ return svn_error_trace(
+ b->cb_table->set_wc_prop(b->cb_baton, path, name, value,
+ pool));
+ }
+ else
+ {
+ return SVN_NO_ERROR;
+ }
+}
+
+/* This implements the `svn_ra_invalidate_wc_props_func_t' interface. */
+static svn_error_t *
+invalidate_wc_props(void *baton,
+ const char *path,
+ const char *prop_name,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->invalidate_wc_props)
+ {
+ return svn_error_trace(
+ b->cb_table->invalidate_wc_props(b->cb_baton, path,
+ prop_name, pool));
+ }
+ else
+ {
+ return SVN_NO_ERROR;
+ }
+}
+
+static svn_error_t *
+get_client_string(void *baton,
+ const char **name,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->get_client_string)
+ {
+ return svn_error_trace(
+ b->cb_table->get_client_string(b->cb_baton, name, pool));
+ }
+ else
+ {
+ *name = NULL;
+ return SVN_NO_ERROR;
+ }
+}
+
+static svn_error_t *
+cancel_callback(void *baton)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->cancel_func)
+ {
+ return svn_error_trace(b->cb_table->cancel_func(b->cb_baton));
+ }
+ else
+ {
+ return SVN_NO_ERROR;
+ }
+}
+
+static void
+progress_func(apr_off_t progress,
+ apr_off_t total,
+ void *baton,
+ apr_pool_t *pool)
+{
+ cached_session_t *b = baton;
+
+ if (b->cb_table->progress_func)
+ b->cb_table->progress_func(progress, total, b->cb_table->progress_baton,
+ pool);
+}
+
+static svn_error_t *
+find_session_by_url(cached_session_t **cache_entry_p,
+ svn_client__ra_ctx_t *ctx,
+ const char *url,
+ apr_pool_t *scratch_pool)
+{
+ apr_hash_index_t *hi;
+
+ for (hi = apr_hash_first(scratch_pool, ctx->cached_session);
+ hi; hi = apr_hash_next(hi))
+ {
+ cached_session_t *cache_entry = svn__apr_hash_index_val(hi);
+
+ if (cache_entry->owner_pool == NULL)
+ {
+ const char *root_url;
+ SVN_ERR(svn_ra_get_repos_root2(cache_entry->session, &root_url,
+ scratch_pool));
+
+ if (svn_uri__is_ancestor(root_url, url))
+ {
+ const char *session_url;
+ SVN_ERR(svn_ra_get_session_url(cache_entry->session,
+ &session_url, scratch_pool));
+
+ if (strcmp(session_url, url) != 0)
+ {
+ RCTX_DBG(("SESSION(%d): Reparent from '%s' to '%s'\n",
cache_entry->id, session_url, url));
+ SVN_ERR(svn_ra_reparent(cache_entry->session, url,
scratch_pool));
+ }
+
+ RCTX_DBG(("SESSION(%d): Reused\n", cache_entry->id));
+
+ *cache_entry_p = cache_entry;
+ return SVN_NO_ERROR;
+ }
+ }
+ }
+
+ *cache_entry_p = NULL;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__ra_ctx_open_session(svn_ra_session_t **session_p,
+ const char **corrected_p,
+ svn_client__ra_ctx_t *ctx,
+ const char *base_url,
+ const char *uuid,
+ svn_ra_callbacks2_t *cbtable,
+ void *callback_baton,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ cached_session_t *cache_entry;
+
+ if (corrected_p)
+ *corrected_p = NULL;
+
+ SVN_ERR(find_session_by_url(&cache_entry, ctx, base_url, scratch_pool));
+
+ if (cache_entry)
+ {
+ /* We found existing applicable session. Check UUID if requested. */
+ if (uuid)
+ {
+ const char *repository_uuid;
+
+ SVN_ERR(svn_ra_get_uuid2(cache_entry->session, &repository_uuid,
+ scratch_pool));
+ if (strcmp(uuid, repository_uuid) != 0)
+ {
+ /* Duplicate the uuid as it is allocated in sesspool */
+ return svn_error_createf(SVN_ERR_RA_UUID_MISMATCH, NULL,
+ _("Repository UUID '%s' doesn't "
+ "match expected UUID '%s'"),
+ repository_uuid, uuid);
+ }
+ }
+ }
+ else
+ {
+ /* No existing RA session found. Open new one. */
+ svn_ra_callbacks2_t *cbtable_sink;
+ svn_ra_session_t *session;
+
+ cache_entry = apr_pcalloc(ctx->pool, sizeof(*cache_entry));
+
+ SVN_ERR(svn_ra_create_callbacks(&cbtable_sink, ctx->pool));
+ cbtable_sink->open_tmp_file = open_tmp_file;
+ cbtable_sink->get_wc_prop = get_wc_prop;
+ cbtable_sink->set_wc_prop = set_wc_prop;
+ cbtable_sink->push_wc_prop = push_wc_prop;
+ cbtable_sink->invalidate_wc_props = invalidate_wc_props;
+ cbtable_sink->auth_baton = cbtable->auth_baton; /* new-style */
+ cbtable_sink->progress_func = progress_func;
+ cbtable_sink->progress_baton = cache_entry;
+ cbtable_sink->cancel_func = cancel_callback;
+ cbtable_sink->get_client_string = get_client_string;
+ cbtable_sink->get_wc_contents = get_wc_contents;
+
+ cache_entry->owner_pool = result_pool;
+ cache_entry->cb_table = cbtable;
+ cache_entry->cb_baton = callback_baton;
+ cache_entry->id = ctx->next_id;
+
+ SVN_ERR(svn_ra_open4(&session, corrected_p, base_url, uuid, cbtable_sink,
+ cache_entry, ctx->config, ctx->pool));
+
+ if (corrected_p && *corrected_p)
+ {
+ /* Caller is ready to follow redirection and we got redirection.
+ Just return corrected URL without RA session. */
+ return SVN_NO_ERROR;
+ }
+
+ cache_entry->session = session;
+
+ RCTX_DBG(("SESSION(%d): Open('%s')\n", cache_entry->id, base_url));
+
+ apr_hash_set(ctx->cached_session, &cache_entry->session,
+ sizeof(cache_entry->session), cache_entry);
+ ctx->next_id++;
+ }
+
+ cache_entry->owner_pool = result_pool;
+ cache_entry->cb_table = cbtable;
+ cache_entry->cb_baton = callback_baton;
+ apr_pool_cleanup_register(result_pool, cache_entry, cleanup_session,
+ apr_pool_cleanup_null);
+
+ *session_p = cache_entry->session;
+
+ return SVN_NO_ERROR;
+}
+
+void
+svn_client__ra_ctx_release_session(svn_client__ra_ctx_t *ctx,
+ svn_ra_session_t *session)
+{
+ cached_session_t *cache_entry = apr_hash_get(ctx->cached_session,
+ &session, sizeof(session));
+
+ SVN_ERR_ASSERT_NO_RETURN(cache_entry != NULL);
+ SVN_ERR_ASSERT_NO_RETURN(cache_entry->owner_pool != NULL);
+
+ apr_pool_cleanup_run(cache_entry->owner_pool, cache_entry,
+ cleanup_session);
+}
Index: subversion/libsvn_client/ra_ctx.h
===================================================================
--- subversion/libsvn_client/ra_ctx.h (revision 0)
+++ subversion/libsvn_client/ra_ctx.h (working copy)
@@ -0,0 +1,75 @@
+/**
+ * ra_ctx.h : RA session abstraction layer
+ *
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ */
+
+#ifndef SVN_LIBSVN_RA_CTX_H
+#define SVN_LIBSVN_RA_CTX_H
+
+#include <apr_pools.h>
+
+#include "svn_auth.h"
+#include "svn_ra.h"
+#include "svn_types.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+typedef struct svn_client__ra_ctx_s svn_client__ra_ctx_t;
+
+/* Allocates RA_CTX structure in POOL. Will use CONFIG for
+ for RA sessions created in this context. */
+svn_client__ra_ctx_t *
+svn_client__ra_ctx_create(apr_hash_t *config,
+ apr_pool_t *pool);
+
+/* Open new repository access session to the repository at BASE_URL or
+ reuses existing session cached in CTX.
+
+ The function behavior is the same as svn_ra_open4() with ability
+ to reuse sessions for same repository.
+
+ The created session will be automatically returned to CTX on RESULT_POOL
+ cleanup or by explicit svn_client__ra_ctx_release_session() call.
+
+ Uses SCRATCH_POOL for temporary allocations. */
+svn_error_t *
+svn_client__ra_ctx_open_session(svn_ra_session_t **session_p,
+ const char **corrected_p,
+ svn_client__ra_ctx_t *ctx,
+ const char *base_url,
+ const char *uuid,
+ svn_ra_callbacks2_t *cbtable,
+ void *callback_baton,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool);
+
+/* Returns RA SESSION back to CTX. */
+void
+svn_client__ra_ctx_release_session(svn_client__ra_ctx_t *ctx,
+ svn_ra_session_t *session);
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+#endif /* SVN_LIBSVN_RA_CTX_H */
