On Thu, May 30, 2024 at 5:25 PM Nathan Hartman <hartman.nat...@gmail.com> wrote:
>
> On Thu, May 30, 2024 at 5:09 PM Sands, Daniel N. via users
> <us...@subversion.apache.org> wrote:
>>
>>
>> On 2024/02/15 17:42:59 "Sands, Daniel N. via users" wrote:
>> > On Thu, 2024-02-15 at 08:55 -0500, Nico Kadel-Garcia wrote:
>> > > [You don't often get email from nka...@gmail.com. Learn why this is
>> > > important at https://aka.ms/LearnAboutSenderIdentification ]
>> > >
>> > > On Wed, Feb 14, 2024 at 4:59 PM Sands, Daniel N. via users
>> > > <us...@subversion.apache.org> wrote:
>> > >
>> > > > So lesson learned: Always make a pristine copy of the trunk
>> before
>> > > > making ANY changes, so that there is a revision to fall back on
>> > > > where
>> > > > the two branches exactly match.
>> > >
>> > > That's what tags are for!
>> >
>> > I'd heard of tagging but wasn't sure how to do it since I'm not
>> > responsible for the releases... but it looks like you tag by using
>> the
>> > copy command. Even worse, the text under "complex tagging" shows
>> > copying your working directory to a new repo, which is what breaks
>> the
>> > file move/rename detection.
>> >
>> > On a further note, my real repo has 260 moves due to source tree
>> > restructuring. There were 290 deletions. The current move detection
>> > algorithm is an O(n^2) search to find all moves, where it ends up
>> > querying the SVN server 260*290 times for merge info, per file
>> > conflict. Perhaps it would be a good cost savings to cache the merge
>> > info for each file during the search, so that there are O(n) trips to
>> > the server and everything else is resolved locally?
>> >
>> I came up with a patch for this issue. It cuts the resolve time down
>> from literal hours in my case, to less than a minute. I can't say it's
>> production ready, but it's a template at least.
>>
>> It attacks the core of the problem, where every time it comes up with a
>> candidate pair to check, it downloads the history from the repo on each
>> file. This happened for the same left-side file hundreds of times
>> while it tried each candidate right-side file.
>>
>> The patch leaves in (commented-out) printfs to show the problem in
>> action. The other part is, now that I have to persist data as long as
>> the client context, do the temporary results pools get used for
>> anything at all? Finally, there is one change to a public API that
>> would need to be fixed.
>>
>> >
>> >
>
>
>
> Hi,
>
> Any chance you can send your patch uncompressed, with an extension like
> ".patch.txt"?
>
> Thanks,
>
> Nathan
Hi all,
Moving this discussion from users@ to dev@subversion...
The users@ discussion started at [1] is a bit disconnected in the
archive, but [2], [3], and [4] are the continuations.
There's a patch in [4], which is compressed. I asked for an
uncompressed patch, which I received right away, but I noticed just
now that it wasn't sent to the list.
The patch was the output of 'diff', not 'svn diff', against the 1.14.3
sources. It applies to trunk with 'svn patch --strip 1', with some
offsets. For convenience, I created a new patch with 'svn diff -x-p',
which applies cleanly to trunk without offsets (attached).
I haven't had a chance to really study this yet (or build or test),
but the main part of the patch is in ra.c, where
svn_client__get_youngest_common_ancestor() was calling
svn_client__get_history_as_mergeinfo(). The patch wraps those calls in
a new function get_history_from_cache_or_as_mergeinfo().
Conceptually a local in-memory cache seems reasonable, though I need
to study this patch some more (and build and test).
I'd also wonder about possible unbounded memory usage if there should
be a very large number of items to cache, but I suppose the cache size
could be limited to some reasonable value.
This part of the OP's message (at [4]) is relevant, especially the
questions about context and the public API:
> On Thu, May 30, 2024 at 5:09 PM Sands, Daniel N. via users
> <us...@subversion.apache.org> >> I came up with a patch for this issue. It
> cuts the resolve time down
>> from literal hours in my case, to less than a minute. I can't say it's
>> production ready, but it's a template at least.
>>
>> It attacks the core of the problem, where every time it comes up with a
>> candidate pair to check, it downloads the history from the repo on each
>> file. This happened for the same left-side file hundreds of times
>> while it tried each candidate right-side file.
>>
>> The patch leaves in (commented-out) printfs to show the problem in
>> action. The other part is, now that I have to persist data as long as
>> the client context, do the temporary results pools get used for
>> anything at all? Finally, there is one change to a public API that
>> would need to be fixed.
References:
[1] https://lists.apache.org/thread/8bj5rn82kyrdzlkrnrp6m60qboxvkhrj
[2] https://lists.apache.org/thread/mhgsr1sb8sq90bfzmpqn3j49m35n038w
[3] https://lists.apache.org/thread/gkf26qkrc8p7lgqknk35hxymf6w6y1dw
[4] https://lists.apache.org/thread/9voqsw845f3msobybp13gmy2808h6qw2
Cheers,
Nathan
Index: subversion/include/private/svn_client_private.h
===================================================================
--- subversion/include/private/svn_client_private.h (revision 1918192)
+++ subversion/include/private/svn_client_private.h (working copy)
@@ -116,6 +116,7 @@ svn_client__pathrev_create(const char *repos_root_
const char *repos_uuid,
svn_revnum_t rev,
const char *url,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool);
/* Return a new path-rev structure, allocated in RESULT_POOL,
@@ -126,6 +127,7 @@ svn_client__pathrev_create_with_relpath(const char
const char *repos_uuid,
svn_revnum_t rev,
const char *relpath,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool);
/* Set *PATHREV_P to a new path-rev structure, allocated in RESULT_POOL,
@@ -136,6 +138,7 @@ svn_client__pathrev_create_with_session(svn_client
svn_ra_session_t *ra_session,
svn_revnum_t rev,
const char *url,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool);
/* Return a deep copy of PATHREV, allocated in RESULT_POOL. */
@@ -148,6 +151,7 @@ svn_client__pathrev_dup(const svn_client__pathrev_
svn_client__pathrev_t *
svn_client__pathrev_join_relpath(const svn_client__pathrev_t *pathrev,
const char *relpath,
+ svn_client_ctx_t *ctx,
apr_pool_t *result_pool);
/* Return the repository-relative relpath of PATHREV. */
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 1918192)
+++ subversion/libsvn_client/client.h (working copy)
@@ -63,6 +63,10 @@ typedef struct svn_client__private_ctx_t
/* Total number of bytes transferred over network across all RA sessions. */
apr_off_t total_progress;
+ apr_hash_t *pathrev_cache;
+ apr_hash_t *pathrev_err_cache;
+ apr_hash_t *pathrev_string_cache;
+
/* The public context. */
svn_client_ctx_t public_ctx;
} svn_client__private_ctx_t;
@@ -235,6 +239,7 @@ svn_client__calc_youngest_common_ancestor(svn_clie
const svn_client__pathrev_t *loc2,
apr_hash_t *history2,
svn_boolean_t has_rev_zero_history2,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
Index: subversion/libsvn_client/conflicts.c
===================================================================
--- subversion/libsvn_client/conflicts.c (revision 1918192)
+++ subversion/libsvn_client/conflicts.c (working copy)
@@ -473,13 +473,17 @@ find_yca(svn_client__pathrev_t **yca_loc,
svn_client__pathrev_t *loc1;
svn_client__pathrev_t *loc2;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+
*yca_loc = NULL;
loc1 = svn_client__pathrev_create_with_relpath(repos_root_url, repos_uuid,
peg_rev1, repos_relpath1,
+ priv->pathrev_string_cache,
scratch_pool);
loc2 = svn_client__pathrev_create_with_relpath(repos_root_url, repos_uuid,
peg_rev2, repos_relpath2,
+ priv->pathrev_string_cache,
scratch_pool);
SVN_ERR(svn_client__get_youngest_common_ancestor(yca_loc, loc1, loc2,
ra_session, ctx,
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 1918192)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -2425,6 +2425,8 @@ svn_client__repos_to_wc_copy_dir(svn_boolean_t *ti
SVN_ERR_ASSERT(svn_dirent_is_absolute(dst_abspath));
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+
if (!same_repositories)
{
svn_client__pathrev_t *location;
@@ -2435,6 +2437,7 @@ svn_client__repos_to_wc_copy_dir(svn_boolean_t *ti
a foreign repo, surely we need a new RA session? */
SVN_ERR(svn_client__pathrev_create_with_session(&location, ra_session,
src_revnum, src_url,
+
priv->pathrev_string_cache,
scratch_pool));
SVN_ERR(svn_ra_reparent(ra_session, src_url, scratch_pool));
SVN_ERR(copy_foreign_dir(ra_session, location,
Index: subversion/libsvn_client/ctx.c
===================================================================
--- subversion/libsvn_client/ctx.c (revision 1918192)
+++ subversion/libsvn_client/ctx.c (working copy)
@@ -107,6 +107,10 @@ svn_client_create_context2(svn_client_ctx_t **ctx,
private_ctx->magic_null = 0;
private_ctx->magic_id = CLIENT_CTX_MAGIC;
+ private_ctx->pathrev_cache = apr_hash_make(pool);
+ private_ctx->pathrev_err_cache = apr_hash_make(pool);
+ private_ctx->pathrev_string_cache = apr_hash_make(pool);
+
public_ctx->notify_func2 = call_notify_func;
public_ctx->notify_baton2 = public_ctx;
Index: subversion/libsvn_client/info.c
===================================================================
--- subversion/libsvn_client/info.c (revision 1918192)
+++ subversion/libsvn_client/info.c (working copy)
@@ -214,7 +214,7 @@ push_dir_info(svn_ra_session_t *ra_session,
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
path = svn_relpath_join(dir, name, subpool);
- child_pathrev = svn_client__pathrev_join_relpath(pathrev, name, subpool);
+ child_pathrev = svn_client__pathrev_join_relpath(pathrev, name, ctx,
subpool);
fs_path = svn_client__pathrev_fspath(child_pathrev, subpool);
lock = svn_hash_gets(locks, fs_path);
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1918192)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -5234,9 +5234,9 @@ populate_remaining_ranges(apr_array_header_t *chil
SVN_ERR_ASSERT(child_repos_path != NULL);
child_source.loc1 = svn_client__pathrev_join_relpath(
- source->loc1, child_repos_path, iterpool);
+ source->loc1, child_repos_path, merge_b->ctx,
iterpool);
child_source.loc2 = svn_client__pathrev_join_relpath(
- source->loc2, child_repos_path, iterpool);
+ source->loc2, child_repos_path, merge_b->ctx,
iterpool);
/* ### Is the child 'ancestral' over the same revision range? It's
* not necessarily true that a child is 'ancestral' if the parent is,
* nor that it's not if the parent is not. However, here we claim
@@ -7130,6 +7130,7 @@ combine_range_with_segments(apr_array_header_t **m
const svn_merge_range_t *range,
const apr_array_header_t *segments,
const svn_client__pathrev_t *source_loc,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *pool)
{
apr_array_header_t *merge_source_ts =
@@ -7193,10 +7194,11 @@ combine_range_with_segments(apr_array_header_t **m
/* Build our merge source structure. */
loc1 = svn_client__pathrev_create_with_relpath(
source_loc->repos_root_url, source_loc->repos_uuid,
- rev1, path1, pool);
+ rev1, path1, pathrev_string_cache, pool);
loc2 = svn_client__pathrev_create_with_relpath(
source_loc->repos_root_url, source_loc->repos_uuid,
- MIN(segment->range_end, maxrev), segment->path, pool);
+ MIN(segment->range_end, maxrev), segment->path,
+ pathrev_string_cache, pool);
/* If this is subtractive, reverse the whole calculation. */
if (subtractive)
merge_source = merge_source_create(loc2, loc1, TRUE /* ancestral */,
@@ -7234,6 +7236,7 @@ normalize_merge_sources_internal(apr_array_header_
svn_revnum_t trim_revision = SVN_INVALID_REVNUM;
apr_array_header_t *segments;
int i;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
/* Initialize our return variable. */
*merge_sources_p = apr_array_make(result_pool, 1, sizeof(merge_source_t *));
@@ -7364,6 +7367,7 @@ normalize_merge_sources_internal(apr_array_header_
/* Copy the resulting merge sources into master list thereof. */
SVN_ERR(combine_range_with_segments(&merge_sources, range,
segments, source_loc,
+ priv->pathrev_string_cache,
result_pool));
apr_array_cat(*merge_sources_p, merge_sources);
}
@@ -8614,6 +8618,7 @@ record_mergeinfo_for_dir_merge(svn_mergeinfo_catal
int i;
svn_boolean_t is_rollback = (merged_range->start > merged_range->end);
svn_boolean_t operative_merge;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(merge_b->ctx);
/* Update the WC mergeinfo here to account for our new
merges, minus any unresolved conflicts and skips. */
@@ -8770,7 +8775,7 @@ record_mergeinfo_for_dir_merge(svn_mergeinfo_catal
merge_b->target->loc.repos_root_url,
merge_b->target->loc.repos_uuid,
merged_range->end, child_merge_src_fspath + 1,
- iterpool);
+ priv->pathrev_string_cache, iterpool);
/* Confirm that the naive mergeinfo we want to set on
CHILD->ABSPATH both exists and is part of
@@ -8933,6 +8938,7 @@ record_mergeinfo_for_added_subtrees(
const char *rel_added_path;
const char *added_path_mergeinfo_fspath;
svn_client__pathrev_t *added_path_pathrev;
+ svn_client__private_ctx_t *priv =
svn_client__get_private_ctx(merge_b->ctx);
SVN_ERR(svn_wc_read_kind2(&added_path_kind, merge_b->ctx->wc_ctx,
added_abspath, FALSE, FALSE, iterpool));
@@ -8974,7 +8980,8 @@ record_mergeinfo_for_added_subtrees(
merge_b->target->loc.repos_root_url,
merge_b->target->loc.repos_uuid,
MAX(merged_range->start, merged_range->end),
- added_path_mergeinfo_fspath + 1, iterpool);
+ added_path_mergeinfo_fspath + 1,
+ priv->pathrev_string_cache, iterpool);
SVN_ERR(svn_client__get_history_as_mergeinfo(
&adds_history_as_mergeinfo, NULL,
added_path_pathrev,
@@ -11272,6 +11279,7 @@ find_unmerged_mergeinfo(svn_mergeinfo_catalog_t *u
apr_hash_index_t *hi;
svn_mergeinfo_catalog_t new_catalog = apr_hash_make(result_pool);
apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
assert(session_url_is(source_ra_session, source_loc->url, scratch_pool));
assert(session_url_is(target_ra_session, target->loc.url, scratch_pool));
@@ -11297,7 +11305,7 @@ find_unmerged_mergeinfo(svn_mergeinfo_catalog_t *u
source_path = svn_relpath_join(source_repos_rel_path,
path_rel_to_session, iterpool);
source_pathrev = svn_client__pathrev_join_relpath(
- source_loc, path_rel_to_session, iterpool);
+ source_loc, path_rel_to_session, ctx, iterpool);
/* Remove any target history that is also part of the source's history,
i.e. their common ancestry. By definition this has already been
@@ -11384,7 +11392,7 @@ find_unmerged_mergeinfo(svn_mergeinfo_catalog_t *u
source_url = svn_path_url_add_component2(source_loc->url,
path_rel_to_session, iterpool);
target_pathrev = svn_client__pathrev_join_relpath(
- &target->loc, path_rel_to_session, iterpool);
+ &target->loc, path_rel_to_session, ctx, iterpool);
err = svn_client__get_history_as_mergeinfo(&target_history_as_mergeinfo,
NULL /* has_rev_zero_history
*/,
target_pathrev,
@@ -11422,7 +11430,7 @@ find_unmerged_mergeinfo(svn_mergeinfo_catalog_t *u
/* ### Why looking at SOURCE_url at TARGET_rev? */
SVN_ERR(svn_client__pathrev_create_with_session(
&pathrev, source_ra_session, target->loc.rev, source_url,
- iterpool));
+ priv->pathrev_string_cache, iterpool));
SVN_ERR(find_unmerged_mergeinfo_subroutine(
&filtered_mergeinfo, target_history_as_mergeinfo,
source_mergeinfo, pathrev,
@@ -11501,6 +11509,7 @@ calculate_left_hand_side(svn_client__pathrev_t **l
apr_hash_t *target_history_hash = apr_hash_make(scratch_pool);
svn_revnum_t youngest_merged_rev;
svn_client__pathrev_t *yc_ancestor;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
assert(session_url_is(source_ra_session, source_loc->url, scratch_pool));
assert(session_url_is(target_ra_session, target->loc.url, scratch_pool));
@@ -11536,7 +11545,8 @@ calculate_left_hand_side(svn_client__pathrev_t **l
scratch_pool, iterpool));
target_child = svn_client__pathrev_create_with_relpath(
target->loc.repos_root_url, target->loc.repos_uuid,
- target->loc.rev, repos_relpath, iterpool);
+ target->loc.rev, repos_relpath,
+ priv->pathrev_string_cache, iterpool);
SVN_ERR(svn_client__get_history_as_mergeinfo(&target_history_as_mergeinfo,
NULL /* has_rev_zero_hist
*/,
target_child,
@@ -12179,6 +12189,7 @@ typedef struct branch_history_t
static svn_client__pathrev_t *
location_on_branch_at_rev(const branch_history_t *branch_history,
svn_revnum_t rev,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
@@ -12199,7 +12210,7 @@ location_on_branch_at_rev(const branch_history_t *
return svn_client__pathrev_create_with_relpath(
branch_history->tip->repos_root_url,
branch_history->tip->repos_uuid,
- rev, fspath + 1, result_pool);
+ rev, fspath + 1, pathrev_string_cache, result_pool);
}
}
}
@@ -12275,6 +12286,7 @@ static svn_error_t *
branch_history_get_endpoints(svn_client__pathrev_t **oldest_p,
svn_client__pathrev_t **youngest_p,
const branch_history_t *branch,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
@@ -12285,10 +12297,10 @@ branch_history_get_endpoints(svn_client__pathrev_t
branch->history, scratch_pool));
if (oldest_p)
*oldest_p = location_on_branch_at_rev(
- branch, oldest_rev + 1, result_pool, scratch_pool);
+ branch, oldest_rev + 1, pathrev_string_cache, result_pool,
scratch_pool);
if (youngest_p)
*youngest_p = location_on_branch_at_rev(
- branch, youngest_rev, result_pool, scratch_pool);
+ branch, youngest_rev, pathrev_string_cache, result_pool,
scratch_pool);
return SVN_NO_ERROR;
}
@@ -12438,6 +12450,7 @@ find_last_merged_location(svn_client__pathrev_t **
target_opt_rev;
svn_revnum_t youngest_merged_rev = SVN_INVALID_REVNUM;
svn_mergeinfo_catalog_t target_mergeinfo_cat = NULL;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
/* Using a local subpool for 'target_mergeinfo_cat' can make a big
reduction in overall memory usage. */
@@ -12518,6 +12531,7 @@ find_last_merged_location(svn_client__pathrev_t **
base_rev,
scratch_pool, scratch_pool));
SVN_ERR(branch_history_get_endpoints(NULL, base_p, contiguous_source,
+ priv->pathrev_string_cache,
result_pool, scratch_pool));
}
@@ -12616,6 +12630,7 @@ find_automatic_merge(svn_client__pathrev_t **base_
apr_pool_t *scratch_pool)
{
svn_client__pathrev_t *base_on_source, *base_on_target;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
/* Get the location-history of each branch. */
s_t->source_branch.tip = s_t->source;
@@ -12634,7 +12649,7 @@ find_automatic_merge(svn_client__pathrev_t **base_
s_t->source_branch.has_r0_history,
&s_t->target->loc, s_t->target_branch.history,
s_t->target_branch.has_r0_history,
- result_pool, scratch_pool));
+ priv->pathrev_string_cache, result_pool, scratch_pool));
if (! s_t->yca)
return svn_error_createf(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
Index: subversion/libsvn_client/ra.c
===================================================================
--- subversion/libsvn_client/ra.c (revision 1918192)
+++ subversion/libsvn_client/ra.c (working copy)
@@ -502,6 +502,8 @@ svn_client__resolve_rev_and_url(svn_client__pathre
const char *url;
svn_revnum_t rev;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+
/* Default revisions: peg -> working or head; operative -> peg. */
SVN_ERR(svn_opt_resolve_revisions(&peg_rev, &start_rev,
svn_path_is_url(path_or_url),
@@ -515,7 +517,9 @@ svn_client__resolve_rev_and_url(svn_client__pathre
&start_rev, NULL, ctx, pool));
SVN_ERR(svn_client__pathrev_create_with_session(resolved_loc_p,
- ra_session, rev, url, pool));
+ ra_session, rev, url,
+ priv->pathrev_string_cache,
+ pool));
return SVN_NO_ERROR;
}
@@ -777,6 +781,8 @@ svn_client__repos_location(svn_client__pathrev_t *
const char *op_url;
svn_error_t *err;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+
SVN_ERR(svn_client__ensure_ra_session_url(&old_session_url, ra_session,
peg_loc->url, scratch_pool));
err = repos_locations(&op_url, NULL, ra_session,
@@ -788,7 +794,9 @@ svn_client__repos_location(svn_client__pathrev_t *
*op_loc_p = svn_client__pathrev_create(peg_loc->repos_root_url,
peg_loc->repos_uuid,
- op_revnum, op_url, result_pool);
+ op_revnum, op_url,
+ priv->pathrev_string_cache,
+ result_pool);
return SVN_NO_ERROR;
}
@@ -935,6 +943,7 @@ svn_client__calc_youngest_common_ancestor(svn_clie
const svn_client__pathrev_t *loc2,
apr_hash_t *history2,
svn_boolean_t has_rev_zero_history2,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
@@ -991,7 +1000,8 @@ svn_client__calc_youngest_common_ancestor(svn_clie
{
*ancestor_p = svn_client__pathrev_create_with_relpath(
loc1->repos_root_url, loc1->repos_uuid,
- yc_revision, yc_relpath, result_pool);
+ yc_revision, yc_relpath, pathrev_string_cache,
+ result_pool);
}
else
{
@@ -1000,6 +1010,77 @@ svn_client__calc_youngest_common_ancestor(svn_clie
return SVN_NO_ERROR;
}
+static svn_error_t *
+get_history_from_cache_or_as_mergeinfo(apr_hash_t **history,
+ svn_boolean_t *has_rev_zero_history,
+ const svn_client__pathrev_t *loc,
+ svn_ra_session_t *session,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+ svn_error_t *ret;
+
+ /* First look up the entry in the hash table */
+ *history = apr_hash_get(priv->pathrev_cache,
+ loc,
+ sizeof(*loc));
+ ret = apr_hash_get(priv->pathrev_err_cache,
+ loc,
+ sizeof(*loc));
+
+ if (ret != NULL)
+ {
+ /* Create an error to return */
+/* printf("Cached %s (%d) as error\n", loc->url, loc->rev); */
+ return svn_error_create(ret->apr_err, NULL, ret->message);
+ }
+
+ if (*history == NULL)
+ {
+ apr_pool_t *hash_pool = apr_hash_pool_get(priv->pathrev_cache);
+/* printf("Looking up %s (%d)\n", loc->url, loc->rev); */
+ /* We're going to cheat and use history-as-mergeinfo because it
+ saves us a bunch of annoying custom data comparisons and such. */
+ ret = svn_client__get_history_as_mergeinfo(history,
+ has_rev_zero_history,
+ loc,
+ SVN_INVALID_REVNUM,
+ SVN_INVALID_REVNUM,
+ session, ctx, hash_pool);
+ if(ret == SVN_NO_ERROR)
+ {
+ svn_client__pathrev_t *dup = svn_client__pathrev_dup(loc, hash_pool);
+ apr_hash_set(priv->pathrev_cache,
+ dup,
+ sizeof(*dup),
+ *history);
+ }
+ else
+ {
+ hash_pool = apr_hash_pool_get(priv->pathrev_err_cache);
+ svn_error_t *save_err = apr_palloc(hash_pool, sizeof(*save_err));
+ save_err->apr_err = ret->apr_err;
+ save_err->message = apr_pstrdup(hash_pool, ret->message);
+ save_err->child = NULL; /* We aren't going to rebuild an entire error
chain */
+
+ svn_client__pathrev_t *dup = svn_client__pathrev_dup(loc, hash_pool);
+ apr_hash_set(priv->pathrev_err_cache,
+ dup,
+ sizeof(*dup),
+ save_err);
+
+ return svn_error_trace(ret);
+ }
+ }
+ else
+ {
+/* printf("Cached %s (%d)\n", loc->url, loc->rev); */
+ }
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_client__get_youngest_common_ancestor(svn_client__pathrev_t **ancestor_p,
const svn_client__pathrev_t *loc1,
@@ -1014,6 +1095,8 @@ svn_client__get_youngest_common_ancestor(svn_clien
svn_boolean_t has_rev_zero_history1;
svn_boolean_t has_rev_zero_history2;
+ svn_client__private_ctx_t *priv = svn_client__get_private_ctx(ctx);
+
if (strcmp(loc1->repos_root_url, loc2->repos_root_url) != 0)
{
*ancestor_p = NULL;
@@ -1030,18 +1113,15 @@ svn_client__get_youngest_common_ancestor(svn_clien
/* We're going to cheat and use history-as-mergeinfo because it
saves us a bunch of annoying custom data comparisons and such. */
- SVN_ERR(svn_client__get_history_as_mergeinfo(&history1,
- &has_rev_zero_history1,
- loc1,
- SVN_INVALID_REVNUM,
- SVN_INVALID_REVNUM,
- session, ctx, scratch_pool));
- SVN_ERR(svn_client__get_history_as_mergeinfo(&history2,
- &has_rev_zero_history2,
- loc2,
- SVN_INVALID_REVNUM,
- SVN_INVALID_REVNUM,
- session, ctx, scratch_pool));
+ SVN_ERR(get_history_from_cache_or_as_mergeinfo(&history1,
+ &has_rev_zero_history1,
+ loc1,
+ session, ctx, scratch_pool));
+ SVN_ERR(get_history_from_cache_or_as_mergeinfo(&history2,
+ &has_rev_zero_history2,
+ loc2,
+ session, ctx, scratch_pool));
+
/* Close the ra session if we opened one. */
if (sesspool)
svn_pool_destroy(sesspool);
@@ -1051,6 +1131,7 @@ svn_client__get_youngest_common_ancestor(svn_clien
has_rev_zero_history1,
loc2, history2,
has_rev_zero_history2,
+ priv->pathrev_string_cache,
result_pool,
scratch_pool));
Index: subversion/libsvn_client/util.c
===================================================================
--- subversion/libsvn_client/util.c (revision 1918192)
+++ subversion/libsvn_client/util.c (working copy)
@@ -47,6 +47,7 @@ svn_client__pathrev_create(const char *repos_root_
const char *repos_uuid,
svn_revnum_t rev,
const char *url,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool)
{
svn_client__pathrev_t *loc = apr_palloc(result_pool, sizeof(*loc));
@@ -54,10 +55,42 @@ svn_client__pathrev_create(const char *repos_root_
SVN_ERR_ASSERT_NO_RETURN(svn_path_is_url(repos_root_url));
SVN_ERR_ASSERT_NO_RETURN(svn_path_is_url(url));
- loc->repos_root_url = apr_pstrdup(result_pool, repos_root_url);
- loc->repos_uuid = apr_pstrdup(result_pool, repos_uuid);
+ const char *istring;
+
+ apr_pool_t *hash_pool = apr_hash_pool_get(pathrev_string_cache);
+
+ istring = apr_hash_get(pathrev_string_cache, repos_root_url,
+ APR_HASH_KEY_STRING);
+ if (istring == NULL)
+ {
+ istring = apr_pstrdup(hash_pool, repos_root_url);
+ apr_hash_set(pathrev_string_cache, istring, APR_HASH_KEY_STRING,
+ istring);
+ }
+ loc->repos_root_url = istring;
+
+ istring = apr_hash_get(pathrev_string_cache, repos_uuid,
+ APR_HASH_KEY_STRING);
+ if (istring == NULL)
+ {
+ istring = apr_pstrdup(hash_pool, repos_uuid);
+ apr_hash_set(pathrev_string_cache, istring, APR_HASH_KEY_STRING,
+ istring);
+ }
+ loc->repos_uuid = istring;
+
loc->rev = rev;
- loc->url = apr_pstrdup(result_pool, url);
+
+ istring = apr_hash_get(pathrev_string_cache, url,
+ APR_HASH_KEY_STRING);
+ if (istring == NULL)
+ {
+ istring = apr_pstrdup(hash_pool, url);
+ apr_hash_set(pathrev_string_cache, istring, APR_HASH_KEY_STRING,
+ istring);
+ }
+ loc->url = istring;
+
return loc;
}
@@ -66,6 +99,7 @@ svn_client__pathrev_create_with_relpath(const char
const char *repos_uuid,
svn_revnum_t rev,
const char *relpath,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool)
{
SVN_ERR_ASSERT_NO_RETURN(svn_relpath_is_canonical(relpath));
@@ -73,7 +107,7 @@ svn_client__pathrev_create_with_relpath(const char
return svn_client__pathrev_create(
repos_root_url, repos_uuid, rev,
svn_path_url_add_component2(repos_root_url, relpath, result_pool),
- result_pool);
+ pathrev_string_cache, result_pool);
}
svn_error_t *
@@ -81,17 +115,25 @@ svn_client__pathrev_create_with_session(svn_client
svn_ra_session_t *ra_session,
svn_revnum_t rev,
const char *url,
+ apr_hash_t *pathrev_string_cache,
apr_pool_t *result_pool)
{
- svn_client__pathrev_t *pathrev = apr_palloc(result_pool, sizeof(*pathrev));
+ char const *repos_root_url;
+ char const *repos_uuid;
+ svn_client__pathrev_t *pathrev;
SVN_ERR_ASSERT(svn_path_is_url(url));
- SVN_ERR(svn_ra_get_repos_root2(ra_session, &pathrev->repos_root_url,
+ SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root_url,
result_pool));
- SVN_ERR(svn_ra_get_uuid2(ra_session, &pathrev->repos_uuid, result_pool));
- pathrev->rev = rev;
- pathrev->url = apr_pstrdup(result_pool, url);
+ SVN_ERR(svn_ra_get_uuid2(ra_session, &repos_uuid, result_pool));
+
+ pathrev = svn_client__pathrev_create(repos_root_url,
+ repos_uuid,
+ rev, url, pathrev_string_cache,
+ result_pool);
+ SVN_ERR_ASSERT(pathrev != NULL);
+
*pathrev_p = pathrev;
SVN_ERR_ASSERT(svn_uri__is_ancestor(pathrev->repos_root_url, url));
return SVN_NO_ERROR;
@@ -101,20 +143,21 @@ svn_client__pathrev_t *
svn_client__pathrev_dup(const svn_client__pathrev_t *pathrev,
apr_pool_t *result_pool)
{
- return svn_client__pathrev_create(
- pathrev->repos_root_url, pathrev->repos_uuid,
- pathrev->rev, pathrev->url, result_pool);
+ svn_client__pathrev_t *ret = apr_palloc(result_pool, sizeof(*ret));
+ memcpy( ret, pathrev, sizeof(*ret) );
+ return ret;
}
svn_client__pathrev_t *
svn_client__pathrev_join_relpath(const svn_client__pathrev_t *pathrev,
const char *relpath,
+ svn_client_ctx_t *ctx,
apr_pool_t *result_pool)
{
return svn_client__pathrev_create(
pathrev->repos_root_url, pathrev->repos_uuid, pathrev->rev,
svn_path_url_add_component2(pathrev->url, relpath, result_pool),
- result_pool);
+ svn_client__get_private_ctx(ctx)->pathrev_string_cache,
result_pool);
}
const char *
Index: subversion/svnbench/null-info-cmd.c
===================================================================
--- subversion/svnbench/null-info-cmd.c (revision 1918192)
+++ subversion/svnbench/null-info-cmd.c (working copy)
@@ -90,7 +90,7 @@ push_dir_info(svn_ra_session_t *ra_session,
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
path = svn_relpath_join(dir, name, subpool);
- child_pathrev = svn_client__pathrev_join_relpath(pathrev, name, subpool);
+ child_pathrev = svn_client__pathrev_join_relpath(pathrev, name, ctx,
subpool);
if (depth >= svn_depth_immediates
|| (depth == svn_depth_files && the_ent->kind == svn_node_file))
Index: subversion/tests/libsvn_client/client-test.c
===================================================================
--- subversion/tests/libsvn_client/client-test.c (revision 1918192)
+++ subversion/tests/libsvn_client/client-test.c (working copy)
@@ -682,6 +682,7 @@ test_youngest_common_ancestor(const svn_test_opts_
const char *repos_url;
const char *repos_uuid = "fake-uuid"; /* the functions we call don't care */
svn_client_ctx_t *ctx;
+ svn_client__private_ctx_t *priv;
svn_opt_revision_t head_rev = { svn_opt_revision_head, { 0 } };
svn_opt_revision_t zero_rev = { svn_opt_revision_number, { 0 } };
svn_client_copy_source_t source;
@@ -693,6 +694,7 @@ test_youngest_common_ancestor(const svn_test_opts_
SVN_ERR(create_greek_repos(&repos_url, "test-youngest-common-ancestor",
opts, pool));
SVN_ERR(svn_client_create_context(&ctx, pool));
+ priv = svn_client__get_private_ctx(ctx);
/* Copy a file into dir 'A', keeping its own basename. */
sources = apr_array_make(pool, 1, sizeof(svn_client_copy_source_t *));
@@ -710,9 +712,9 @@ test_youngest_common_ancestor(const svn_test_opts_
SVN_ERR(svn_client__get_youngest_common_ancestor(
&yc_ancestor,
svn_client__pathrev_create_with_relpath(
- repos_url, repos_uuid, 2, "iota", pool),
+ repos_url, repos_uuid, 2, "iota", priv->pathrev_string_cache,
pool),
svn_client__pathrev_create_with_relpath(
- repos_url, repos_uuid, 2, "A/iota", pool),
+ repos_url, repos_uuid, 2, "A/iota", priv->pathrev_string_cache,
pool),
NULL, ctx, pool, pool));
SVN_TEST_STRING_ASSERT(svn_client__pathrev_relpath(yc_ancestor, pool),
"iota");
@@ -734,9 +736,9 @@ test_youngest_common_ancestor(const svn_test_opts_
SVN_ERR(svn_client__get_youngest_common_ancestor(
&yc_ancestor,
svn_client__pathrev_create_with_relpath(
- repos_url, repos_uuid, 0, "", pool),
+ repos_url, repos_uuid, 0, "", priv->pathrev_string_cache, pool),
svn_client__pathrev_create_with_relpath(
- repos_url, repos_uuid, 3, "A/ROOT", pool),
+ repos_url, repos_uuid, 3, "A/ROOT", priv->pathrev_string_cache,
pool),
NULL, ctx, pool, pool));
SVN_TEST_STRING_ASSERT(svn_client__pathrev_relpath(yc_ancestor, pool), "");
SVN_TEST_ASSERT(yc_ancestor->rev == 0);
