You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/05/26 00:20:25 UTC
svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Author: stefan2
Date: Wed May 25 22:20:25 2011
New Revision: 1127709
URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
Log:
Fix a pool usage issue: svn_cache__get_partial() may be called many
times in a row. Thus, the internal pool used to construct keys should
be cleared in this function as well from time to time.
* subversion/libsvn_subr/cache-membuffer.c
(svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
Modified:
subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
@@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
DEBUG_CACHE_MEMBUFFER_INIT_TAG
+ if (++cache->alloc_counter > ALLOCATIONS_PER_POOL_CLEAR)
+ {
+ apr_pool_clear(cache->pool);
+ cache->alloc_counter = 0;
+ }
+
combine_key(cache->prefix,
sizeof(cache->prefix),
key,
Re: svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <da...@apache.org>.
Stefan Fuhrmann wrote on Tue, Jul 05, 2011 at 00:02:27 +0200:
> On 27.06.2011 13:54, Daniel Shahaf wrote:
> >Stefan Fuhrmann wrote on Sun, Jun 26, 2011 at 23:00:24 +0200:
> >>On 11.06.2011 14:53, Daniel Shahaf wrote:
> >>>Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
> >>>>On 10.06.2011 22:28, Daniel Shahaf wrote:
> >>>>>stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> >>>>>>Author: stefan2
> >>>>>>Date: Wed May 25 22:20:25 2011
> >>>>>>New Revision: 1127709
> >>>>>>
> >>>>>>URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> >>>>>>Log:
> >>>>>>Fix a pool usage issue: svn_cache__get_partial() may be called many
> >>>>>>times in a row. Thus, the internal pool used to construct keys should
> >>>>>>be cleared in this function as well from time to time.
> >>>>>>
> >>>>>>* subversion/libsvn_subr/cache-membuffer.c
> >>>>>> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
> >>>>>>
> >>>>>>Modified:
> >>>>>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>>>>>
> >>>>>>Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>>>>>URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> >>>>>>==============================================================================
> >>>>>>--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> >>>>>>+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> >>>>>>@@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
> >>>>>>
> >>>>>> DEBUG_CACHE_MEMBUFFER_INIT_TAG
> >>>>>>
> >>>>>>+ if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
> >>>>>>+ {
> >>>>>>+ apr_pool_clear(cache->pool);
> >>>>>>+ cache->alloc_counter = 0;
> >>>>>>+ }
> >>>>>>+
> >>>>>Does this need to be guarded by a cache lock?
> >>>>>
> >>>>No. This happens in the outer / front-end code
> >>>>that merely adds a key prefix (combine_key below)
> >>>>before calling the shared cache object.
> >>>>
> >>>>All front-end operations assume single-threaded
> >>>>access, which should be o.k. for fs_t-local objects.
> >>>Okay, if that function is guaranteed (perhaps by API contract) not to
> >>>run concurrently to any other `front end' function (and in particular to
> >>>itself), then my concerns are resolved.
> >>>
> >>>(I was worried about accessing cache->pool and cache->alloc_counter from
> >>>multiple threads concurrently --- writer-writer or writer-reader ---
> >>>which might to undefined behaviour.)
> >>>
> >>>What guarantees the single-threaded access? I don't see it documented
> >>>in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
> >>>parameters) and the code doesn't take a lock at at that point either.
> >>>
> >>I implemented optional thread-safety some days ago but it
> >>turned out to require / suggest a new API for svn_mutex_t
> >>structures (handles NULL, APR w/o threading etc.)
> >>Otherwise, there would be even more duplicated mutex
> >>handling code.
> >>
> >>However, to keep API churn low for 1.7 stabilization, I like
> >>to suggest the following: simply adding a thread_safe
> >>parameter to the membuffer cache constructor API and
> >>returning "not supported / implemented" if set to TRUE.
> >>
> >In other words, you're suggesting to have *all* membuffer caches declare
> >themselves as non-thread-safe in 1.7?
> Since this is a completely private API, I decided to not
> change the definition for 1.7. Currently, all these "frontend"
> objects (forwarding to the already thread-safe singleton)
> are used from a single thread.
>
> We just apply the same guarantees that we already gave
> to the inprocess_cache in 1.6. The latter provides optional
> synchronization but that is never activated.
Are you saying "the code is safe because svnserve and mod_dav won't
access a given svn_fs_t object (and, by extension, the caches therein)
from more than one thread"?
If yes, that's IMO irrelevant since the svn_fs API may allow that even
if our code doesn't use that.
If not, perhaps it's best that I catch you on IRC to hash this point
out.
Daniel
(who recalls seeing other new code in FSFS that also optimizes for the
"one thread" case)
> >Assuming the cache users are fine with that, it solves the issue
> >I raised, and all these are private API's, so +1.
> >
> >
> >Also: creating the membuffer singleton respects the public API's config
> >singleton's SINGLE_THREADED parameter... so we'd have to somehow
> >"ignore" that parameter (and rely on FSFS-level mutexes?). Hmm.
> No, the singleton already uses a thread-safe API.
> This is not going to change soon.
>
> -- Stefan^2.
Re: svn commit: r1127709 - /subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 27.06.2011 13:54, Daniel Shahaf wrote:
> Stefan Fuhrmann wrote on Sun, Jun 26, 2011 at 23:00:24 +0200:
>> On 11.06.2011 14:53, Daniel Shahaf wrote:
>>> Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
>>>> On 10.06.2011 22:28, Daniel Shahaf wrote:
>>>>> stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
>>>>>> Author: stefan2
>>>>>> Date: Wed May 25 22:20:25 2011
>>>>>> New Revision: 1127709
>>>>>>
>>>>>> URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
>>>>>> Log:
>>>>>> Fix a pool usage issue: svn_cache__get_partial() may be called many
>>>>>> times in a row. Thus, the internal pool used to construct keys should
>>>>>> be cleared in this function as well from time to time.
>>>>>>
>>>>>> * subversion/libsvn_subr/cache-membuffer.c
>>>>>> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
>>>>>>
>>>>>> Modified:
>>>>>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>>>>>
>>>>>> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
>>>>>> ==============================================================================
>>>>>> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
>>>>>> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
>>>>>> @@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
>>>>>>
>>>>>> DEBUG_CACHE_MEMBUFFER_INIT_TAG
>>>>>>
>>>>>> + if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
>>>>>> + {
>>>>>> + apr_pool_clear(cache->pool);
>>>>>> + cache->alloc_counter = 0;
>>>>>> + }
>>>>>> +
>>>>> Does this need to be guarded by a cache lock?
>>>>>
>>>> No. This happens in the outer / front-end code
>>>> that merely adds a key prefix (combine_key below)
>>>> before calling the shared cache object.
>>>>
>>>> All front-end operations assume single-threaded
>>>> access, which should be o.k. for fs_t-local objects.
>>> Okay, if that function is guaranteed (perhaps by API contract) not to
>>> run concurrently to any other `front end' function (and in particular to
>>> itself), then my concerns are resolved.
>>>
>>> (I was worried about accessing cache->pool and cache->alloc_counter from
>>> multiple threads concurrently --- writer-writer or writer-reader ---
>>> which might to undefined behaviour.)
>>>
>>> What guarantees the single-threaded access? I don't see it documented
>>> in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
>>> parameters) and the code doesn't take a lock at at that point either.
>>>
>> I implemented optional thread-safety some days ago but it
>> turned out to require / suggest a new API for svn_mutex_t
>> structures (handles NULL, APR w/o threading etc.)
>> Otherwise, there would be even more duplicated mutex
>> handling code.
>>
>> However, to keep API churn low for 1.7 stabilization, I like
>> to suggest the following: simply adding a thread_safe
>> parameter to the membuffer cache constructor API and
>> returning "not supported / implemented" if set to TRUE.
>>
> In other words, you're suggesting to have *all* membuffer caches declare
> themselves as non-thread-safe in 1.7?
Since this is a completely private API, I decided to not
change the definition for 1.7. Currently, all these "frontend"
objects (forwarding to the already thread-safe singleton)
are used from a single thread.
We just apply the same guarantees that we already gave
to the inprocess_cache in 1.6. The latter provides optional
synchronization but that is never activated.
> Assuming the cache users are fine with that, it solves the issue
> I raised, and all these are private API's, so +1.
>
>
> Also: creating the membuffer singleton respects the public API's config
> singleton's SINGLE_THREADED parameter... so we'd have to somehow
> "ignore" that parameter (and rely on FSFS-level mutexes?). Hmm.
No, the singleton already uses a thread-safe API.
This is not going to change soon.
-- Stefan^2.
Re: svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sun, Jun 26, 2011 at 23:00:24 +0200:
> On 11.06.2011 14:53, Daniel Shahaf wrote:
> >Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
> >>On 10.06.2011 22:28, Daniel Shahaf wrote:
> >>>stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> >>>>Author: stefan2
> >>>>Date: Wed May 25 22:20:25 2011
> >>>>New Revision: 1127709
> >>>>
> >>>>URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> >>>>Log:
> >>>>Fix a pool usage issue: svn_cache__get_partial() may be called many
> >>>>times in a row. Thus, the internal pool used to construct keys should
> >>>>be cleared in this function as well from time to time.
> >>>>
> >>>>* subversion/libsvn_subr/cache-membuffer.c
> >>>> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
> >>>>
> >>>>Modified:
> >>>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>>>
> >>>>Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>>>URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> >>>>==============================================================================
> >>>>--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> >>>>+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> >>>>@@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
> >>>>
> >>>> DEBUG_CACHE_MEMBUFFER_INIT_TAG
> >>>>
> >>>>+ if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
> >>>>+ {
> >>>>+ apr_pool_clear(cache->pool);
> >>>>+ cache->alloc_counter = 0;
> >>>>+ }
> >>>>+
> >>>Does this need to be guarded by a cache lock?
> >>>
> >>No. This happens in the outer / front-end code
> >>that merely adds a key prefix (combine_key below)
> >>before calling the shared cache object.
> >>
> >>All front-end operations assume single-threaded
> >>access, which should be o.k. for fs_t-local objects.
> >Okay, if that function is guaranteed (perhaps by API contract) not to
> >run concurrently to any other `front end' function (and in particular to
> >itself), then my concerns are resolved.
> >
> >(I was worried about accessing cache->pool and cache->alloc_counter from
> >multiple threads concurrently --- writer-writer or writer-reader ---
> >which might to undefined behaviour.)
> >
> >What guarantees the single-threaded access? I don't see it documented
> >in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
> >parameters) and the code doesn't take a lock at at that point either.
> >
> I implemented optional thread-safety some days ago but it
> turned out to require / suggest a new API for svn_mutex_t
> structures (handles NULL, APR w/o threading etc.)
> Otherwise, there would be even more duplicated mutex
> handling code.
>
> However, to keep API churn low for 1.7 stabilization, I like
> to suggest the following: simply adding a thread_safe
> parameter to the membuffer cache constructor API and
> returning "not supported / implemented" if set to TRUE.
>
In other words, you're suggesting to have *all* membuffer caches declare
themselves as non-thread-safe in 1.7?
Assuming the cache users are fine with that, it solves the issue
I raised, and all these are private API's, so +1.
Also: creating the membuffer singleton respects the public API's config
singleton's SINGLE_THREADED parameter... so we'd have to somehow
"ignore" that parameter (and rely on FSFS-level mutexes?). Hmm.
> -- Stefan^2.
Re: svn commit: r1127709 - /subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 11.06.2011 14:53, Daniel Shahaf wrote:
> Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
>> On 10.06.2011 22:28, Daniel Shahaf wrote:
>>> stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
>>>> Author: stefan2
>>>> Date: Wed May 25 22:20:25 2011
>>>> New Revision: 1127709
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
>>>> Log:
>>>> Fix a pool usage issue: svn_cache__get_partial() may be called many
>>>> times in a row. Thus, the internal pool used to construct keys should
>>>> be cleared in this function as well from time to time.
>>>>
>>>> * subversion/libsvn_subr/cache-membuffer.c
>>>> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
>>>>
>>>> Modified:
>>>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>>>
>>>> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
>>>> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
>>>> @@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
>>>>
>>>> DEBUG_CACHE_MEMBUFFER_INIT_TAG
>>>>
>>>> + if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
>>>> + {
>>>> + apr_pool_clear(cache->pool);
>>>> + cache->alloc_counter = 0;
>>>> + }
>>>> +
>>> Does this need to be guarded by a cache lock?
>>>
>> No. This happens in the outer / front-end code
>> that merely adds a key prefix (combine_key below)
>> before calling the shared cache object.
>>
>> All front-end operations assume single-threaded
>> access, which should be o.k. for fs_t-local objects.
> Okay, if that function is guaranteed (perhaps by API contract) not to
> run concurrently to any other `front end' function (and in particular to
> itself), then my concerns are resolved.
>
> (I was worried about accessing cache->pool and cache->alloc_counter from
> multiple threads concurrently --- writer-writer or writer-reader ---
> which might to undefined behaviour.)
>
> What guarantees the single-threaded access? I don't see it documented
> in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
> parameters) and the code doesn't take a lock at at that point either.
>
I implemented optional thread-safety some days ago but it
turned out to require / suggest a new API for svn_mutex_t
structures (handles NULL, APR w/o threading etc.)
Otherwise, there would be even more duplicated mutex
handling code.
However, to keep API churn low for 1.7 stabilization, I like
to suggest the following: simply adding a thread_safe
parameter to the membuffer cache constructor API and
returning "not supported / implemented" if set to TRUE.
-- Stefan^2.
Re: svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sat, Jun 11, 2011 at 10:13:27 +0200:
> On 10.06.2011 22:28, Daniel Shahaf wrote:
> >stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> >>Author: stefan2
> >>Date: Wed May 25 22:20:25 2011
> >>New Revision: 1127709
> >>
> >>URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> >>Log:
> >>Fix a pool usage issue: svn_cache__get_partial() may be called many
> >>times in a row. Thus, the internal pool used to construct keys should
> >>be cleared in this function as well from time to time.
> >>
> >>* subversion/libsvn_subr/cache-membuffer.c
> >> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
> >>
> >>Modified:
> >> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>
> >>Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> >>URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> >>==============================================================================
> >>--- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> >>+++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> >>@@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
> >>
> >> DEBUG_CACHE_MEMBUFFER_INIT_TAG
> >>
> >>+ if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
> >>+ {
> >>+ apr_pool_clear(cache->pool);
> >>+ cache->alloc_counter = 0;
> >>+ }
> >>+
> >Does this need to be guarded by a cache lock?
> >
> No. This happens in the outer / front-end code
> that merely adds a key prefix (combine_key below)
> before calling the shared cache object.
>
> All front-end operations assume single-threaded
> access, which should be o.k. for fs_t-local objects.
Okay, if that function is guaranteed (perhaps by API contract) not to
run concurrently to any other `front end' function (and in particular to
itself), then my concerns are resolved.
(I was worried about accessing cache->pool and cache->alloc_counter from
multiple threads concurrently --- writer-writer or writer-reader ---
which might to undefined behaviour.)
What guarantees the single-threaded access? I don't see it documented
in svn_cache.h (on the contrary, that one has explicit 'thread_safe'
parameters) and the code doesn't take a lock at at that point either.
> >> combine_key(cache->prefix,
> >> sizeof(cache->prefix),
> >> key,
> >
> -- Stefan^2.
Re: svn commit: r1127709 - /subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Stefan Fuhrmann <eq...@web.de>.
On 10.06.2011 22:28, Daniel Shahaf wrote:
> stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
>> Author: stefan2
>> Date: Wed May 25 22:20:25 2011
>> New Revision: 1127709
>>
>> URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
>> Log:
>> Fix a pool usage issue: svn_cache__get_partial() may be called many
>> times in a row. Thus, the internal pool used to construct keys should
>> be cleared in this function as well from time to time.
>>
>> * subversion/libsvn_subr/cache-membuffer.c
>> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>>
>> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
>> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
>> @@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
>>
>> DEBUG_CACHE_MEMBUFFER_INIT_TAG
>>
>> + if (++cache->alloc_counter> ALLOCATIONS_PER_POOL_CLEAR)
>> + {
>> + apr_pool_clear(cache->pool);
>> + cache->alloc_counter = 0;
>> + }
>> +
> Does this need to be guarded by a cache lock?
>
No. This happens in the outer / front-end code
that merely adds a key prefix (combine_key below)
before calling the shared cache object.
All front-end operations assume single-threaded
access, which should be o.k. for fs_t-local objects.
>> combine_key(cache->prefix,
>> sizeof(cache->prefix),
>> key,
>
-- Stefan^2.
Re: svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> Author: stefan2
> Date: Wed May 25 22:20:25 2011
> New Revision: 1127709
>
> URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> Log:
> Fix a pool usage issue: svn_cache__get_partial() may be called many
> times in a row. Thus, the internal pool used to construct keys should
> be cleared in this function as well from time to time.
>
> * subversion/libsvn_subr/cache-membuffer.c
> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> @@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
>
> DEBUG_CACHE_MEMBUFFER_INIT_TAG
>
> + if (++cache->alloc_counter > ALLOCATIONS_PER_POOL_CLEAR)
> + {
> + apr_pool_clear(cache->pool);
> + cache->alloc_counter = 0;
> + }
> +
Does this need to be guarded by a cache lock?
> combine_key(cache->prefix,
> sizeof(cache->prefix),
> key,
>
>
Re: svn commit: r1127709 -
/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Wed, May 25, 2011 at 22:20:25 -0000:
> Author: stefan2
> Date: Wed May 25 22:20:25 2011
> New Revision: 1127709
>
> URL: http://svn.apache.org/viewvc?rev=1127709&view=rev
> Log:
> Fix a pool usage issue: svn_cache__get_partial() may be called many
> times in a row. Thus, the internal pool used to construct keys should
> be cleared in this function as well from time to time.
>
> * subversion/libsvn_subr/cache-membuffer.c
> (svn_membuffer_cache_get_partial): regularly clear the internal scratch pool
>
> Modified:
> subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/cache-membuffer.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/cache-membuffer.c?rev=1127709&r1=1127708&r2=1127709&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/cache-membuffer.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/cache-membuffer.c Wed May 25 22:20:25 2011
> @@ -1668,6 +1668,12 @@ svn_membuffer_cache_get_partial(void **v
>
> DEBUG_CACHE_MEMBUFFER_INIT_TAG
>
> + if (++cache->alloc_counter > ALLOCATIONS_PER_POOL_CLEAR)
> + {
> + apr_pool_clear(cache->pool);
> + cache->alloc_counter = 0;
> + }
> +
Does this need to be guarded by a cache lock?
> combine_key(cache->prefix,
> sizeof(cache->prefix),
> key,
>
>