[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1588651 - in /subversion/trunk/subversion/libsvn_fs_fs: caching.c fs.c fs_fs.h

From: Branko Čibej <brane_at_wandisco.com>
Date: Sat, 19 Apr 2014 17:53:16 +0200

On 19.04.2014 17:35, Ivan Zhakov wrote:
> On 19 April 2014 19:25, Branko Čibej <brane_at_wandisco.com> wrote:
>> On 19.04.2014 17:12, Ivan Zhakov wrote:
>>
>> On 19 April 2014 16:44, <stefan2_at_apache.org> wrote:
>>
>> Author: stefan2
>> Date: Sat Apr 19 12:44:07 2014
>> New Revision: 1588651
>>
>> URL: http://svn.apache.org/r1588651
>> Log:
>> Reduce the size of an FSFS instance by using temporary pools
>> for temp. data during fs_open().
>>
>> * subversion/libsvn_fs_fs/fs_fs.h
>> (svn_fs_fs__initialize_caches): Document that the POOL shall be
>> used for temporaries only.
>>
>> * subversion/libsvn_fs_fs/caching.c
>> (svn_fs_fs__initialize_caches): Fix the only place where we used
>> POOL instead of FS->POOL for
>> something with svn_fs_t lifetime.
>>
>> * subversion/libsvn_fs_fs/fs.c
>> (fs_serialized_init): Document POOL usage that we will now rely on.
>> (fs_open): Use a SUBPOOL for anything temporary during FS init.
>>
>> Just creating subpool for temporary allocation violates pool design
>> imho. Proper fix would be add scratch pool parameter to
>> svn_fs_open()/svn_fs_create().
>>
>>
> Hi Branko,
>
>> Doesn't "violate" it, as such ... there's a balance to be maintained here.
> I agree about balance, but general idea of pool design that caller
> controls memory usage of callee.
>
>> Adding a scratch-pool parameter means revising the API and propagating the
>> revision up the call chain to all the places where a meaningful scratch pool
>> is already available. Sure it's better to do do that in the long run; but
>> does the change warrant the related code churn now? Consider that there's a
>> vtable involved here, too.
>>
> I still prefer adding scratch_pool: users interested of improved
> memory usage (like mod_dav_svn) can use new API. Btw memory usage may
> increase in some cases with subpools: every pool consumes 8k of
> memory, subpools are not cleared on error in general. So we got 8k
> memory wasted on error cases.

I think we're in violent agreement :) except on the minor point about
optimizing the error case.

-- Brane

-- 
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane_at_wandisco.com
Received on 2014-04-19 17:53:43 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.