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

On 16 February 2016 at 00:47, <stefan2_at_apache.org> wrote:
> Author: stefan2
> Date: Mon Feb 15 21:47:00 2016
> New Revision: 1730617
>
> URL: http://svn.apache.org/viewvc?rev=1730617&view=rev
> Log:
> Continue work on the svn_repos_get_logs4 to svn_repos_get_logs5 migration:
> Switch the last svn_fs_paths_changed2 call to svn_fs_paths_changed3.
>
> * subversion/libsvn_repos/log.c
> (fs_mergeinfo_changed): No longer fetch the whole changes list. However,
> we need to iterate twice for best total performance
> and we need to minimize FS iterator lifetimes.
>

It seems that I would be -1 against this particular change. In the
current implementation the svn_fs_paths_changed3() is called twice
that in the worst case will lead to *double read from disk*.

As far as I understand you're relying to the fact that the second call
will hit the FSFS/FSX cache. But there will be a significant
performance degradation comparing to the 1.9 implementation in the
case of cache miss.

As we are adding more and more of such code, more and more users
become faced with the significant performance regression (see [1] and
other cases).

Do you intend to resolve this problem in the future commits? I have
some obvious solutions in mind, but maybe you also know something
about this.

[1] http://svn.haxx.se/users/archive-2015-12/0060.shtml

-- 
Ivan Zhakov