You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/07/22 12:13:49 UTC

svn commit: r1149543 - /subversion/trunk/subversion/libsvn_client/mergeinfo.c

Author: stsp
Date: Fri Jul 22 10:13:49 2011
New Revision: 1149543

URL: http://svn.apache.org/viewvc?rev=1149543&view=rev
Log:
Convert libsvn_client/mergeinfo.c over to svn_rangelist_merge2().

* subversion/libsvn_client/mergeinfo.c
  (svn_client_mergeinfo_log): Use svn_rangelist_merge2() instead of
    svn_rangelist_merge(). Create and use iterpools as the scratch_pool for
    svn_rangelist_merge2() where appropriate.

Modified:
    subversion/trunk/subversion/libsvn_client/mergeinfo.c

Modified: subversion/trunk/subversion/libsvn_client/mergeinfo.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/mergeinfo.c?rev=1149543&r1=1149542&r2=1149543&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/mergeinfo.c (original)
+++ subversion/trunk/subversion/libsvn_client/mergeinfo.c Fri Jul 22 10:13:49 2011
@@ -1847,16 +1847,20 @@ svn_client_mergeinfo_log(svn_boolean_t f
          subtrees. */
       if (apr_hash_count(merged_noninheritable))
         {
+          apr_pool_t *iterpool2 = svn_pool_create(iterpool);
+
           for (hi = apr_hash_first(iterpool, merged_noninheritable);
                hi;
                hi = apr_hash_next(hi))
             {
               apr_array_header_t *list = svn__apr_hash_index_val(hi);
-              SVN_ERR(svn_rangelist_merge(
-                &master_noninheritable_rangelist,
+              svn_pool_clear(iterpool2);
+              SVN_ERR(svn_rangelist_merge2(
+                master_noninheritable_rangelist,
                 svn_rangelist_dup(list, scratch_pool),
-                scratch_pool));
+                scratch_pool, iterpool2));
             }
+          svn_pool_destroy(iterpool2);
         }
 
       /* Find the intersection of the inheritable part of TGT_MERGEINFO
@@ -1874,6 +1878,7 @@ svn_client_mergeinfo_log(svn_boolean_t f
              to SUBTREE_PATH. */
           apr_array_header_t *subtree_merged_rangelist =
             apr_array_make(scratch_pool, 1, sizeof(svn_merge_range_t *));
+          apr_pool_t *iterpool2 = svn_pool_create(iterpool);
 
           for (hi = apr_hash_first(iterpool, merged);
                hi;
@@ -1881,15 +1886,17 @@ svn_client_mergeinfo_log(svn_boolean_t f
             {
               apr_array_header_t *list = svn__apr_hash_index_val(hi);
 
-              SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
-                                          svn_rangelist_dup(list,
-                                                            scratch_pool),
-                                          scratch_pool));
-              SVN_ERR(svn_rangelist_merge(&subtree_merged_rangelist,
-                                          svn_rangelist_dup(list,
+              svn_pool_clear(iterpool2);
+              SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
+                                           svn_rangelist_dup(list,
+                                                             scratch_pool),
+                                           scratch_pool, iterpool2));
+              SVN_ERR(svn_rangelist_merge2(subtree_merged_rangelist,
+                                           svn_rangelist_dup(list,
                                                             scratch_pool),
-                                          scratch_pool));
+                                           scratch_pool, iterpool2));
             }
+          svn_pool_destroy(iterpool2);
 
           apr_hash_set(inheritable_subtree_merges,
                        apr_pstrdup(scratch_pool, subtree_path),
@@ -1937,9 +1944,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
           if (deleted_rangelist->nelts)
             {
               svn_rangelist__set_inheritance(deleted_rangelist, FALSE);
-              SVN_ERR(svn_rangelist_merge(&master_noninheritable_rangelist,
-                                          deleted_rangelist,
-                                          scratch_pool));
+              SVN_ERR(svn_rangelist_merge2(master_noninheritable_rangelist,
+                                           deleted_rangelist,
+                                           scratch_pool, iterpool));
               SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
                                            deleted_rangelist,
                                            master_inheritable_rangelist,
@@ -1952,9 +1959,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
   if (finding_merged)
     {
       /* Roll all the merged revisions into one rangelist. */
-      SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
-                                  master_noninheritable_rangelist,
-                                  scratch_pool));
+      SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
+                                   master_noninheritable_rangelist,
+                                   scratch_pool, scratch_pool));
 
     }
   else
