Cache XParseColor results in the X display info structure.

With repeated lookups of foreground and background colors for multiple
faces per frame, we issue a lot of redundant color name lookups to the
X server, waiting every time for the response.  On a remote network
with, say, 30ms round-trip time, this can add nearly a full second to
creation of a new frame.

* src/gtkutil.c (xg_check_special_colors): Call x_parse_color.
* src/image.c (get_spec_bg_or_alpha_as_argb):
(xpm_init_color_cache, xpm_lookup_color):
* src/xfns.c (x_defined_color):
* src/xterm.c (x_parse_color): New function; caches color names not
starting with "#" in the display-info structure.
(x_delete_display): Delete the cache content.
* src/xterm.h (struct color_name_cache_entry): New type.
(x_parse_color): Declare.
(struct x_display_info): Add a new field for the cache.

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 725e3306c82515567aea6c30b057145bdace0fdf..34e81b5c104e20b23f3dae0c991b317f5d70b1bd 100644 (file)
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -583,9 +583,7 @@ xg_check_special_colors (struct frame *f,
              (unsigned) (col.red * 65535),
              (unsigned) (col.green * 65535),
              (unsigned) (col.blue * 65535));
-    success_p = (XParseColor (FRAME_X_DISPLAY (f), FRAME_X_COLORMAP (f),
-                             buf, color)
-                != 0);
+    success_p = x_parse_color (f, buf, color) != 0;
 #else
     GtkStyle *gsty = gtk_widget_get_style (FRAME_GTK_WIDGET (f));
     GdkColor *grgb = get_bg

