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 2010/08/04 20:43:29 UTC
svn commit: r982355 -
/subversion/branches/performance/subversion/svnserve/main.c
Author: stefan2
Date: Wed Aug 4 18:43:29 2010
New Revision: 982355
URL: http://svn.apache.org/viewvc?rev=982355&view=rev
Log:
Fix an issue with fulltext caching already present in production SVN:
APR pools often won't reuse memory fragments if they are larger
than 80kB. Using string buffers while reconstructing fulltexts does
exactly The Bad Thing: request large buffers of various sizes that
APR pools will often not reuse due to their differing and often just
a tad bit too small size.
Use an allocator to limit the amount of unused memory fragments
held by the root pools.
* subversion/svnserve/main.c
(main): limit the idle memory allocated per request root pool to 4 MB.
Modified:
subversion/branches/performance/subversion/svnserve/main.c
Modified: subversion/branches/performance/subversion/svnserve/main.c
URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/svnserve/main.c?rev=982355&r1=982354&r2=982355&view=diff
==============================================================================
--- subversion/branches/performance/subversion/svnserve/main.c (original)
+++ subversion/branches/performance/subversion/svnserve/main.c Wed Aug 4 18:43:29 2010
@@ -402,6 +402,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;
@@ -859,10 +860,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);
status = apr_socket_accept(&usock, sock, connection_pool);
if (handling_mode == connection_mode_fork)
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Blair Zajac <bl...@orcaware.com>.
On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Wed Aug 4 18:43:29 2010
> New Revision: 982355
>
> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
> Log:
> Fix an issue with fulltext caching already present in production SVN:
> APR pools often won't reuse memory fragments if they are larger
> than 80kB. Using string buffers while reconstructing fulltexts does
> exactly The Bad Thing: request large buffers of various sizes that
> APR pools will often not reuse due to their differing and often just
> a tad bit too small size.
>
> Use an allocator to limit the amount of unused memory fragments
> held by the root pools.
>
> * subversion/svnserve/main.c
> (main): limit the idle memory allocated per request root pool to 4 MB.
>
> Modified:
> subversion/branches/performance/subversion/svnserve/main.c
>
> Modified: subversion/branches/performance/subversion/svnserve/main.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/svnserve/main.c?rev=982355&r1=982354&r2=982355&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/svnserve/main.c (original)
> +++ subversion/branches/performance/subversion/svnserve/main.c Wed Aug 4 18:43:29 2010
> @@ -402,6 +402,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;
> @@ -859,10 +860,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);
I'm just spamming this commit, but given svnserve is multithreaded, don't you need to register a mutex with the allocator?
Blair
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Blair Zajac <bl...@orcaware.com>.
On Nov 9, 2010, at 3:38 PM, Stefan Fuhrmann wrote:
> On 09.11.2010 17:52, Blair Zajac wrote:
>> On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
>>
>>> Author: stefan2
>>> Date: Wed Aug 4 18:43:29 2010
>>> New Revision: 982355
>>>
>>> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
>>> Log:
>>> Fix an issue with fulltext caching already present in production SVN:
>>> APR pools often won't reuse memory fragments if they are larger
>>> than 80kB. Using string buffers while reconstructing fulltexts does
>>> exactly The Bad Thing: request large buffers of various sizes that
>>> APR pools will often not reuse due to their differing and often just
>>> a tad bit too small size.
>>> @@ -859,10 +860,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
>> s/cachde/cache/
>>
>> Blair
> Thanks. Committed as r1033294.
Great!
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Stefan Fuhrmann <st...@alice-dsl.de>.
On 09.11.2010 17:52, Blair Zajac wrote:
> On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
>
>> Author: stefan2
>> Date: Wed Aug 4 18:43:29 2010
>> New Revision: 982355
>>
>> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
>> Log:
>> Fix an issue with fulltext caching already present in production SVN:
>> APR pools often won't reuse memory fragments if they are larger
>> than 80kB. Using string buffers while reconstructing fulltexts does
>> exactly The Bad Thing: request large buffers of various sizes that
>> APR pools will often not reuse due to their differing and often just
>> a tad bit too small size.
>> @@ -859,10 +860,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
> s/cachde/cache/
>
> Blair
Thanks. Committed as r1033294.
-- Stefan^2.
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Blair Zajac <bl...@orcaware.com>.
On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Wed Aug 4 18:43:29 2010
> New Revision: 982355
>
> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
> Log:
> Fix an issue with fulltext caching already present in production SVN:
> APR pools often won't reuse memory fragments if they are larger
> than 80kB. Using string buffers while reconstructing fulltexts does
> exactly The Bad Thing: request large buffers of various sizes that
> APR pools will often not reuse due to their differing and often just
> a tad bit too small size.
> @@ -859,10 +860,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
s/cachde/cache/
Blair
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Blair Zajac <bl...@orcaware.com>.
On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Wed Aug 4 18:43:29 2010
> New Revision: 982355
>
> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
> Log:
> Fix an issue with fulltext caching already present in production SVN:
> APR pools often won't reuse memory fragments if they are larger
> than 80kB. Using string buffers while reconstructing fulltexts does
> exactly The Bad Thing: request large buffers of various sizes that
> APR pools will often not reuse due to their differing and often just
> a tad bit too small size.
>
> Use an allocator to limit the amount of unused memory fragments
> held by the root pools.
>
> * subversion/svnserve/main.c
> (main): limit the idle memory allocated per request root pool to 4 MB.
>
> Modified:
> subversion/branches/performance/subversion/svnserve/main.c
>
> Modified: subversion/branches/performance/subversion/svnserve/main.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/svnserve/main.c?rev=982355&r1=982354&r2=982355&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/svnserve/main.c (original)
> +++ subversion/branches/performance/subversion/svnserve/main.c Wed Aug 4 18:43:29 2010
> @@ -402,6 +402,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;
> @@ -859,10 +860,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);
I'm just spamming this commit, but given svnserve is multithreaded, don't you need to register a mutex with the allocator?
Blair
Re: svn commit: r982355 - /subversion/branches/performance/subversion/svnserve/main.c
Posted by Blair Zajac <bl...@orcaware.com>.
On Aug 4, 2010, at 11:43 AM, stefan2@apache.org wrote:
> Author: stefan2
> Date: Wed Aug 4 18:43:29 2010
> New Revision: 982355
>
> URL: http://svn.apache.org/viewvc?rev=982355&view=rev
> Log:
> Fix an issue with fulltext caching already present in production SVN:
> APR pools often won't reuse memory fragments if they are larger
> than 80kB. Using string buffers while reconstructing fulltexts does
> exactly The Bad Thing: request large buffers of various sizes that
> APR pools will often not reuse due to their differing and often just
> a tad bit too small size.
>
> Use an allocator to limit the amount of unused memory fragments
> held by the root pools.
Some questions on this:
Do we not need this in mod_dav_svn because we assume that Apache exits child processes after N HTTP requests?
What about for Windows Apache servers, where they only have a threading server model, should this be enabled? Or would it be too hard to have only svn pools use this
We have a custom, multithreaded Ice Subversion server that can run for months at a time. It only serves full texts to clients and it doesn't expose any diff methods. I've seen slow growth over time, but we also have an internal LRU cache for svn_repos_t, svn_fs_t, svn_fs_root_t, so it's hard to tell what's allocating memory. I guess we should do that in this process just to be safe?
It looks like every command line tool we have does this:
./subversion/svnadmin/main.c:1578: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svndumpfilter/main.c:1277: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svnlook/main.c:2214: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svn/main.c:1260: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svnserve/main.c:806: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svnsync/main.c:1808: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./subversion/svnversion/main.c:149: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./tools/client-side/svnmucc/svnmucc.c:88: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./tools/dev/svnraisetreeconflict/main.c:341: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
./tools/server-side/svn-rep-sharing-stats.c:451: apr_allocator_max_free_set(allocator, SVN_ALLOCATOR_RECOMMENDED_MAX_FREE);
Any reason we just don't make this part of svn_cmdline_init()?
Blair