Re: svn commit: r1094692 - /subversion/trunk/subversion/libsvn_repos/rev_hunt.c

["C. Michael Pilato" <cm...@collab.net>]

On 04/18/2011 02:38 PM, philip@apache.org wrote:
> Author: philip
> Date: Mon Apr 18 18:38:58 2011
> New Revision: 1094692
> 
> URL: http://svn.apache.org/viewvc?rev=1094692&view=rev
> Log:
> Make "blame -g" more efficient on the server when svn:mergeinfo is
> large.
> 
> * subversion/libsvn_repos/rev_hunt.c
>   (get_merged_mergeinfo): Use the FS path_changed API to check
>    for property changes before doing expensive svn:mergeinfo
>    manipulations, don't treat newly created paths as a merge,
>    avoid allocating some empty hashes.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/rev_hunt.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c

[...]

> @@ -1045,6 +1046,30 @@ get_merged_mergeinfo(apr_hash_t **merged
>    apr_pool_t *subpool = svn_pool_create(pool);
>    apr_hash_t *curr_mergeinfo, *prev_mergeinfo, *deleted, *changed;
>    svn_error_t *err;
> +  svn_fs_root_t *root;
> +  apr_hash_t *changed_paths;
> +  const char *path = old_path_rev->path;
> +
> +  /* Getting/parsing/diffing svn:mergeinfo is expensive, so only do it
> +     if there is a property change. */
> +  SVN_ERR(svn_fs_revision_root(&root, repos->fs, old_path_rev->revnum,
> +                               subpool));
> +  SVN_ERR(svn_fs_paths_changed2(&changed_paths, root, subpool));
> +  while (1)
> +    {
> +      svn_fs_path_change2_t *changed_path = apr_hash_get(changed_paths,
> +                                                         path,
> +                                                         APR_HASH_KEY_STRING);
> +      if (changed_path && changed_path->prop_mod)
> +        break;
> +      if (svn_dirent_is_root(path, strlen(path)))

I'd need to get my bearings before reviewing the meat of this change, but
this use of the wrong path API caught my eye.  These aren't dirents.  They
are probably fspaths.


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand

Re: svn commit: r1094692 - /subversion/trunk/subversion/libsvn_repos/rev_hunt.c

[Philip Martin <ph...@wandisco.com>]

"C. Michael Pilato" <cm...@collab.net> writes:

>> +      if (changed_path && changed_path->prop_mod)
>> +        break;
>> +      if (svn_dirent_is_root(path, strlen(path)))
>
> I'd need to get my bearings before reviewing the meat of this change, but
> this use of the wrong path API caught my eye.  These aren't dirents.  They
> are probably fspaths.

Yes, I wrote this patch in 1.6 first.  It's fixed on trunk.

-- 
Philip