You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2011/07/21 12:53:15 UTC
svn commit: r1149105 - /subversion/trunk/subversion/libsvn_client/merge.c
Author: philip
Date: Thu Jul 21 10:53:15 2011
New Revision: 1149105
URL: http://svn.apache.org/viewvc?rev=1149105&view=rev
Log:
Fix issue 3966, log_noop_revs in merge is far too slow.
* subversion/libsvn_client/merge.c:
(rangelist_merge_revision): New, specialised rangelist merge.
(log_noop_revs): Use specialised rangelist merge.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1149105&r1=1149104&r2=1149105&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Thu Jul 21 10:53:15 2011
@@ -7868,6 +7868,36 @@ typedef struct log_noop_baton_t
apr_pool_t *pool;
} log_noop_baton_t;
+/* Helper for log_noop_revs, this is not a general purpose rangelist
+ merge. Merge the single revision range REVISION-1 to REVISION into
+ RANGELIST. The existing ranges in RANGELIST must be ordered from
+ highest/youngest to lowest/oldest. */
+static svn_error_t *
+rangelist_merge_revision(apr_array_header_t *rangelist,
+ svn_revnum_t revision,
+ apr_pool_t *result_pool)
+{
+ svn_merge_range_t *new_range;
+ if (rangelist->nelts)
+ {
+ svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, rangelist->nelts - 1,
+ svn_merge_range_t *);
+ if (range->start == revision)
+ {
+ range->start = revision - 1;
+ return SVN_NO_ERROR;
+ }
+ }
+ new_range = apr_palloc(result_pool, sizeof(*new_range));
+ new_range->start = revision - 1;
+ new_range->end = revision;
+ new_range->inheritable = TRUE;
+
+ APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = new_range;
+
+ return SVN_NO_ERROR;
+}
+
/* Implements the svn_log_entry_receiver_t interface.
BATON is an log_noop_baton_t *.
@@ -7892,7 +7922,6 @@ log_noop_revs(void *baton,
svn_boolean_t log_entry_rev_required = FALSE;
apr_array_header_t *rl1;
apr_array_header_t *rl2;
- apr_array_header_t *rangelist;
/* The baton's pool is essentially an iterpool so we must clear it
* for each invocation of this function, preserving the result
@@ -7909,12 +7938,10 @@ log_noop_revs(void *baton,
revision = log_entry->revision;
- rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
- log_gap_baton->pool);
/* Unconditionally add LOG_ENTRY->REVISION to BATON->OPERATIVE_MERGES. */
- SVN_ERR(svn_rangelist_merge(&(log_gap_baton->operative_ranges),
- rangelist,
- log_gap_baton->pool));
+ SVN_ERR(rangelist_merge_revision(log_gap_baton->operative_ranges,
+ revision,
+ log_gap_baton->pool));
/* Examine each path affected by LOG_ENTRY->REVISION. If the explicit or
inherited mergeinfo for *all* of the corresponding paths under
@@ -7977,6 +8004,10 @@ log_noop_revs(void *baton,
if (paths_explicit_rangelist)
{
apr_array_header_t *intersecting_range;
+ apr_array_header_t *rangelist;
+
+ rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
+ log_gap_baton->pool);
/* If PATH inherited mergeinfo we must consider inheritance in the
event the inherited mergeinfo is actually non-inheritable. */
@@ -7995,9 +8026,9 @@ log_noop_revs(void *baton,
}
if (!log_entry_rev_required)
- SVN_ERR(svn_rangelist_merge(&(log_gap_baton->merged_ranges),
- rangelist,
- log_gap_baton->pool));
+ SVN_ERR(rangelist_merge_revision(log_gap_baton->merged_ranges,
+ revision,
+ log_gap_baton->pool));
return SVN_NO_ERROR;
}
Re: svn commit: r1149105 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Jul 21, 2011 at 6:53 AM, <ph...@apache.org> wrote:
> Author: philip
> Date: Thu Jul 21 10:53:15 2011
> New Revision: 1149105
>
> URL: http://svn.apache.org/viewvc?rev=1149105&view=rev
> Log:
> Fix issue 3966, log_noop_revs in merge is far too slow.
>
> * subversion/libsvn_client/merge.c:
> (rangelist_merge_revision): New, specialised rangelist merge.
> (log_noop_revs): Use specialised rangelist merge.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1149105&r1=1149104&r2=1149105&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Jul 21 10:53:15 2011
> @@ -7868,6 +7868,36 @@ typedef struct log_noop_baton_t
> apr_pool_t *pool;
> } log_noop_baton_t;
>
> +/* Helper for log_noop_revs, this is not a general purpose rangelist
> + merge. Merge the single revision range REVISION-1 to REVISION into
> + RANGELIST. The existing ranges in RANGELIST must be ordered from
> + highest/youngest to lowest/oldest. */
Hi Philip,
The rangelist APIs expect[1] the elements to be sorted from to
lowest/oldest to highest/youngest, so you are creating an invalid
array which the svn_rangelist_APIs won't understand correctly.
In rr1149228 I switched remove_noop_subtree_ranges's call to
svn_ra_get_log2 to get the logs from oldest to youngest, so
log_noop_revs/rangelist_merge_revision get the revs in that order, can
build a valid rangelist, and can still avoid the worst-case
performance of svn_rangelist_merge when adding a single range younger
than any in the output rangelist.
Paul
[1] svn_mergeinfo.h:
* (b) A "rangelist". An array (@c apr_array_header_t *) of non-overlapping
* merge ranges (@c svn_merge_range_t *), sorted as said by
* @c svn_sort_compare_ranges(). An empty range list is represented by
* an empty array. Unless specifically noted otherwise, all APIs require
* rangelists that describe only forward ranges, i.e. the range's start
* revision is less than its end revision.
> +static svn_error_t *
> +rangelist_merge_revision(apr_array_header_t *rangelist,
> + svn_revnum_t revision,
> + apr_pool_t *result_pool)
> +{
> + svn_merge_range_t *new_range;
> + if (rangelist->nelts)
> + {
> + svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, rangelist->nelts - 1,
> + svn_merge_range_t *);
> + if (range->start == revision)
> + {
> + range->start = revision - 1;
> + return SVN_NO_ERROR;
> + }
> + }
> + new_range = apr_palloc(result_pool, sizeof(*new_range));
> + new_range->start = revision - 1;
> + new_range->end = revision;
> + new_range->inheritable = TRUE;
> +
> + APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = new_range;
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* Implements the svn_log_entry_receiver_t interface.
>
> BATON is an log_noop_baton_t *.
> @@ -7892,7 +7922,6 @@ log_noop_revs(void *baton,
> svn_boolean_t log_entry_rev_required = FALSE;
> apr_array_header_t *rl1;
> apr_array_header_t *rl2;
> - apr_array_header_t *rangelist;
>
> /* The baton's pool is essentially an iterpool so we must clear it
> * for each invocation of this function, preserving the result
> @@ -7909,12 +7938,10 @@ log_noop_revs(void *baton,
>
> revision = log_entry->revision;
>
> - rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> - log_gap_baton->pool);
> /* Unconditionally add LOG_ENTRY->REVISION to BATON->OPERATIVE_MERGES. */
> - SVN_ERR(svn_rangelist_merge(&(log_gap_baton->operative_ranges),
> - rangelist,
> - log_gap_baton->pool));
> + SVN_ERR(rangelist_merge_revision(log_gap_baton->operative_ranges,
> + revision,
> + log_gap_baton->pool));
>
> /* Examine each path affected by LOG_ENTRY->REVISION. If the explicit or
> inherited mergeinfo for *all* of the corresponding paths under
> @@ -7977,6 +8004,10 @@ log_noop_revs(void *baton,
> if (paths_explicit_rangelist)
> {
> apr_array_header_t *intersecting_range;
> + apr_array_header_t *rangelist;
> +
> + rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> + log_gap_baton->pool);
>
> /* If PATH inherited mergeinfo we must consider inheritance in the
> event the inherited mergeinfo is actually non-inheritable. */
> @@ -7995,9 +8026,9 @@ log_noop_revs(void *baton,
> }
>
> if (!log_entry_rev_required)
> - SVN_ERR(svn_rangelist_merge(&(log_gap_baton->merged_ranges),
> - rangelist,
> - log_gap_baton->pool));
> + SVN_ERR(rangelist_merge_revision(log_gap_baton->merged_ranges,
> + revision,
> + log_gap_baton->pool));
>
> return SVN_NO_ERROR;
> }
>
>
>
Re: svn commit: r1149105 - /subversion/trunk/subversion/libsvn_client/merge.c
Posted by Paul Burba <pt...@gmail.com>.
On Thu, Jul 21, 2011 at 6:53 AM, <ph...@apache.org> wrote:
> Author: philip
> Date: Thu Jul 21 10:53:15 2011
> New Revision: 1149105
>
> URL: http://svn.apache.org/viewvc?rev=1149105&view=rev
> Log:
> Fix issue 3966, log_noop_revs in merge is far too slow.
>
> * subversion/libsvn_client/merge.c:
> (rangelist_merge_revision): New, specialised rangelist merge.
> (log_noop_revs): Use specialised rangelist merge.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1149105&r1=1149104&r2=1149105&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Jul 21 10:53:15 2011
> @@ -7868,6 +7868,36 @@ typedef struct log_noop_baton_t
> apr_pool_t *pool;
> } log_noop_baton_t;
>
> +/* Helper for log_noop_revs, this is not a general purpose rangelist
> + merge. Merge the single revision range REVISION-1 to REVISION into
> + RANGELIST. The existing ranges in RANGELIST must be ordered from
> + highest/youngest to lowest/oldest. */
Hi Philip,
The rangelist APIs expect[1] the elements to be sorted from to
lowest/oldest to highest/youngest, so you are creating an invalid
array which the svn_rangelist_APIs won't understand correctly.
In rr1149228 I switched remove_noop_subtree_ranges's call to
svn_ra_get_log2 to get the logs from oldest to youngest, so
log_noop_revs/rangelist_merge_revision get the revs in that order, can
build a valid rangelist, and can still avoid the worst-case
performance of svn_rangelist_merge when adding a single range younger
than any in the output rangelist.
Paul
[1] svn_mergeinfo.h:
* (b) A "rangelist". An array (@c apr_array_header_t *) of non-overlapping
* merge ranges (@c svn_merge_range_t *), sorted as said by
* @c svn_sort_compare_ranges(). An empty range list is represented by
* an empty array. Unless specifically noted otherwise, all APIs require
* rangelists that describe only forward ranges, i.e. the range's start
* revision is less than its end revision.
> +static svn_error_t *
> +rangelist_merge_revision(apr_array_header_t *rangelist,
> + svn_revnum_t revision,
> + apr_pool_t *result_pool)
> +{
> + svn_merge_range_t *new_range;
> + if (rangelist->nelts)
> + {
> + svn_merge_range_t *range = APR_ARRAY_IDX(rangelist, rangelist->nelts - 1,
> + svn_merge_range_t *);
> + if (range->start == revision)
> + {
> + range->start = revision - 1;
> + return SVN_NO_ERROR;
> + }
> + }
> + new_range = apr_palloc(result_pool, sizeof(*new_range));
> + new_range->start = revision - 1;
> + new_range->end = revision;
> + new_range->inheritable = TRUE;
> +
> + APR_ARRAY_PUSH(rangelist, svn_merge_range_t *) = new_range;
> +
> + return SVN_NO_ERROR;
> +}
> +
> /* Implements the svn_log_entry_receiver_t interface.
>
> BATON is an log_noop_baton_t *.
> @@ -7892,7 +7922,6 @@ log_noop_revs(void *baton,
> svn_boolean_t log_entry_rev_required = FALSE;
> apr_array_header_t *rl1;
> apr_array_header_t *rl2;
> - apr_array_header_t *rangelist;
>
> /* The baton's pool is essentially an iterpool so we must clear it
> * for each invocation of this function, preserving the result
> @@ -7909,12 +7938,10 @@ log_noop_revs(void *baton,
>
> revision = log_entry->revision;
>
> - rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> - log_gap_baton->pool);
> /* Unconditionally add LOG_ENTRY->REVISION to BATON->OPERATIVE_MERGES. */
> - SVN_ERR(svn_rangelist_merge(&(log_gap_baton->operative_ranges),
> - rangelist,
> - log_gap_baton->pool));
> + SVN_ERR(rangelist_merge_revision(log_gap_baton->operative_ranges,
> + revision,
> + log_gap_baton->pool));
>
> /* Examine each path affected by LOG_ENTRY->REVISION. If the explicit or
> inherited mergeinfo for *all* of the corresponding paths under
> @@ -7977,6 +8004,10 @@ log_noop_revs(void *baton,
> if (paths_explicit_rangelist)
> {
> apr_array_header_t *intersecting_range;
> + apr_array_header_t *rangelist;
> +
> + rangelist = svn_rangelist__initialize(revision - 1, revision, TRUE,
> + log_gap_baton->pool);
>
> /* If PATH inherited mergeinfo we must consider inheritance in the
> event the inherited mergeinfo is actually non-inheritable. */
> @@ -7995,9 +8026,9 @@ log_noop_revs(void *baton,
> }
>
> if (!log_entry_rev_required)
> - SVN_ERR(svn_rangelist_merge(&(log_gap_baton->merged_ranges),
> - rangelist,
> - log_gap_baton->pool));
> + SVN_ERR(rangelist_merge_revision(log_gap_baton->merged_ranges,
> + revision,
> + log_gap_baton->pool));
>
> return SVN_NO_ERROR;
> }
>
>
>