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

Re: [PATCH] Re: 1.1rc1 performance regression in 'svn status' (and also 1.1rc2)

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2004-08-18 16:22:44 CEST

Philip Martin <philip@codematters.co.uk> writes:

> "Peter N. Lundblad" <peter@famlundblad.se> writes:
>
>>> > + handle_pool = svn_pool_create (NULL);
>>>
>>> This creates a pool in every thread, but I don't see the pool getting
>>> cleared or destroyed. An application that creates threads continually
>>> (e.g. threaded svnserve) will cause the number of pools to grow
>>> without bound.
>>>
>> xlate_cleanup will take care of that. It is called when the thread is
>> destoyed.
>
> Unless you are planning to write some more code I don't understand.
> xlate_cleanup appears to be applied to the pool passed to
> svn_utf_initialize. That pool could well have a lifetime much longer
> than the thread lifetime, typically I would expect the pool to last as
> long as the application runs.

I see now, it's a thread cleanup not a pool cleanup. That would work
if APR were not broken as you point out below.

>> Problems is, when I check the implementation of
>> apr_threadkey_private_create, the destructor is only used on POSIX and
>> netware systems, not on Windows/OS2. So on these platforms, it is a leak
>> after all. Maybe there is no way to register cleanups on these
>> platforms,. This should have been documented in the APR headers!!!

That does look broken, it should probably be brought up on the dev@apr
list.

Without a working thread destructor it will be very hard to use a
thread private solution to solve the caching problem.

>>
>>> > + handle_hash = apr_hash_make (pool);
>>> > + if (apr_threadkey_private_set (handle_hash, xlate_handle_key)
>>>
>>> If pool is cleared handle_hash will become invalid, but will still be
>>> referenced by the thread private data. The next time the thread calls
>>> get_xlate_handle it will crash, or do something worse.
>>>
>> When the pool is cleared, the thread is dying, so the TSD for that thread
>> isn't used again. This would be true where the pool is destroyed at all...
>
> Again I don't understand. pool is an argument to one of the
> svn_utf_xxx_(to/from)_xxx functions, it could well have a lifetime
> much shorter than the thread and may well get repeatedly cleared.

Did you mean to allocate handle_hash from handle_pool rather than
pool? Then the hash would have the correct lifetime. It's all a bit
academic if thread destructors don't work on win32.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Aug 18 16:23:13 2004

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.