[Emacs-diffs] master b8eea1d 1/4: Cache XParseColor results in the X display info structure.

[Ken Raeburn]

branch: master
commit b8eea1d7b1485a147f112127c0ca58cb1a0a8ebb
Author: Ken Raeburn <address@hidden>
Commit: Ken Raeburn <address@hidden>

    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.
---
 src/gtkutil.c |    4 +---
 src/image.c   |   16 +++++++---------
 src/xfns.c    |    2 +-
 src/xterm.c   |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/xterm.h   |   14 ++++++++++++++
 5 files changed, 79 insertions(+), 13 deletions(-)

diff --git a/src/gtkutil.c b/src/gtkutil.c
index 725e330..34e81b5 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 10b067f..79bf21e 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 fc6111c..7a236e7 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 dd54552..6065f96 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 6165906..300a8b7 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;