diff --git a/src/image.c b/src/image.c
index 10b067f889c36a0e929f823441fd41ffa0c33862..79bf21e88651e125f1a2807f6e49f62a78dee59e 100644 (file)
--- a/src/image.c
+++ b/src/image.c
@@ -1108,10 +1108,7 @@ get_spec_bg_or_alpha_as_argb (struct image *img,
   XColor xbgcolor;
   Lisp_Object bg = image_spec_value (img->spec, QCbackground, NULL);
 
-  if (STRINGP (bg) && XParseColor (FRAME_X_DISPLAY (f),
-                                   FRAME_X_COLORMAP (f),
-                                   SSDATA (bg),
-                                   &xbgcolor))
+  if (STRINGP (bg) && x_parse_color (f, SSDATA (bg), &xbgcolor))
     bgcolor = xcolor_to_argb32 (xbgcolor);
 
   return bgcolor;
@@ -3241,7 +3238,10 @@ static struct xpm_cached_color *xpm_cache_color (struct frame *, char *,
 /* An entry in a hash table used to cache color definitions of named
    colors.  This cache is necessary to speed up XPM image loading in
    case we do color allocations ourselves.  Without it, we would need
-   a call to XParseColor per pixel in the image.  */
+   a call to XParseColor per pixel in the image.
+
+   FIXME Now that we're using x_parse_color and its cache, reevaluate
+   the need for this caching layer.  */
 
 struct xpm_cached_color
 {
@@ -3276,8 +3276,7 @@ xpm_init_color_cache (struct frame *f, XpmAttributes *attrs)
       XColor color;
 
       for (i = 0; i < attrs->numsymbols; ++i)
-       if (XParseColor (FRAME_X_DISPLAY (f), FRAME_X_COLORMAP (f),
-                        attrs->colorsymbols[i].value, &color))
+       if (x_parse_color (f, attrs->colorsymbols[i].value, &color))
          {
            color.pixel = lookup_rgb_color (f, color.red, color.green,
                                            color.blue);
@@ -3356,8 +3355,7 @@ xpm_lookup_color (struct frame *f, char *color_name, XColor *color)
 
   if (p != NULL)
     *color = p->color;
-  else if (XParseColor (FRAME_X_DISPLAY (f), FRAME_X_COLORMAP (f),
-                       color_name, color))
+  else if (x_parse_color (f, color_name, color))
     {
       color->pixel = lookup_rgb_color (f, color->red, color->green,
                                       color->blue);

diff --git a/src/xfns.c b/src/xfns.c
index fc6111c4fabdc8e59db59a100569eee46702a1ca..7a236e7d6b98edd82da725c8f6e4b84144a5007a 100644 (file)
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -454,7 +454,7 @@ x_defined_color (struct frame *f, const char *color_name,
   success_p = xg_check_special_colors (f, color_name, color);
 #endif
   if (!success_p)
-    success_p = XParseColor (dpy, cmap, color_name, color) != 0;
+    success_p = x_parse_color (f, color_name, color) != 0;
   if (success_p && alloc_p)
     success_p = x_alloc_nearest_color (f, cmap, color);
   unblock_input ();

diff --git a/src/xterm.c b/src/xterm.c
index dd54552510c9b336e88fdbbaac53b32eeaeed46c..6065f964063c8e6d63d713ed50f06391ad8b9a05 100644 (file)
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2223,6 +2223,52 @@ x_query_color (struct frame *f, XColor *color)
 }
 
 
+/* On frame F, translate the color name to RGB values.  Use cached
+   information, if possible.
+
+   Note that there is currently no way to clean old entries out of the
+   cache.  However, it is limited to names in the server's database,
+   and names we've actually looked up; list-colors-display is probably
+   the most color-intensive case we're likely to hit.  */
+
+Status x_parse_color (struct frame *f, const char *color_name,
+                     XColor *color)
+{
+  Display *dpy = FRAME_X_DISPLAY (f);
+  Colormap cmap = FRAME_X_COLORMAP (f);
+  Status status;
+  struct color_name_cache_entry *cache_entry;
+
+  if (color_name[0] == '#')
+    {
+      /* The hex form is parsed directly by XParseColor without
+        talking to the X server.  No need for caching.  */
+      return XParseColor (dpy, cmap, color_name, color);
+    }
+
+  for (cache_entry = FRAME_DISPLAY_INFO (f)->color_names; cache_entry;
+       cache_entry = cache_entry->next)
+    {
+      if (!xstrcasecmp(cache_entry->name, color_name))
+       {
+         *color = cache_entry->rgb;
+         return 1;
+       }
+    }
+
+  if (XParseColor (dpy, cmap, color_name, color) == 0)
+    /* No caching of negative results, currently.  */
+    return 0;
+
+  cache_entry = xzalloc (sizeof *cache_entry);
+  cache_entry->rgb = *color;
+  cache_entry->name = xstrdup (color_name);
+  cache_entry->next = FRAME_DISPLAY_INFO (f)->color_names;
+  FRAME_DISPLAY_INFO (f)->color_names = cache_entry;
+  return 1;
+}
+
+
 /* Allocate the color COLOR->pixel on DISPLAY, colormap CMAP.  If an
    exact match can't be allocated, try the nearest color available.
    Value is true if successful.  Set *COLOR to the color
@@ -12119,6 +12165,7 @@ static void
 x_delete_display (struct x_display_info *dpyinfo)
 {
   struct terminal *t;
+  struct color_name_cache_entry *color_entry, *next_color_entry;
 
   /* Close all frames and delete the generic struct terminal for this
      X display.  */
@@ -12148,6 +12195,15 @@ x_delete_display (struct x_display_info *dpyinfo)
          tail->next = tail->next->next;
     }
 
+  for (color_entry = dpyinfo->color_names;
+       color_entry;
+       color_entry = next_color_entry)
+    {
+      next_color_entry = color_entry->next;
+      xfree (color_entry->name);
+      xfree (color_entry);
+    }
+
   xfree (dpyinfo->x_id_name);
   xfree (dpyinfo->x_dnd_atoms);
   xfree (dpyinfo->color_cells);

diff --git a/src/xterm.h b/src/xterm.h
index 61659066809d7fdd7b04b004c3c0513b12773164..300a8b769f212651d5db98c1a4760c3b286acd60 100644 (file)
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -152,6 +152,17 @@ struct x_gc_ext_data
 };
 #endif
 
+\f
+struct color_name_cache_entry
+{
+  struct color_name_cache_entry *next;
+  XColor rgb;
+  char *name;
+};
+
+Status x_parse_color (struct frame *f, const char *color_name,
+                     XColor *color);
+
 \f
 /* For each X display, we have a structure that records
    information about it.  */
@@ -385,6 +396,9 @@ struct x_display_info
   struct xim_inst_t *xim_callback_data;
 #endif
 
+  /* A cache mapping color names to RGB values.  */
+  struct color_name_cache_entry *color_names;
+
   /* If non-null, a cache of the colors in the color map.  Don't
      use this directly, call x_color_cells instead.  */
   XColor *color_cells;