You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/08/07 17:08:08 UTC

Re: svn commit: r38273 - trunk/subversion/libsvn_wc

On Tue, Jun 30, 2009 at 21:40, Hyrum K. Wright<hy...@hyrumwright.org> wrote:
>...
> +++ trunk/subversion/libsvn_wc/update_editor.c  Tue Jun 30 12:40:57 2009        (r38273)
>...
> @@ -1754,8 +1755,8 @@ already_in_a_tree_conflict(const char **
>   ancestors = apr_array_make(pool, 0, sizeof(const char *));
>
>   /* If PATH is under version control, put it on the ancestor list. */
> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, ancestor, pool));
> -  err = svn_wc__get_entry(&entry, db, local_abspath, TRUE,
> +  SVN_ERR(svn_dirent_get_absolute(&ancestor_abspath, ancestor, pool));
> +  err = svn_wc__get_entry(&entry, db, ancestor_abspath, TRUE,
>                           svn_node_unknown, FALSE, pool, pool);

If you create the iterpool sooner, then you could use it here.

>   if (err)
>     {
> @@ -1775,11 +1776,15 @@ already_in_a_tree_conflict(const char **
>
>   /* Append to the list all ancestor-dirs in the working copy.  Ignore
>      the root because it can't be tree-conflicted. */
> +  iterpool = svn_pool_create(pool);
>   while (! svn_path_is_empty(ancestor))
>     {
>       svn_boolean_t is_wc_root;
>
> -      SVN_ERR(check_wc_root(&is_wc_root, NULL, ancestor, db, pool));
> +      svn_pool_clear(iterpool);
> +      SVN_ERR(svn_dirent_get_absolute(&ancestor_abspath, ancestor, iterpool));
> +      SVN_ERR(check_wc_root(&is_wc_root, NULL, db, ancestor_abspath,
> +                            iterpool));

These loops do a lot of svn_dirent_get_absolute() calls. Instead, you
could have a running pair of ancestor/ancestor_abspath. Put the
abspath into the array, and then the last loop wouldn't need a get_abs
at all.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2381386


Re: svn commit: r38273 - trunk/subversion/libsvn_wc

Posted by "Hyrum K. Wright" <hy...@hyrumwright.org>.
On Aug 7, 2009, at 12:08 PM, Greg Stein wrote:

> On Tue, Jun 30, 2009 at 21:40, Hyrum K.  
> Wright<hy...@hyrumwright.org> wrote:
>> ...
>> +++ trunk/subversion/libsvn_wc/update_editor.c  Tue Jun 30 12:40:57  
>> 2009        (r38273)
>> ...
>> @@ -1754,8 +1755,8 @@ already_in_a_tree_conflict(const char **
>>   ancestors = apr_array_make(pool, 0, sizeof(const char *));
>>
>>   /* If PATH is under version control, put it on the ancestor list.  
>> */
>> -  SVN_ERR(svn_dirent_get_absolute(&local_abspath, ancestor, pool));
>> -  err = svn_wc__get_entry(&entry, db, local_abspath, TRUE,
>> +  SVN_ERR(svn_dirent_get_absolute(&ancestor_abspath, ancestor,  
>> pool));
>> +  err = svn_wc__get_entry(&entry, db, ancestor_abspath, TRUE,
>>                           svn_node_unknown, FALSE, pool, pool);
>
> If you create the iterpool sooner, then you could use it here.
>
>>   if (err)
>>     {
>> @@ -1775,11 +1776,15 @@ already_in_a_tree_conflict(const char **
>>
>>   /* Append to the list all ancestor-dirs in the working copy.   
>> Ignore
>>      the root because it can't be tree-conflicted. */
>> +  iterpool = svn_pool_create(pool);
>>   while (! svn_path_is_empty(ancestor))
>>     {
>>       svn_boolean_t is_wc_root;
>>
>> -      SVN_ERR(check_wc_root(&is_wc_root, NULL, ancestor, db, pool));
>> +      svn_pool_clear(iterpool);
>> +      SVN_ERR(svn_dirent_get_absolute(&ancestor_abspath, ancestor,  
>> iterpool));
>> +      SVN_ERR(check_wc_root(&is_wc_root, NULL, db, ancestor_abspath,
>> +                            iterpool));
>
> These loops do a lot of svn_dirent_get_absolute() calls. Instead, you
> could have a running pair of ancestor/ancestor_abspath. Put the
> abspath into the array, and then the last loop wouldn't need a get_abs
> at all.

Looking back at this function, I got a little carried away with  
further changes.  See r38626, which addresses the above, but also  
addresses some other issues.

-Hyrum

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2381442