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

Re: [PATCH] Fix memory leak like situation when parsing a svn_config_t object repeatedly

From: Bernd Rinn <bernd_at_sdf.lonestar.org>
Date: 2005-07-29 01:57:38 CEST

Julian Foad wrote:
> Bernd Rinn wrote:
>
>> Greg Hudson wrote:
>>
>>> Committed with minor adjustments in r15436 and r15437.
>>>
>>> While reviewing the patch, it occurred to me that the enumeration
>>> callbacks should also take a pool parameter, which would be an iteration
>>> subpool of the pool passed to the enumeration functions.
>
> [...]
>
>> I agree. Please find attached my suggestion of a patch.
>>
>> The callback function then needs to distinguish whether a particular
>> allocation has to survive the complete enumeration or only one iteration
>> and choose the appropriate pool.
>>
>> Since creating a new pool adds overhead, I feel that the caller of
>> svn_config_enumerate2() & co. should have a choice on whether an
>> iteration pool is to be created or not. That's controlled by the value
>> of the argument create_iteration_pool.
>
>
> I've not looked at this at all but I think that's a horrible idea
> (sorry!). The execution-time overhead of creating a pool is small; the
> interface-complexity overhead of an argument to control it is worse. I
> can investigate in more detail tomorrow if necessary; I just wanted to
> mention it straight away.

Well, actually I measured the execution time of the function
svn_repos_authz_check_access() (which is one of the more performance
critical use case for those *_enumerate functions) before choosing to
add the argument that controls pool creation. I wanted to get a rough
idea about how relevant the argument would be. Of course I can't claim
that the results are "typical" (there are too many parameter that can be
adjusted to be sure that a result is typical) but I can say that I
didn't do anything special to make pool creation look bad.

The result is that pool creation increases execution times for a
recursive read access check by 45%-100%. I am sure one can find other
cases with different numbers but the precise numbers are not my point.
The question seems to be: how big a performance penalty is considered to
be acceptable in order to keep one parameter out of the interface? Hard
to say. 10-15% performance? Probably yes. 50-100%? More difficult.

--
Bernd Rinn
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Jul 29 01:58:38 2005

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