On Sun, May 12, 2013 at 8:20 PM, Daniel Shahaf <danie...@elego.de> wrote:
> 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(©from_rev, ©from_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(©from_rev, ©from_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. > I agree with the general idea but in this case, the changes won't be used anywhere else in this function or the caller. Also, there is a pool lifetime issue. -- Stefan^2. -- *Join one of our free daily demo sessions on* *Scaling Subversion for the Enterprise <http://www.wandisco.com/training/webinars>* * *
