[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1149105 - /subversion/trunk/subversion/libsvn_client/merge.c

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 21 Jul 2011 11:38:07 -0400

On Thu, Jul 21, 2011 at 6:53 AM, <philip_at_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;
>  }
>
>
>
Received on 2011-07-21 17:38:43 CEST

This is an archived mail posted to the Subversion Dev mailing list.