Re: svn commit: r1481594 - /subversion/trunk/subversion/libsvn_repos/log.c

[Daniel Shahaf]

stef...@apache.org wrote on Sun, May 12, 2013 at 16:17:50 -0000:
> Author: stefan2
> Date: Sun May 12 16:17:50 2013
> New Revision: 1481594
> 
> URL: http://svn.apache.org/r1481594
> Log:
> Significantly reduce I/O during 'svn log -v' and 'svn log' with authz.
> There is no need to read the copy-from info from the repository
> if it is already available in the changed path list.
> 
> * subversion/libsvn_repos/log.c
>   (detect_changed): use the copyfrom info of the change, if available
> 
> Modified:
>     subversion/trunk/subversion/libsvn_repos/log.c
> 
> Modified: subversion/trunk/subversion/libsvn_repos/log.c
> URL: 
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/log.c?rev=1481594&r1=1481593&r2=1481594&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_repos/log.c (original)
> +++ subversion/trunk/subversion/libsvn_repos/log.c Sun May 12 16:17:50 2013
> @@ -311,11 +311,15 @@ detect_changed(apr_hash_t **changed,
>  
>        if ((action == 'A') || (action == 'R'))
>          {
> -          const char *copyfrom_path;
> -          svn_revnum_t copyfrom_rev;
> +          const char *copyfrom_path = change->copyfrom_path;
> +          svn_revnum_t copyfrom_rev = change->copyfrom_rev;
>  
> -          SVN_ERR(svn_fs_copied_from(&copyfrom_rev, &copyfrom_path,
> -                                     root, path, subpool));
> +          /* the following is a potentially expensive operation since on FSFS
> +             we will follow the DAG from ROOT to PATH and that requires
> +             actually reading the directories along the way. */
> +          if (!change->copyfrom_known)
> +            SVN_ERR(svn_fs_copied_from(&copyfrom_rev, &copyfrom_path,
> +                                      root, path, subpool));
>  

Elsewhere I've seen Philip do:

          if (!change->copyfrom_known)
            SVN_ERR(svn_fs_copied_from(&change->copyfrom_rev, 
&change->copyfrom_path,
                                       root, path, subpool));
          copyfrom_rev = change->copyfrom_rev;
          copyfrom_path = change->copyfrom_path;

That seems to make sense here, since 'change' will remain available to
the caller after this function returns.

Daniel
 
>            if (copyfrom_path && SVN_IS_VALID_REVNUM(copyfrom_rev))
>              {
> 
>