Hi,
Automatic merge creates minimum 8 connection using code in trunk. The
RA cache framework implemented in reuse-ra-session reduce number of
used RA sessions to 3.
The attached patch for reuse-ra-session branch reworks merge.c a bit
to use just 2 RA sessions. It also resolves RA CACHE TODO: comment and
makes code more clear IMO.
I'm pretty sure about the patch itself. But may be proposed changes
are out of scope ra-session-reuse branch. So this patch should be
committed to reuse-ra-session branch or to trunk after
reuse-ra-session branch will be merged to trunk. Any opinions?
[[[
Optimize and simplify RA session usage in merge code.
* subversion/libsvn_client/merge.c
(normalize_merge_sources_internal): Open new temporary RA session if
RA_SESSION argument is NULL. Update docstring to document new behavior.
(ensure_ra_session_url): Remove now unused function.
(do_merge): Remove SRC_SESSION argument and code to save/restore session
URL for passed RA session. Open and release RA session for every merge
source -- RA cache framework will reuse them if possible. All callers
are updated.
(merge_cousins_and_supplement_mergeinfo): Remove URL1_RA_SESSION and
URL2_RA_SESSION arguments. Pass NULL as RA_SESSION argument to
normalize_merge_sources_internal() -- it will open/release RA session
internally. All callers are updated.
(merge_locked): Remove SESSPOOL and release RA session early. Pass NULL
as RA_SESSION argument to normalize_merge_sources_internal() -- it
will open/release RA session internally.
(merge_reintegrate_locked): Release temporary RA sessions early.
(merge_peg_locked): Remove SESSPOOL and release RA session early.
(do_automatic_merge_locked): Release RIGHT_RA_SESSION early. Pass NULL
as RA_SESSION argument to normalize_merge_sources_internal().
]]]
--
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1657782)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -7023,7 +7023,8 @@
}
/* Similar to normalize_merge_sources() except the input MERGE_RANGE_TS is a
- * rangelist.
+ * rangelist. RA_SESSION may be NULL, in this case temporary RA session will
+ * be opened using SOURCE_LOC->URL.
*/
static svn_error_t *
normalize_merge_sources_internal(apr_array_header_t **merge_sources_p,
@@ -7039,6 +7040,7 @@
svn_revnum_t trim_revision = SVN_INVALID_REVNUM;
apr_array_header_t *segments;
int i;
+ svn_boolean_t new_ra_session = FALSE;
/* Initialize our return variable. */
*merge_sources_p = apr_array_make(result_pool, 1, sizeof(merge_source_t *));
@@ -7047,6 +7049,14 @@
if (merge_range_ts->nelts == 0)
return SVN_NO_ERROR;
+ if (!ra_session)
+ {
+ SVN_ERR(svn_client_open_ra_session2(&ra_session, source_loc->url,
+ NULL, ctx, scratch_pool,
+ scratch_pool));
+ new_ra_session = TRUE;
+ }
+
/* Find the extremes of the revisions across our set of ranges. */
merge_range_find_extremes(&oldest_requested, &youngest_requested,
merge_range_ts);
@@ -7173,6 +7183,9 @@
apr_array_cat(*merge_sources_p, merge_sources);
}
+ if (new_ra_session)
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session));
+
return SVN_NO_ERROR;
}
@@ -9595,41 +9608,6 @@
return SVN_NO_ERROR;
}
-/** Ensure that *RA_SESSION is opened to URL, either by reusing
- * *RA_SESSION if it is non-null and already opened to URL's
- * repository, or by allocating a new *RA_SESSION in POOL.
- * (RA_SESSION itself cannot be null, of course.)
- *
- * CTX is used as for svn_client_open_ra_session().
- */
-static svn_error_t *
-ensure_ra_session_url(svn_ra_session_t **ra_session,
- const char *url,
- const char *wri_abspath,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
-{
- svn_error_t *err = SVN_NO_ERROR;
-
- if (*ra_session)
- {
- err = svn_ra_reparent(*ra_session, url, pool);
- }
-
- /* SVN_ERR_RA_ILLEGAL_URL is raised when url doesn't point to the same
- repository as ra_session. */
- if (! *ra_session || (err && err->apr_err == SVN_ERR_RA_ILLEGAL_URL))
- {
- svn_error_clear(err);
- /* RA_CACHE TODO: release RA session */
- err = svn_client_open_ra_session2(ra_session, url, wri_abspath,
- ctx, pool, pool);
- }
- SVN_ERR(err);
-
- return SVN_NO_ERROR;
-}
-
/* Drive a merge of MERGE_SOURCES into working copy node TARGET
and possibly record mergeinfo describing the merge -- see
RECORD_MERGEINFO().
@@ -9709,7 +9687,6 @@
svn_boolean_t *use_sleep,
const apr_array_header_t *merge_sources,
const merge_target_t *target,
- svn_ra_session_t *src_session,
svn_boolean_t sources_related,
svn_boolean_t same_repos,
svn_boolean_t ignore_mergeinfo,
@@ -9732,8 +9709,6 @@
const char *preserved_exts_str;
int i;
svn_boolean_t checked_mergeinfo_capability = FALSE;
- svn_ra_session_t *ra_session1 = NULL;
- const char *old_src_session_url = NULL;
apr_pool_t *iterpool;
const svn_diff_tree_processor_t *processor;
@@ -9851,13 +9826,6 @@
processor = merge_processor;
}
- if (src_session)
- {
- SVN_ERR(svn_ra_get_session_url(src_session, &old_src_session_url,
- scratch_pool));
- ra_session1 = src_session;
- }
-
for (i = 0; i < merge_sources->nelts; i++)
{
svn_node_kind_t src1_kind;
@@ -9864,6 +9832,7 @@
merge_source_t *source =
APR_ARRAY_IDX(merge_sources, i, merge_source_t *);
single_range_conflict_report_t *conflicted_range_report;
+ svn_ra_session_t *ra_session1;
svn_ra_session_t *ra_session2;
svn_pool_clear(iterpool);
@@ -9875,8 +9844,9 @@
continue;
/* Establish RA sessions to our URLs, reuse where possible. */
- SVN_ERR(ensure_ra_session_url(&ra_session1, source->loc1->url,
- target->abspath, ctx, scratch_pool));
+ SVN_ERR(svn_client_open_ra_session2(&ra_session1, source->loc1->url,
+ target->abspath, ctx,
+ iterpool, iterpool));
SVN_ERR(svn_client_open_ra_session2(&ra_session2, source->loc2->url,
target->abspath, ctx, iterpool,
@@ -9957,6 +9927,7 @@
while (source);
SVN_ERR(svn_client__ra_session_release(ctx, ra_session2));
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session1));
/* The final mergeinfo on TARGET_WCPATH may itself elide. */
if (! dry_run)
@@ -10001,9 +9972,6 @@
merge_cmd_baton.tree_conflicted_abspaths);
}
- if (src_session)
- SVN_ERR(svn_ra_reparent(src_session, old_src_session_url, iterpool));
-
svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
@@ -10016,9 +9984,8 @@
Set *CONFLICT_REPORT to indicate if there were any conflicts, as in
do_merge().
- The diff to be merged is between SOURCE->loc1 (in URL1_RA_SESSION1)
- and SOURCE->loc2 (in URL2_RA_SESSION2); YCA is their youngest
- common ancestor.
+ The diff to be merged is between SOURCE->loc1 and SOURCE->loc2;
+ YCA is their youngest common ancestor.
SAME_REPOS must be true if and only if the source URLs are in the same
repository as the target working copy.
@@ -10036,8 +10003,6 @@
merge_cousins_and_supplement_mergeinfo(conflict_report_t **conflict_report,
svn_boolean_t *use_sleep,
const merge_target_t *target,
- svn_ra_session_t *URL1_ra_session,
- svn_ra_session_t *URL2_ra_session,
const merge_source_t *source,
const svn_client__pathrev_t *yca,
svn_boolean_t same_repos,
@@ -10060,9 +10025,6 @@
subtree mergeinfo, then this will help keep memory use in check. */
apr_pool_t *subpool = svn_pool_create(scratch_pool);
- assert(session_url_is(URL1_ra_session, source->loc1->url, scratch_pool));
- assert(session_url_is(URL2_ra_session, source->loc2->url, scratch_pool));
-
SVN_ERR_ASSERT(svn_dirent_is_absolute(target->abspath));
SVN_ERR_ASSERT(! source->ancestral);
@@ -10070,13 +10032,13 @@
&remove_sources, source->loc1,
svn_rangelist__initialize(source->loc1->rev, yca->rev, TRUE,
scratch_pool),
- URL1_ra_session, ctx, scratch_pool, subpool));
+ NULL, ctx, scratch_pool, subpool));
SVN_ERR(normalize_merge_sources_internal(
&add_sources, source->loc2,
svn_rangelist__initialize(yca->rev, source->loc2->rev, TRUE,
scratch_pool),
- URL2_ra_session, ctx, scratch_pool, subpool));
+ NULL, ctx, scratch_pool, subpool));
*conflict_report = NULL;
@@ -10091,7 +10053,7 @@
APR_ARRAY_PUSH(faux_sources, const merge_source_t *) = source;
SVN_ERR(do_merge(&modified_subtrees, NULL, conflict_report, use_sleep,
faux_sources, target,
- URL1_ra_session, TRUE, same_repos,
+ TRUE, same_repos,
FALSE /*ignore_mergeinfo*/, diff_ignore_ancestry,
force_delete, dry_run, FALSE, NULL, TRUE,
FALSE, depth, merge_options, ctx,
@@ -10131,7 +10093,7 @@
svn_pool_clear(subpool);
SVN_ERR(do_merge(NULL, add_result_catalog, conflict_report, use_sleep,
add_sources, target,
- URL1_ra_session, TRUE, same_repos,
+ TRUE, same_repos,
FALSE /*ignore_mergeinfo*/, diff_ignore_ancestry,
force_delete, dry_run, TRUE,
modified_subtrees, TRUE,
@@ -10146,7 +10108,7 @@
svn_pool_clear(subpool);
SVN_ERR(do_merge(NULL, remove_result_catalog, conflict_report, use_sleep,
remove_sources, target,
- URL1_ra_session, TRUE, same_repos,
+ TRUE, same_repos,
FALSE /*ignore_mergeinfo*/, diff_ignore_ancestry,
force_delete, dry_run, TRUE,
modified_subtrees, TRUE,
@@ -10400,7 +10362,6 @@
svn_error_t *err;
svn_boolean_t use_sleep = FALSE;
svn_client__pathrev_t *yca = NULL;
- apr_pool_t *sesspool;
svn_boolean_t same_repos;
/* ### FIXME: This function really ought to do a history check on
@@ -10414,13 +10375,12 @@
/* Open RA sessions to both sides of our merge source, and resolve URLs
* and revisions. */
- sesspool = svn_pool_create(scratch_pool);
SVN_ERR(svn_client__ra_session_from_path2(
&ra_session1, &source1_loc,
- source1, NULL, revision1, revision1, ctx, sesspool));
+ source1, NULL, revision1, revision1, ctx, scratch_pool));
SVN_ERR(svn_client__ra_session_from_path2(
&ra_session2, &source2_loc,
- source2, NULL, revision2, revision2, ctx, sesspool));
+ source2, NULL, revision2, revision2, ctx, scratch_pool));
/* We can't do a diff between different repositories. */
/* ### We should also insist that the root URLs of the two sources match,
@@ -10440,6 +10400,10 @@
&yca, source1_loc, source2_loc, ra_session1, ctx,
scratch_pool, scratch_pool));
+ /* Close our temporary RA sessions. */
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session2));
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session1));
+
/* Check for a youngest common ancestor. If we have one, we'll be
doing merge tracking.
@@ -10471,7 +10435,7 @@
&merge_sources, source1_loc,
svn_rangelist__initialize(source1_loc->rev, yca->rev, TRUE,
scratch_pool),
- ra_session1, ctx, scratch_pool, scratch_pool));
+ NULL, ctx, scratch_pool, scratch_pool));
}
/* If the common ancestor matches the left side of our merge,
then we only need to merge the right side. */
@@ -10482,7 +10446,7 @@
&merge_sources, source2_loc,
svn_rangelist__initialize(yca->rev, source2_loc->rev, TRUE,
scratch_pool),
- ra_session2, ctx, scratch_pool, scratch_pool));
+ NULL, ctx, scratch_pool, scratch_pool));
}
/* And otherwise, we need to do both: reverse merge the left
side, and merge the right. */
@@ -10497,8 +10461,6 @@
err = merge_cousins_and_supplement_mergeinfo(conflict_report,
&use_sleep,
target,
- ra_session1,
- ra_session2,
&source,
yca,
same_repos,
@@ -10510,10 +10472,6 @@
ctx,
result_pool,
scratch_pool);
- /* Close our temporary RA sessions (this could've happened
- after the second call to normalize_merge_sources() inside
- the merge_cousins_and_supplement_mergeinfo() routine). */
- svn_pool_destroy(sesspool);
if (use_sleep)
svn_io_sleep_for_timestamps(target->abspath, scratch_pool);
@@ -10532,19 +10490,11 @@
err = do_merge(NULL, NULL, conflict_report, &use_sleep,
merge_sources, target,
- ra_session1, sources_related, same_repos,
+ sources_related, same_repos,
ignore_mergeinfo, diff_ignore_ancestry, force_delete, dry_run,
record_only, NULL, FALSE, FALSE, depth, merge_options,
ctx, result_pool, scratch_pool);
- /* Close our temporary RA sessions. */
- if (!err)
- {
- SVN_ERR(svn_client__ra_session_release(ctx, ra_session2));
- SVN_ERR(svn_client__ra_session_release(ctx, ra_session1));
- }
- svn_pool_destroy(sesspool);
-
if (use_sleep)
svn_io_sleep_for_timestamps(target->abspath, scratch_pool);
@@ -11677,6 +11627,9 @@
target_ra_session, target,
ctx, scratch_pool, scratch_pool));
+ SVN_ERR(svn_client__ra_session_release(ctx, source_ra_session));
+ SVN_ERR(svn_client__ra_session_release(ctx, target_ra_session));
+
if (! source)
{
return SVN_NO_ERROR;
@@ -11692,8 +11645,6 @@
err = merge_cousins_and_supplement_mergeinfo(conflict_report,
&use_sleep,
target,
- target_ra_session,
- source_ra_session,
source, yc_ancestor,
TRUE /* same_repos */,
svn_depth_infinity,
@@ -11705,12 +11656,6 @@
ctx,
result_pool, scratch_pool);
- if (!err)
- {
- SVN_ERR(svn_client__ra_session_release(ctx, source_ra_session));
- SVN_ERR(svn_client__ra_session_release(ctx, target_ra_session));
- }
-
if (use_sleep)
svn_io_sleep_for_timestamps(target_abspath, scratch_pool);
@@ -11779,7 +11724,6 @@
svn_client__pathrev_t *source_loc;
apr_array_header_t *merge_sources;
svn_ra_session_t *ra_session;
- apr_pool_t *sesspool;
svn_boolean_t use_sleep = FALSE;
svn_error_t *err;
svn_boolean_t same_repos;
@@ -11790,14 +11734,11 @@
allow_mixed_rev, TRUE, TRUE,
ctx, scratch_pool, scratch_pool));
- /* Create a short lived session pool */
- sesspool = svn_pool_create(scratch_pool);
-
/* Open an RA session to our source URL, and determine its root URL. */
SVN_ERR(svn_client__ra_session_from_path2(
&ra_session, &source_loc,
source_path_or_url, NULL, source_peg_revision, source_peg_revision,
- ctx, sesspool));
+ ctx, scratch_pool));
/* Normalize our merge sources. */
SVN_ERR(normalize_merge_sources(&merge_sources, source_path_or_url,
@@ -11808,20 +11749,18 @@
/* Check for same_repos. */
same_repos = is_same_repos(&target->loc, source_loc, TRUE /* strict_urls */);
+ /* We're done with our RA session. */
+ SVN_ERR(svn_client__ra_session_release(ctx, ra_session));
+
/* Do the real merge! (We say with confidence that our merge
sources are both ancestral and related.) */
err = do_merge(NULL, NULL, conflict_report, &use_sleep,
- merge_sources, target, ra_session,
+ merge_sources, target,
TRUE /*sources_related*/, same_repos, ignore_mergeinfo,
diff_ignore_ancestry, force_delete, dry_run,
record_only, NULL, FALSE, FALSE, depth, merge_options,
ctx, result_pool, scratch_pool);
- /* We're done with our RA session. */
- if (!err)
- SVN_ERR(svn_client__ra_session_release(ctx, ra_session));
- svn_pool_destroy(sesspool);
-
if (use_sleep)
svn_io_sleep_for_timestamps(target_abspath, scratch_pool);
@@ -12650,8 +12589,6 @@
if (reintegrate_like)
{
merge_source_t source;
- svn_ra_session_t *base_ra_session;
- svn_ra_session_t *right_ra_session;
if (record_only)
return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
@@ -12671,17 +12608,18 @@
"and the force_delete option "
"cannot be used with this kind of merge"));
- SVN_ERR(svn_client_open_ra_session2(&right_ra_session, merge->right->url,
- target->abspath, ctx, scratch_pool,
- scratch_pool));
-
/* Check for and reject any abnormalities -- such as revisions that
* have not yet been merged in the opposite direction -- that a
* 'reintegrate' merge would have rejected. */
{
merge_source_t *source2;
+ svn_ra_session_t *right_ra_session;
svn_ra_session_t *target_ra_session;
+ SVN_ERR(svn_client_open_ra_session2(&right_ra_session,
merge->right->url,
+ target->abspath, ctx, scratch_pool,
+ scratch_pool));
+
SVN_ERR(svn_client_open_ra_session2(&target_ra_session,
target->loc.url, target->abspath,
ctx, scratch_pool, scratch_pool));
@@ -12692,6 +12630,7 @@
ctx, scratch_pool, scratch_pool));
SVN_ERR(svn_client__ra_session_release(ctx, target_ra_session));
+ SVN_ERR(svn_client__ra_session_release(ctx, right_ra_session));
}
source.loc1 = merge->base;
@@ -12698,16 +12637,9 @@
source.loc2 = merge->right;
source.ancestral = ! merge->is_reintegrate_like;
- SVN_ERR(svn_client_open_ra_session2(&base_ra_session,
- merge->base->url,
- target->abspath, ctx, scratch_pool,
- scratch_pool));
-
err = merge_cousins_and_supplement_mergeinfo(conflict_report,
&use_sleep,
target,
- base_ra_session,
- right_ra_session,
&source, merge->yca,
TRUE /* same_repos */,
depth,
@@ -12717,11 +12649,6 @@
merge_options,
ctx,
result_pool, scratch_pool);
- if (!err)
- {
- SVN_ERR(svn_client__ra_session_release(ctx, base_ra_session));
- SVN_ERR(svn_client__ra_session_release(ctx, right_ra_session));
- }
}
else /* ! merge->is_reintegrate_like */
{
@@ -12735,28 +12662,22 @@
find the base for each sutree, and then here use the oldest base
among all subtrees. */
apr_array_header_t *merge_sources;
- svn_ra_session_t *ra_session;
/* Normalize our merge sources, do_merge() requires this. See the
'MERGEINFO MERGE SOURCE NORMALIZATION' global comment. */
- SVN_ERR(svn_client_open_ra_session2(&ra_session, merge->right->url,
- target->abspath, ctx, scratch_pool,
- scratch_pool));
SVN_ERR(normalize_merge_sources_internal(
&merge_sources, merge->right,
svn_rangelist__initialize(merge->yca->rev, merge->right->rev, TRUE,
scratch_pool),
- ra_session, ctx, scratch_pool, scratch_pool));
+ NULL, ctx, scratch_pool, scratch_pool));
err = do_merge(NULL, NULL, conflict_report, &use_sleep,
- merge_sources, target, ra_session,
+ merge_sources, target,
TRUE /*related*/, TRUE /*same_repos*/,
FALSE /*ignore_mergeinfo*/, diff_ignore_ancestry,
force_delete, dry_run,
record_only, NULL, FALSE, FALSE, depth, merge_options,
ctx, result_pool, scratch_pool);
- if (!err)
- SVN_ERR(svn_client__ra_session_release(ctx, ra_session));
}
if (use_sleep)
