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

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

From: Peter N. Lundblad <peter_at_famlundblad.se>
Date: 2004-08-15 22:41:46 CEST

On Sun, 15 Aug 2004, Greg Hudson wrote:

> On Sun, 2004-08-15 at 10:29, Peter N. Lundblad wrote:
> > But apr_pool_userdata_{get,set} aren't thread-safe, so you can't use them
> > on the same pool in different threads simultanously.
>
> Yeah, I think this is the lynchpin of the problem.
>
> > Oh, had we only had init routines that were required to be called
> > by the user!
>
> What would we do with them? We could add one, and just say that xlate
> handle caching isn't very good unless you call it. But adding an
> initializer before 1.1 frightens me, since we wouldn't really have a lot
> of time to consider the API design.
>
I would initialize thread specific data and store the xlate handles there.
Seems like APR detects if initialize has been called by a plain int. We
could do the same, requiring the init function to be called before
anything in other threads use the lib. Then we can detect if the TSD is to
be used.

> (Also, if you're going to use the init routine to initialize a global
> lock, I think that would hurt the MP performance of apps which use svn.
> Right now, if you have N threads performing svn operations, each with a
> totally independent pool, they won't lock against each other. But if
> you add a global lock in a frequently-used low-level routine, that
> changes.)
>
Correct, so I wouldn't.

> I think I've identified an alternative. The APR pool code locks pools
> (or rather, locks the allocator which I believe is shared between all
> pools in a tree of pools) like this:
>
...
> All of those functions are public. So we should be able to do the same
> thing ourselves. (The allocator mutex could be retrieved again in the
> unlock code, so we could add svn_pool_lock() and svn_pool_unlock() to
> svn_pools.h, although we might not want to do that in the 1.1 backport.)
>
> That gives us a couple of options for storing xlate handles in parent
> pools. We can either store bare xlate handles and lock around the
...

But the apr_pool_userdata_{get,set} isn't thread-safe just because we
protect our uses by a lock. Other callers don't.

I don't see a way to improve the performance via caching in pools. If we
want to fix this, I think we need to add init functions and be able to
detect if they were called. If this is problematic for 1.1 ( to which I
might agree), then we won't fix this until 1.2. This will hurt TSVN users
I think and that's bad because that's an important application on Windows.

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Aug 15 22:28:49 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.