On Fri, Apr 18, 2014 at 10:06 AM, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> On 14 April 2014 20:11, Ivan Zhakov <ivan_at_visualsvn.com> wrote:
> > On 13 April 2014 08:40, <stefan2_at_apache.org> wrote:
> >> Author: stefan2
> >> Date: Sun Apr 13 04:40:40 2014
> >> New Revision: 1586922
> >>
> >> URL: http://svn.apache.org/r1586922
> >> Log:
> >> Speed up file / stream comparison, i.e. minimize the processing overhead
> >> for finding the first mismatch.
> >>
> >> The approach is two-sided. Instead of fetching SVN__STREAM_CHUNK_SIZE
> >> from all sources before comparing data, we start with a much lower 4kB
> >> and increase the chunk size until we reach SVN__STREAM_CHUNK_SIZE while
> >> making sure that all reads are naturally aligned. So, we quickly find
> >> mismatches near the beginning of the file.
> >>
> >> On the other end side, we bump the SVN__STREAM_CHUNK_SIZE to 64kB which
> >> gives better throughput for longer distances to the first mismatch -
> >> without causing ill effects in APR's memory management.
> >>
> >> * subversion/include/svn_types.h
> >> (SVN__STREAM_CHUNK_SIZE): Bump to 64k and add some documentation on
> >> the general restrictions for future changes.
> >>
> > Hi Stefan,
> >
> > You effectively reverted my recent fix (r1581296) for high memory
> > usage with many repositories open at the same time (about 500k per
> > repository. Also please consider r857671 and discussion before that
> > commit:
> > http://svn.haxx.se/dev/archive-2004-11/0123.shtml
> >
> > So please revert.
> >
> Sorry, may be I was not clear enough, but I was vetoing this change.
> Given no reply, I'm going to revert it.
>
[Been AFK for the last 3 days]
Don't revert; your veto is based on faulty measurements & assumptions.
Here the shortlist:
* SVN__STREAM_CHUNK_SIZE is only to be used with files and
streams (duh!) - not with data structures that persist in RAM.
If your use of that constant were to actually effect the size of a
repo instance, that use would be faulty.
* parse_context_t is only instantiated in one place,
svn_config__parse_stream
and will be destroyed at the end of that function. So, there is no
persistent memory use from this one.
* According to *my* measurements, a FSFS instance takes about
46kB just after fs_open() and under 64 bit Unix. Of those 46k,
64k were used - according to you - for the stream buffer alone ...
So, I doubt you actually verified by how much the marginal (as in
costs per new instance in the long run) memory consumption went
down due to your initial patch.
In the meanwhile, I'm looking into ways to reduce FSFS instance
size to something like 10k. There are several places where we
could be tighter with our pool usage.
-- Stefan^2.
Received on 2014-04-18 13:46:54 CEST