You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2004/09/01 00:15:16 UTC

Re: svn commit: r10788 - in trunk/subversion: include libsvn_subr

lundblad@tigris.org wrote:

>Author: lundblad
>Date: Tue Aug 31 14:20:47 2004
>New Revision: 10788
>
>Modified:
>   trunk/subversion/include/svn_utf.h
>   trunk/subversion/libsvn_subr/cmdline.c
>   trunk/subversion/libsvn_subr/utf.c
>Log:
>Fix issue #2016.  Cache UTF8 translation handles in a global hash table
>instead of in the current pool, which improves performance, especially on
>Windows.
>  
>
Apart from a bug that breaks compilation on (at least) FreeBSD, which I 
suspect is caused by using "#ifdef APR_HAS_THREADS" in some places 
instead of consistently testing with "#if"...

I think there is a better way to do this, that could avoid using global 
data and locks: add the xlate cache to the svn_client_context_t struct, 
and document that this structure is thread-specific. Most public 
functions need to grow a thread-specific context parameter anyway, 
although that will have to wait for 2.0. But I think it should be 
possible to use the cache from the context in most places where we do 
charset conversions now, by adding a set of conversion functions that 
take a context parameter.

-- Brane


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r10788 - in trunk/subversion: include libsvn_subr

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 1 Sep 2004, [UTF-8] Branko Ä^Libej wrote:

> lundblad@tigris.org wrote:
>
> >Author: lundblad
> >Date: Tue Aug 31 14:20:47 2004
> >New Revision: 10788
> >
> >Modified:
> >   trunk/subversion/include/svn_utf.h
> >   trunk/subversion/libsvn_subr/cmdline.c
> >   trunk/subversion/libsvn_subr/utf.c
> >Log:
> >Fix issue #2016.  Cache UTF8 translation handles in a global hash table
> >instead of in the current pool, which improves performance, especially on
> >Windows.
> >
> >
> Apart from a bug that breaks compilation on (at least) FreeBSD, which I
> suspect is caused by using "#ifdef APR_HAS_THREADS" in some places
> instead of consistently testing with "#if"...
>
Thanks for analyzing this. Fixed in r10796.

> I think there is a better way to do this, that could avoid using global
> data and locks: add the xlate cache to the svn_client_context_t struct,
> and document that this structure is thread-specific. Most public
> functions need to grow a thread-specific context parameter anyway,
> although that will have to wait for 2.0. But I think it should be
> possible to use the cache from the context in most places where we do
> charset conversions now, by adding a set of conversion functions that
> take a context parameter.
>
I don't agree. How would libsvn_wc get the client context? IN the st case,
most of the translations are related to converting filenames. So you would
have to push the context down to the svn_io functions for this. An svn
context could be a good design for 2.0, but it doesn't solve the current
problem. Note that I'm talking about actual calls, not call sites in the
code.

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn commit: r10788 - in trunk/subversion: include libsvn_subr

Posted by Branko Čibej <br...@xbc.nu>.
Mark Benedetto King wrote:

>On Wed, Sep 01, 2004 at 02:15:16AM +0200, Branko ??ibej wrote:
>  
>
>>I think there is a better way to do this, that could avoid using global 
>>data and locks: add the xlate cache to the svn_client_context_t struct, 
>>and document that this structure is thread-specific. Most public 
>>functions need to grow a thread-specific context parameter anyway, 
>>although that will have to wait for 2.0. But I think it should be 
>>possible to use the cache from the context in most places where we do 
>>charset conversions now, by adding a set of conversion functions that 
>>take a context parameter.
>>
>>    
>>
>
>Why stop there?  Why not just put an xlate callback in the client context?
>  
>
Hum. That would definitely avoid the need to add new public xlate 
functions. The problem is, though, that there are quite a few of those, 
so it's not just one xlate callback but yet another vtable. I do think 
this approach is worth considering, tbough.

(The words "C++" and "abstract classes" and "context objects" also keep 
popping up in my mind every time I think about these things, *sigh*)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r10788 - in trunk/subversion: include libsvn_subr

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 1 Sep 2004, Mark Benedetto King wrote:

> On Wed, Sep 01, 2004 at 02:15:16AM +0200, Branko ??ibej wrote:
> >
> > I think there is a better way to do this, that could avoid using global
> > data and locks: add the xlate cache to the svn_client_context_t struct,
> > and document that this structure is thread-specific. Most public
> > functions need to grow a thread-specific context parameter anyway,
> > although that will have to wait for 2.0. But I think it should be
> > possible to use the cache from the context in most places where we do
> > charset conversions now, by adding a set of conversion functions that
> > take a context parameter.
> >
>
> Why stop there?  Why not just put an xlate callback in the client context?
>
Sorry, but I don't see at all why this would be useful. Why would one want
to replace the translation *function*? We'd have to include handles
anyway, and that's what varies. I am probably totally missing something.

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r10788 - in trunk/subversion: include libsvn_subr

Posted by Mark Benedetto King <mb...@lowlatency.com>.
On Wed, Sep 01, 2004 at 02:15:16AM +0200, Branko ??ibej wrote:
> 
> I think there is a better way to do this, that could avoid using global 
> data and locks: add the xlate cache to the svn_client_context_t struct, 
> and document that this structure is thread-specific. Most public 
> functions need to grow a thread-specific context parameter anyway, 
> although that will have to wait for 2.0. But I think it should be 
> possible to use the cache from the context in most places where we do 
> charset conversions now, by adding a set of conversion functions that 
> take a context parameter.
> 

Why stop there?  Why not just put an xlate callback in the client context?

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org