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

Re: r10788 (translation handle caching)

From: <kfogel_at_collab.net>
Date: 2004-10-14 21:07:45 CEST

Sorry, I don't have the original commit email for r10788 anymore, so
I'm using the output of tools/client-side/showchange.pl below:

> ------------------------------------------------------------------------
> r10788 | lundblad | 2004-08-31 14:20:47 -0500 (Tue, 31 Aug 2004) | 19 lines
> Changed paths:
> M /trunk/subversion/include/svn_utf.h
> M /trunk/subversion/libsvn_subr/cmdline.c
> M /trunk/subversion/libsvn_subr/utf.c
>
> Fix issue #2016. Cache UTF8 translation handles in a global hash table
> instead of in the current pool, which improves performance, especially on
> Windows.
>
> * include/svn_utf.h (svn_utf_initialize): New function.
> * libsvn_subr/utf.c (xlate_handle_mutex, xlate_handle_hash): New variables.
> (xlate_handle_node_t): New struct.
> (xlate_cleanup, svn_utf_initialize, put_xlate_handle_node): New functions.
> (get_xlate_handle_node): Renamed from get_xlate_handle and rewritten using
> a global hash table and locking in MT environments.
> (get_ntou_xlate_handle_node, get_uton_xlate_handle_node,
> svn_utf_stringbuf_to_utf8, svn_utf_string_to_utf8, svn_utf_cstring_to_utf8,
> svn_utf_cstring_to_utf8_ex, svn_utf_stringbuf_from_utf8,
> svn_utf_string_from_utf8, svn_utf_cstring_from_utf8,
> svn_utf_cstring_from_utf8_ex, svn_utf_cstring_from_utf8_string): Use
> {get,put}_xlate_handle_node for the
> xlate handle.
> * libsvn_subr/cmdline.c (svn_cmdline_initialize): Initialize UTF-8 routines.

Okay, got that...

> --- subversion/libsvn_subr/utf.c (revision 10787)
> +++ subversion/libsvn_subr/utf.c (revision 10788)
>
> [...]
>
> @@ -44,27 +106,79 @@
> return SVN_NO_ERROR; if fail for some other reason, return
> error. */
> static svn_error_t *
> -get_xlate_handle (apr_xlate_t **ret,
> - const char *topage, const char *frompage,
> - const char *userdata_key, apr_pool_t *pool)
> +get_xlate_handle_node (xlate_handle_node_t **ret,
> + const char *topage, const char *frompage,
> + const char *userdata_key, apr_pool_t *pool)
> {
> - void *old_handle = NULL;
> + xlate_handle_node_t *old_handle = NULL;
> apr_status_t apr_err;
>
> /* If we already have a handle, just return it. */
> if (userdata_key)
> {
> - apr_pool_userdata_get (&old_handle, userdata_key, pool);
> - if (old_handle != NULL)
> +#if APR_HAS_THREADS
> + if (xlate_handle_mutex)
> {
> - *ret = old_handle;
> - return SVN_NO_ERROR;
> + apr_err = apr_thread_mutex_lock (xlate_handle_mutex);
> + if (apr_err != APR_SUCCESS)
> + return svn_error_create (apr_err, NULL,
> + "Can't lock charset translation "
> + "mutex");
> + old_handle = apr_hash_get (xlate_handle_hash, userdata_key,
> + APR_HASH_KEY_STRING);
> + if (old_handle)
> + {
> + /* Remove from the hash table. */
> + apr_hash_set (xlate_handle_hash, userdata_key,
> + APR_HASH_KEY_STRING, old_handle->next);
> + old_handle->next = NULL;
> + apr_err = apr_thread_mutex_unlock (xlate_handle_mutex);
> + if (apr_err != APR_SUCCESS)
> + return svn_error_create (apr_err, NULL,
> + "Can't unlock charset "
> + "translation mutex");
> + *ret = old_handle;
> + return SVN_NO_ERROR;
> + }

That comment "Remove from the hash table" confuses me a bit. To
remove something from a hash table, we set the value to NULL. But if
'old_handle->next' was already NULL (which we have no reason to
believe it is, right?) then assignment line right afterwards

   old_handle->next = NULL;

would be pointless...

Am I misunderstanding something?

Haven't finished reviewing the commit, so I may have more questions
later. I wanted to ask this now, though.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Oct 14 22:57:22 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.