You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2010/11/10 04:05:13 UTC
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./ CHANGES STATUS subversion/svnserve/main.c subversion/tests/cmdline/merge_tests.py
On Nov 9, 2010, at 1:03 PM, hwright@apache.org wrote:
> Author: hwright
> Date: Tue Nov 9 21:03:16 2010
> New Revision: 1033228
>
> URL: http://svn.apache.org/viewvc?rev=1033228&view=rev
> Log:
> Merge r1022675 from trunk:
>
> * r1022675 (r982355 from ^subversion/branches/performance)
> Limit the amount of unused memory fragments held by the root pools.
> Justification:
> In combination with blame -g server side memory leaks, see r1032808 and
> http://svn.haxx.se/dev/archive-2010-11/0102.shtml, svnserve's memory
> usage can get bloated and stay that way, see
> see http://svn.haxx.se/dev/archive-2010-11/0165.shtml.
> Notes:
> Use --accept=mine-full to avoid bizzare branch root property conflicts
> on 'svn:ignore' and 'bugtraq:logregex'. This happens for me with both
> 1.6.14 and trunk@1032984.
> Votes:
> +1: pburba, cmpilato, hwright
>
>
> --- subversion/branches/1.6.x/subversion/svnserve/main.c (original)
> +++ subversion/branches/1.6.x/subversion/svnserve/main.c Tue Nov 9 21:03:16 2010
> @@ -356,6 +356,7 @@ int main(int argc, const char *argv[])
> apr_sockaddr_t *sa;
> apr_pool_t *pool;
> apr_pool_t *connection_pool;
> + apr_allocator_t *allocator;
> svn_error_t *err;
> apr_getopt_t *os;
> int opt;
> @@ -747,10 +748,22 @@ int main(int argc, const char *argv[])
> return ERROR_SUCCESS;
> #endif
>
> + /* If we are using fulltext caches etc., we will allocate many large
> + chunks of memory of various sizes outside the cachde for those
> + fulltexts. Make sure, we use the memory wisely: use an allocator
> + that causes memory fragments to be given back to the OS early. */
> +
> + if (apr_allocator_create(&allocator))
> + return EXIT_FAILURE;
> +
> + apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
> +
> /* Non-standard pool handling. The main thread never blocks to join
> the connection threads so it cannot clean up after each one. So
> separate pools, that can be cleared at thread exit, are used */
> - connection_pool = svn_pool_create(NULL);
> +
> + connection_pool = svn_pool_create_ex(NULL, allocator);
> + apr_allocator_owner_set(allocator, connection_pool);
Shouldn't the allocator have a mutex registered with it, given that svnserve is multithreaded?
Blair
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./ CHANGES
STATUS subversion/svnserve/main.c subversion/tests/cmdline/merge_tests.py
Posted by Blair Zajac <bl...@orcaware.com>.
On 11/10/2010 10:34 AM, Philip Martin wrote:
> Blair Zajac<bl...@orcaware.com> writes:
>
>>> + /* If we are using fulltext caches etc., we will allocate many large
>>> + chunks of memory of various sizes outside the cachde for those
>>> + fulltexts. Make sure, we use the memory wisely: use an allocator
>>> + that causes memory fragments to be given back to the OS early. */
>>> +
>>> + if (apr_allocator_create(&allocator))
>>> + return EXIT_FAILURE;
>>> +
>>> + apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
>>> +
>>> /* Non-standard pool handling. The main thread never blocks to join
>>> the connection threads so it cannot clean up after each one. So
>>> separate pools, that can be cleared at thread exit, are used */
>>> - connection_pool = svn_pool_create(NULL);
>>> +
>>> + connection_pool = svn_pool_create_ex(NULL, allocator);
>>> + apr_allocator_owner_set(allocator, connection_pool);
>>
>>
>> Shouldn't the allocator have a mutex registered with it, given that
>> svnserve is multithreaded?
>
> No. I believe each thread gets its own allocator and pool, from which it
> creates subpools as required. When the thread exits it destroys the
> pool, which destroys the allocator since the pool has been set as the
> owner. So each allocator is only used by one thread.
Thanks Philip for looking into it and seeing that we're fine.
Blair
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./ CHANGES STATUS subversion/svnserve/main.c subversion/tests/cmdline/merge_tests.py
Posted by Philip Martin <ph...@wandisco.com>.
Blair Zajac <bl...@orcaware.com> writes:
>> + /* If we are using fulltext caches etc., we will allocate many large
>> + chunks of memory of various sizes outside the cachde for those
>> + fulltexts. Make sure, we use the memory wisely: use an allocator
>> + that causes memory fragments to be given back to the OS early. */
>> +
>> + if (apr_allocator_create(&allocator))
>> + return EXIT_FAILURE;
>> +
>> + apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
>> +
>> /* Non-standard pool handling. The main thread never blocks to join
>> the connection threads so it cannot clean up after each one. So
>> separate pools, that can be cleared at thread exit, are used */
>> - connection_pool = svn_pool_create(NULL);
>> +
>> + connection_pool = svn_pool_create_ex(NULL, allocator);
>> + apr_allocator_owner_set(allocator, connection_pool);
>
>
> Shouldn't the allocator have a mutex registered with it, given that
> svnserve is multithreaded?
No. I believe each thread gets its own allocator and pool, from which it
creates subpools as required. When the thread exits it destroys the
pool, which destroys the allocator since the pool has been set as the
owner. So each allocator is only used by one thread.
--
Philip
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./ CHANGES
STATUS subversion/svnserve/main.c subversion/tests/cmdline/merge_tests.py
Posted by Stefan Fuhrmann <eq...@web.de>.
On 27.12.2010 19:16, Blair Zajac wrote:
> On 12/27/10 7:45 AM, Daniel Shahaf wrote:
>> Ping...
>>
>> Blair Zajac wrote on Tue, Nov 09, 2010 at 20:05:13 -0800:
>>>
>>> On Nov 9, 2010, at 1:03 PM, hwright@apache.org wrote:
>>>
>>>> Author: hwright
>>>> Date: Tue Nov 9 21:03:16 2010
>>>> New Revision: 1033228
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1033228&view=rev
>>>> Log:
>>>> Merge r1022675 from trunk:
>>>>
>>>> * r1022675 (r982355 from ^subversion/branches/performance)
>>>> Limit the amount of unused memory fragments held by the root pools.
>>>> Justification:
>>>> In combination with blame -g server side memory leaks, see
>>>> r1032808 and
>>>> http://svn.haxx.se/dev/archive-2010-11/0102.shtml, svnserve's
>>>> memory
>>>> usage can get bloated and stay that way, see
>>>> see http://svn.haxx.se/dev/archive-2010-11/0165.shtml.
>>>> Notes:
>>>> Use --accept=mine-full to avoid bizzare branch root property
>>>> conflicts
>>>> on 'svn:ignore' and 'bugtraq:logregex'. This happens for me
>>>> with both
>>>> 1.6.14 and trunk@1032984.
>>>> Votes:
>>>> +1: pburba, cmpilato, hwright
>>>>
>>>
>>>>
>>>> --- subversion/branches/1.6.x/subversion/svnserve/main.c (original)
>>>> +++ subversion/branches/1.6.x/subversion/svnserve/main.c Tue Nov 9
>>>> 21:03:16 2010
>>>> @@ -356,6 +356,7 @@ int main(int argc, const char *argv[])
>>>> apr_sockaddr_t *sa;
>>>> apr_pool_t *pool;
>>>> apr_pool_t *connection_pool;
>>>> + apr_allocator_t *allocator;
>>>> svn_error_t *err;
>>>> apr_getopt_t *os;
>>>> int opt;
>>>> @@ -747,10 +748,22 @@ int main(int argc, const char *argv[])
>>>> return ERROR_SUCCESS;
>>>> #endif
>>>>
>>>> + /* If we are using fulltext caches etc., we will allocate
>>>> many large
>>>> + chunks of memory of various sizes outside the cachde for
>>>> those
>>>> + fulltexts. Make sure, we use the memory wisely: use an
>>>> allocator
>>>> + that causes memory fragments to be given back to the OS
>>>> early. */
>>>> +
>>>> + if (apr_allocator_create(&allocator))
>>>> + return EXIT_FAILURE;
>>>> +
>>>> + apr_allocator_max_free_set(allocator,
>>>> SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
>>>> +
>>>> /* Non-standard pool handling. The main thread never blocks
>>>> to join
>>>> the connection threads so it cannot clean up after each
>>>> one. So
>>>> separate pools, that can be cleared at thread exit, are
>>>> used */
>>>> - connection_pool = svn_pool_create(NULL);
>>>> +
>>>> + connection_pool = svn_pool_create_ex(NULL, allocator);
>>>> + apr_allocator_owner_set(allocator, connection_pool);
>>>
>>>
>>> Shouldn't the allocator have a mutex registered with it, given that
>>> svnserve is multithreaded?
>
> We had a chat on IRC and it looks like the code is fine. I'd have to
> dig through IRC logs to fine the conversation.
>
> Blair
>
Every thread creates its own allocator and root pool object
(as indicated by the comment). Both are then only accessed
by the same thread that created them.
-- Stefan^2.
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./ CHANGES
STATUS subversion/svnserve/main.c subversion/tests/cmdline/merge_tests.py
Posted by Blair Zajac <bl...@orcaware.com>.
On 12/27/10 7:45 AM, Daniel Shahaf wrote:
> Ping...
>
> Blair Zajac wrote on Tue, Nov 09, 2010 at 20:05:13 -0800:
>>
>> On Nov 9, 2010, at 1:03 PM, hwright@apache.org wrote:
>>
>>> Author: hwright
>>> Date: Tue Nov 9 21:03:16 2010
>>> New Revision: 1033228
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1033228&view=rev
>>> Log:
>>> Merge r1022675 from trunk:
>>>
>>> * r1022675 (r982355 from ^subversion/branches/performance)
>>> Limit the amount of unused memory fragments held by the root pools.
>>> Justification:
>>> In combination with blame -g server side memory leaks, see r1032808 and
>>> http://svn.haxx.se/dev/archive-2010-11/0102.shtml, svnserve's memory
>>> usage can get bloated and stay that way, see
>>> see http://svn.haxx.se/dev/archive-2010-11/0165.shtml.
>>> Notes:
>>> Use --accept=mine-full to avoid bizzare branch root property conflicts
>>> on 'svn:ignore' and 'bugtraq:logregex'. This happens for me with both
>>> 1.6.14 and trunk@1032984.
>>> Votes:
>>> +1: pburba, cmpilato, hwright
>>>
>>
>>>
>>> --- subversion/branches/1.6.x/subversion/svnserve/main.c (original)
>>> +++ subversion/branches/1.6.x/subversion/svnserve/main.c Tue Nov 9 21:03:16 2010
>>> @@ -356,6 +356,7 @@ int main(int argc, const char *argv[])
>>> apr_sockaddr_t *sa;
>>> apr_pool_t *pool;
>>> apr_pool_t *connection_pool;
>>> + apr_allocator_t *allocator;
>>> svn_error_t *err;
>>> apr_getopt_t *os;
>>> int opt;
>>> @@ -747,10 +748,22 @@ int main(int argc, const char *argv[])
>>> return ERROR_SUCCESS;
>>> #endif
>>>
>>> + /* If we are using fulltext caches etc., we will allocate many large
>>> + chunks of memory of various sizes outside the cachde for those
>>> + fulltexts. Make sure, we use the memory wisely: use an allocator
>>> + that causes memory fragments to be given back to the OS early. */
>>> +
>>> + if (apr_allocator_create(&allocator))
>>> + return EXIT_FAILURE;
>>> +
>>> + apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
>>> +
>>> /* Non-standard pool handling. The main thread never blocks to join
>>> the connection threads so it cannot clean up after each one. So
>>> separate pools, that can be cleared at thread exit, are used */
>>> - connection_pool = svn_pool_create(NULL);
>>> +
>>> + connection_pool = svn_pool_create_ex(NULL, allocator);
>>> + apr_allocator_owner_set(allocator, connection_pool);
>>
>>
>> Shouldn't the allocator have a mutex registered with it, given that svnserve is multithreaded?
We had a chat on IRC and it looks like the code is fine. I'd have to dig
through IRC logs to fine the conversation.
Blair
Re: svn commit: r1033228 - in /subversion/branches/1.6.x: ./
CHANGES STATUS subversion/svnserve/main.c
subversion/tests/cmdline/merge_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Ping...
Blair Zajac wrote on Tue, Nov 09, 2010 at 20:05:13 -0800:
>
> On Nov 9, 2010, at 1:03 PM, hwright@apache.org wrote:
>
> > Author: hwright
> > Date: Tue Nov 9 21:03:16 2010
> > New Revision: 1033228
> >
> > URL: http://svn.apache.org/viewvc?rev=1033228&view=rev
> > Log:
> > Merge r1022675 from trunk:
> >
> > * r1022675 (r982355 from ^subversion/branches/performance)
> > Limit the amount of unused memory fragments held by the root pools.
> > Justification:
> > In combination with blame -g server side memory leaks, see r1032808 and
> > http://svn.haxx.se/dev/archive-2010-11/0102.shtml, svnserve's memory
> > usage can get bloated and stay that way, see
> > see http://svn.haxx.se/dev/archive-2010-11/0165.shtml.
> > Notes:
> > Use --accept=mine-full to avoid bizzare branch root property conflicts
> > on 'svn:ignore' and 'bugtraq:logregex'. This happens for me with both
> > 1.6.14 and trunk@1032984.
> > Votes:
> > +1: pburba, cmpilato, hwright
> >
>
> >
> > --- subversion/branches/1.6.x/subversion/svnserve/main.c (original)
> > +++ subversion/branches/1.6.x/subversion/svnserve/main.c Tue Nov 9 21:03:16 2010
> > @@ -356,6 +356,7 @@ int main(int argc, const char *argv[])
> > apr_sockaddr_t *sa;
> > apr_pool_t *pool;
> > apr_pool_t *connection_pool;
> > + apr_allocator_t *allocator;
> > svn_error_t *err;
> > apr_getopt_t *os;
> > int opt;
> > @@ -747,10 +748,22 @@ int main(int argc, const char *argv[])
> > return ERROR_SUCCESS;
> > #endif
> >
> > + /* If we are using fulltext caches etc., we will allocate many large
> > + chunks of memory of various sizes outside the cachde for those
> > + fulltexts. Make sure, we use the memory wisely: use an allocator
> > + that causes memory fragments to be given back to the OS early. */
> > +
> > + if (apr_allocator_create(&allocator))
> > + return EXIT_FAILURE;
> > +
> > + apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
> > +
> > /* Non-standard pool handling. The main thread never blocks to join
> > the connection threads so it cannot clean up after each one. So
> > separate pools, that can be cleared at thread exit, are used */
> > - connection_pool = svn_pool_create(NULL);
> > +
> > + connection_pool = svn_pool_create_ex(NULL, allocator);
> > + apr_allocator_owner_set(allocator, connection_pool);
>
>
> Shouldn't the allocator have a mutex registered with it, given that svnserve is multithreaded?
>
> Blair
>