@@ -1970,9 +1977,10 @@ svn_client_mergeinfo_log(svn_boolean_t f
           apr_array_header_t *subtree_merged_rangelist =
             svn__apr_hash_index_val(hi);
 
-          SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
-                                      subtree_merged_rangelist,
-                                      iterpool));
+          svn_pool_clear(iterpool);
+          SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
+                                       subtree_merged_rangelist,
+                                       scratch_pool, iterpool));
         }
 
       /* From what might be eligible subtract what we know is partially merged
@@ -1981,9 +1989,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
                                    master_noninheritable_rangelist,
                                    source_master_rangelist,
                                    FALSE, scratch_pool));
-      SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
-                                  master_noninheritable_rangelist,
-                                  scratch_pool));
+      SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
+                                   master_noninheritable_rangelist,
+                                   scratch_pool, scratch_pool));
       SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
                                    master_inheritable_rangelist,
                                    source_master_rangelist,



Re: svn commit: r1149543 - /subversion/trunk/subversion/libsvn_client/mergeinfo.c

Posted by Stefan Sperling <st...@apache.org>.
On Fri, Jul 22, 2011 at 12:29:33PM +0200, Bert Huijben wrote:
> > @@ -1952,9 +1959,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
> >    if (finding_merged)
> >      {
> >        /* Roll all the merged revisions into one rangelist. */
> > -      SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
> > -                                  master_noninheritable_rangelist,
> > -                                  scratch_pool));
> > +      SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
> > +                                   master_noninheritable_rangelist,
> > +                                   scratch_pool, scratch_pool));
> 
> You can use iterpool as scratch_pool here.
> > 
> >      }
> >    else
> > @@ -1970,9 +1977,10 @@ svn_client_mergeinfo_log(svn_boolean_t f
> >            apr_array_header_t *subtree_merged_rangelist =
> >              svn__apr_hash_index_val(hi);
> > 
> > -          SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> > -                                      subtree_merged_rangelist,
> > -                                      iterpool));
> > +          svn_pool_clear(iterpool);
> > +          SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> > +                                       subtree_merged_rangelist,
> > +                                       scratch_pool, iterpool));
> >          }
> > 
> >        /* From what might be eligible subtract what we know is partially merged
> > @@ -1981,9 +1989,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
> >                                     master_noninheritable_rangelist,
> >                                     source_master_rangelist,
> >                                     FALSE, scratch_pool));
> > -      SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> > -                                  master_noninheritable_rangelist,
> > -                                  scratch_pool));
> > +      SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> > +                                   master_noninheritable_rangelist,
> > +                                   scratch_pool, scratch_pool));
> >        SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
> >                                     master_inheritable_rangelist,
> >                                     source_master_rangelist,
> > 
> 
> (same) You can use iterpool for the scratch pool here, as that pool hasn't been destroyed at this scope.

I thought about these two points while I was changing the code.

This function is very long. It should really be split up into
several pieces. The more complex the dependencies between variables
the harder it will be to untangle it. That's why I chose not to reuse
the iterpool outside of loops. Doing so won't save that much memory anyway.
The main point of this commit is to make the number of new allocations
independent of the number of loop iterations.

> And at the very bottom of this function we run a log operation where we could also pass iterpool as a scratch pool if we extend the scope a bit.

I'd say let's split the function up first.
It's already complex enough as it is.

RE: svn commit: r1149543 - /subversion/trunk/subversion/libsvn_client/mergeinfo.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: vrijdag 22 juli 2011 12:14
> To: commits@subversion.apache.org
> Subject: svn commit: r1149543 -
> /subversion/trunk/subversion/libsvn_client/mergeinfo.c
> 
> Author: stsp
> Date: Fri Jul 22 10:13:49 2011
> New Revision: 1149543
> 
> URL: http://svn.apache.org/viewvc?rev=1149543&view=rev
> Log:
> Convert libsvn_client/mergeinfo.c over to svn_rangelist_merge2().
> 
> * subversion/libsvn_client/mergeinfo.c
>   (svn_client_mergeinfo_log): Use svn_rangelist_merge2() instead of
>     svn_rangelist_merge(). Create and use iterpools as the scratch_pool for
>     svn_rangelist_merge2() where appropriate.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_client/mergeinfo.c
> 
> Modified: subversion/trunk/subversion/libsvn_client/mergeinfo.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/m
> ergeinfo.c?rev=1149543&r1=1149542&r2=1149543&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_client/mergeinfo.c (original)
> +++ subversion/trunk/subversion/libsvn_client/mergeinfo.c Fri Jul 22
> 10:13:49 2011
> @@ -1847,16 +1847,20 @@ svn_client_mergeinfo_log(svn_boolean_t f
>           subtrees. */
>        if (apr_hash_count(merged_noninheritable))
>          {
> +          apr_pool_t *iterpool2 = svn_pool_create(iterpool);
> +
>            for (hi = apr_hash_first(iterpool, merged_noninheritable);
>                 hi;
>                 hi = apr_hash_next(hi))
>              {
>                apr_array_header_t *list = svn__apr_hash_index_val(hi);
> -              SVN_ERR(svn_rangelist_merge(
> -                &master_noninheritable_rangelist,
> +              svn_pool_clear(iterpool2);
> +              SVN_ERR(svn_rangelist_merge2(
> +                master_noninheritable_rangelist,
>                  svn_rangelist_dup(list, scratch_pool),
> -                scratch_pool));
> +                scratch_pool, iterpool2));
>              }
> +          svn_pool_destroy(iterpool2);
>          }
> 
>        /* Find the intersection of the inheritable part of TGT_MERGEINFO
> @@ -1874,6 +1878,7 @@ svn_client_mergeinfo_log(svn_boolean_t f
>               to SUBTREE_PATH. */
>            apr_array_header_t *subtree_merged_rangelist =
>              apr_array_make(scratch_pool, 1, sizeof(svn_merge_range_t *));
> +          apr_pool_t *iterpool2 = svn_pool_create(iterpool);
> 
>            for (hi = apr_hash_first(iterpool, merged);
>                 hi;
> @@ -1881,15 +1886,17 @@ svn_client_mergeinfo_log(svn_boolean_t f
>              {
>                apr_array_header_t *list = svn__apr_hash_index_val(hi);
> 
> -              SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
> -                                          svn_rangelist_dup(list,
> -                                                            scratch_pool),
> -                                          scratch_pool));
> -              SVN_ERR(svn_rangelist_merge(&subtree_merged_rangelist,
> -                                          svn_rangelist_dup(list,
> +              svn_pool_clear(iterpool2);
> +              SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
> +                                           svn_rangelist_dup(list,
> +                                                             scratch_pool),
> +                                           scratch_pool, iterpool2));
> +              SVN_ERR(svn_rangelist_merge2(subtree_merged_rangelist,
> +                                           svn_rangelist_dup(list,
>                                                              scratch_pool),
> -                                          scratch_pool));
> +                                           scratch_pool, iterpool2));
>              }
> +          svn_pool_destroy(iterpool2);
> 
>            apr_hash_set(inheritable_subtree_merges,
>                         apr_pstrdup(scratch_pool, subtree_path),
> @@ -1937,9 +1944,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
>            if (deleted_rangelist->nelts)
>              {
>                svn_rangelist__set_inheritance(deleted_rangelist, FALSE);
> -              SVN_ERR(svn_rangelist_merge(&master_noninheritable_rangelist,
> -                                          deleted_rangelist,
> -                                          scratch_pool));
> +              SVN_ERR(svn_rangelist_merge2(master_noninheritable_rangelist,
> +                                           deleted_rangelist,
> +                                           scratch_pool, iterpool));
>                SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
>                                             deleted_rangelist,
>                                             master_inheritable_rangelist,
> @@ -1952,9 +1959,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
>    if (finding_merged)
>      {
>        /* Roll all the merged revisions into one rangelist. */
> -      SVN_ERR(svn_rangelist_merge(&master_inheritable_rangelist,
> -                                  master_noninheritable_rangelist,
> -                                  scratch_pool));
> +      SVN_ERR(svn_rangelist_merge2(master_inheritable_rangelist,
> +                                   master_noninheritable_rangelist,
> +                                   scratch_pool, scratch_pool));

You can use iterpool as scratch_pool here.
> 
>      }
>    else
> @@ -1970,9 +1977,10 @@ svn_client_mergeinfo_log(svn_boolean_t f
>            apr_array_header_t *subtree_merged_rangelist =
>              svn__apr_hash_index_val(hi);
> 
> -          SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> -                                      subtree_merged_rangelist,
> -                                      iterpool));
> +          svn_pool_clear(iterpool);
> +          SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> +                                       subtree_merged_rangelist,
> +                                       scratch_pool, iterpool));
>          }
> 
>        /* From what might be eligible subtract what we know is partially merged
> @@ -1981,9 +1989,9 @@ svn_client_mergeinfo_log(svn_boolean_t f
>                                     master_noninheritable_rangelist,
>                                     source_master_rangelist,
>                                     FALSE, scratch_pool));
> -      SVN_ERR(svn_rangelist_merge(&source_master_rangelist,
> -                                  master_noninheritable_rangelist,
> -                                  scratch_pool));
> +      SVN_ERR(svn_rangelist_merge2(source_master_rangelist,
> +                                   master_noninheritable_rangelist,
> +                                   scratch_pool, scratch_pool));
>        SVN_ERR(svn_rangelist_remove(&master_inheritable_rangelist,
>                                     master_inheritable_rangelist,
>                                     source_master_rangelist,
> 

(same) You can use iterpool for the scratch pool here, as that pool hasn't been destroyed at this scope.


And at the very bottom of this function we run a log operation where we could also pass iterpool as a scratch pool if we extend the scope a bit.

	Bert