From 331e93c3f1ad95cb4add48213613e8eeee8cb386 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 28 Jul 2013 00:21:00 +0100 Subject: [PATCH] Drop iconv caching code This was introduced for Solaris performance theoretically; we have never been able to use it on Linux/glibc because the UTF-16 BOM state isn't reset. We have no data about Solaris performance; were some to still exist, we could reintroduce the code with an explicit check for Solaris, not a check for glibc. https://bugzilla.gnome.org/show_bug.cgi?id=704999 --- configure.ac | 27 ----- glib/gconvert.c | 297 ------------------------------------------------ 2 files changed, 324 deletions(-) diff --git a/configure.ac b/configure.ac index 94437096d..943da7fb9 100644 --- a/configure.ac +++ b/configure.ac @@ -434,33 +434,6 @@ AS_IF([ test x"$glib_native_win32" = xyes], [ fi ]) -AC_ARG_ENABLE(iconv-cache, - [AS_HELP_STRING([--enable-iconv-cache=@<:@yes/no/auto@:>@], - [cache iconv descriptors [default=auto]])],, - [enable_iconv_cache=auto]) - -AC_MSG_CHECKING([whether to cache iconv descriptors]) -case $enable_iconv_cache in - auto) - if test $ac_cv_gnu_library_2_1 = yes; then - enable_iconv_cache=no - else - enable_iconv_cache=yes - fi - ;; - yes|no) - ;; - *) AC_MSG_ERROR([Value given to --enable-iconv-cache must be one of yes, no or auto]) - ;; -esac - -if test $enable_iconv_cache = yes; then - AC_DEFINE(NEED_ICONV_CACHE,1,[Do we cache iconv descriptors]) -fi - -AC_MSG_RESULT($enable_iconv_cache) - - dnl dnl zlib support dnl diff --git a/glib/gconvert.c b/glib/gconvert.c index 3320a5e0b..c28e171d0 100644 --- a/glib/gconvert.c +++ b/glib/gconvert.c @@ -51,11 +51,6 @@ #include "gunicode.h" #include "gfileutils.h" -#ifdef NEED_ICONV_CACHE -#include "glist.h" -#include "ghash.h" -#endif - #include "glibintl.h" #if defined(USE_LIBICONV_GNU) && !defined (_LIBICONV_H) @@ -333,296 +328,6 @@ g_iconv_close (GIConv converter) return iconv_close (cd); } - -#ifdef NEED_ICONV_CACHE - -#define ICONV_CACHE_SIZE (16) - -struct _iconv_cache_bucket { - gchar *key; - guint32 refcount; - gboolean used; - GIConv cd; -}; - -static GList *iconv_cache_list; -static GHashTable *iconv_cache; -static GHashTable *iconv_open_hash; -static guint iconv_cache_size = 0; -G_LOCK_DEFINE_STATIC (iconv_cache_lock); - -/* caller *must* hold the iconv_cache_lock */ -static void -iconv_cache_init (void) -{ - static gboolean initialized = FALSE; - - if (initialized) - return; - - iconv_cache_list = NULL; - iconv_cache = g_hash_table_new (g_str_hash, g_str_equal); - iconv_open_hash = g_hash_table_new (g_direct_hash, g_direct_equal); - - initialized = TRUE; -} - - -/* - * iconv_cache_bucket_new: - * @key: cache key - * @cd: iconv descriptor - * - * Creates a new cache bucket, inserts it into the cache and - * increments the cache size. - * - * This assumes ownership of @key. - * - * Returns a pointer to the newly allocated cache bucket. - **/ -static struct _iconv_cache_bucket * -iconv_cache_bucket_new (gchar *key, GIConv cd) -{ - struct _iconv_cache_bucket *bucket; - - bucket = g_new (struct _iconv_cache_bucket, 1); - bucket->key = key; - bucket->refcount = 1; - bucket->used = TRUE; - bucket->cd = cd; - - g_hash_table_insert (iconv_cache, bucket->key, bucket); - - /* FIXME: if we sorted the list so items with few refcounts were - first, then we could expire them faster in iconv_cache_expire_unused () */ - iconv_cache_list = g_list_prepend (iconv_cache_list, bucket); - - iconv_cache_size++; - - return bucket; -} - - -/* - * iconv_cache_bucket_expire: - * @node: cache bucket's node - * @bucket: cache bucket - * - * Expires a single cache bucket @bucket. This should only ever be - * called on a bucket that currently has no used iconv descriptors - * open. - * - * @node is not a required argument. If @node is not supplied, we - * search for it ourselves. - **/ -static void -iconv_cache_bucket_expire (GList *node, struct _iconv_cache_bucket *bucket) -{ - g_hash_table_remove (iconv_cache, bucket->key); - - if (node == NULL) - node = g_list_find (iconv_cache_list, bucket); - - g_assert (node != NULL); - - if (node->prev) - { - node->prev->next = node->next; - if (node->next) - node->next->prev = node->prev; - } - else - { - iconv_cache_list = node->next; - if (node->next) - node->next->prev = NULL; - } - - g_list_free_1 (node); - - g_free (bucket->key); - g_iconv_close (bucket->cd); - g_free (bucket); - - iconv_cache_size--; -} - - -/* - * iconv_cache_expire_unused: - * - * Expires as many unused cache buckets as it needs to in order to get - * the total number of buckets < ICONV_CACHE_SIZE. - **/ -static void -iconv_cache_expire_unused (void) -{ - struct _iconv_cache_bucket *bucket; - GList *node, *next; - - node = iconv_cache_list; - while (node && iconv_cache_size >= ICONV_CACHE_SIZE) - { - next = node->next; - - bucket = node->data; - if (bucket->refcount == 0) - iconv_cache_bucket_expire (node, bucket); - - node = next; - } -} - -static GIConv -open_converter (const gchar *to_codeset, - const gchar *from_codeset, - GError **error) -{ - struct _iconv_cache_bucket *bucket; - gchar *key, *dyn_key, auto_key[80]; - GIConv cd; - gsize len_from_codeset, len_to_codeset; - - /* create our key */ - len_from_codeset = strlen (from_codeset); - len_to_codeset = strlen (to_codeset); - if (len_from_codeset + len_to_codeset + 2 < sizeof (auto_key)) - { - key = auto_key; - dyn_key = NULL; - } - else - key = dyn_key = g_malloc (len_from_codeset + len_to_codeset + 2); - memcpy (key, from_codeset, len_from_codeset); - key[len_from_codeset] = ':'; - strcpy (key + len_from_codeset + 1, to_codeset); - - G_LOCK (iconv_cache_lock); - - /* make sure the cache has been initialized */ - iconv_cache_init (); - - bucket = g_hash_table_lookup (iconv_cache, key); - if (bucket) - { - g_free (dyn_key); - - if (bucket->used) - { - cd = g_iconv_open (to_codeset, from_codeset); - if (cd == (GIConv) -1) - goto error; - } - else - { - /* Apparently iconv on Solaris <= 7 segfaults if you pass in - * NULL for anything but inbuf; work around that. (NULL outbuf - * or NULL *outbuf is allowed by Unix98.) - */ - gsize inbytes_left = 0; - gchar *outbuf = NULL; - gsize outbytes_left = 0; - - cd = bucket->cd; - bucket->used = TRUE; - - /* reset the descriptor */ - g_iconv (cd, NULL, &inbytes_left, &outbuf, &outbytes_left); - } - - bucket->refcount++; - } - else - { - cd = g_iconv_open (to_codeset, from_codeset); - if (cd == (GIConv) -1) - { - g_free (dyn_key); - goto error; - } - - iconv_cache_expire_unused (); - - bucket = iconv_cache_bucket_new (dyn_key ? dyn_key : g_strdup (key), cd); - } - - g_hash_table_insert (iconv_open_hash, cd, bucket->key); - - G_UNLOCK (iconv_cache_lock); - - return cd; - - error: - - G_UNLOCK (iconv_cache_lock); - - /* Something went wrong. */ - if (error) - { - if (errno == EINVAL) - g_set_error (error, G_CONVERT_ERROR, G_CONVERT_ERROR_NO_CONVERSION, - _("Conversion from character set '%s' to '%s' is not supported"), - from_codeset, to_codeset); - else - g_set_error (error, G_CONVERT_ERROR, G_CONVERT_ERROR_FAILED, - _("Could not open converter from '%s' to '%s'"), - from_codeset, to_codeset); - } - - return cd; -} - -static int -close_converter (GIConv converter) -{ - struct _iconv_cache_bucket *bucket; - const gchar *key; - GIConv cd; - - cd = converter; - - if (cd == (GIConv) -1) - return 0; - - G_LOCK (iconv_cache_lock); - - key = g_hash_table_lookup (iconv_open_hash, cd); - if (key) - { - g_hash_table_remove (iconv_open_hash, cd); - - bucket = g_hash_table_lookup (iconv_cache, key); - g_assert (bucket); - - bucket->refcount--; - - if (cd == bucket->cd) - bucket->used = FALSE; - else - g_iconv_close (cd); - - if (!bucket->refcount && iconv_cache_size > ICONV_CACHE_SIZE) - { - /* expire this cache bucket */ - iconv_cache_bucket_expire (NULL, bucket); - } - } - else - { - G_UNLOCK (iconv_cache_lock); - - g_warning ("This iconv context wasn't opened using open_converter"); - - return g_iconv_close (converter); - } - - G_UNLOCK (iconv_cache_lock); - - return 0; -} - -#else /* !NEED_ICONV_CACHE */ - static GIConv open_converter (const gchar *to_codeset, const gchar *from_codeset, @@ -660,8 +365,6 @@ close_converter (GIConv cd) return g_iconv_close (cd); } -#endif /* NEED_ICONV_CACHE */ - /** * g_convert_with_iconv: * @str: the string to convert