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.

5 files changed, 79 insertions, 13 deletions

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 725e3306c82..34e81b5c104 100644
--- 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 10b067f889c..79bf21e8865 100644
--- 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 fc6111c4fab..7a236e7d6b9 100644
--- 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 dd54552510c..6065f964063 100644
--- 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 61659066809..300a8b769f2 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -153,6 +153,17 @@ struct x_gc_ext_data
 #endif
 
 
+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);
+
+
 /* 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;