[xlib] Improve GC caching efficacy

Shrink the overall size of the per-screen GC cache, but allow multiple GCs
per depth, as it quite common to need up to two temporary GCs along some
drawing paths. Decrease the number of GCs we obtain in total by returning
clean (i.e. a GC without a clip set) back to the screen pool after use.
Compensate for the increased number of put/get by performing the query
using atomic operations where available. So overall we see a dramatic
reduction on the numbers of XCreateGC and XFreeGC, of even greater benefit
for RENDER-less servers.

3 files changed, 197 insertions, 72 deletions

diff --git a/src/cairo-xlib-private.h b/src/cairo-xlib-private.h
index 4995bc6f..c79617bf 100644
--- a/src/cairo-xlib-private.h
+++ b/src/cairo-xlib-private.h
@@ -103,8 +103,8 @@ struct _cairo_xlib_screen_info {
     cairo_bool_t has_font_options;
     cairo_font_options_t font_options;
 
-    GC gc[9];
-    unsigned int gc_needs_clip_reset;
+    GC gc[4];
+    int gc_depths; /* 4 x uint8_t, high bit == needs reset */
 
     cairo_array_t visuals;
 };
@@ -154,12 +154,13 @@ _cairo_xlib_screen_info_close_display (cairo_xlib_screen_info_t *info);
 
 cairo_private GC
 _cairo_xlib_screen_get_gc (cairo_xlib_screen_info_t *info,
-			   int depth,
+			   unsigned int depth,
+			   Drawable drawable,
 			   unsigned int *need_reset);
 
-cairo_private cairo_status_t
+cairo_private void
 _cairo_xlib_screen_put_gc (cairo_xlib_screen_info_t *info,
-			   int depth,
+			   unsigned int depth,
 			   GC gc,
 			   cairo_bool_t reset_clip);
 
diff --git a/src/cairo-xlib-screen.c b/src/cairo-xlib-screen.c
index 8f1e9497..e491a333 100644
--- a/src/cairo-xlib-screen.c
+++ b/src/cairo-xlib-screen.c
@@ -268,19 +268,29 @@ void
 _cairo_xlib_screen_info_close_display (cairo_xlib_screen_info_t *info)
 {
     cairo_xlib_visual_info_t **visuals;
-    int i;
+    Display *dpy;
+    int i, old;
 
     CAIRO_MUTEX_LOCK (info->mutex);
+
+    dpy = info->display->display;
+
+#if HAS_ATOMIC_OPS
+    do {
+	old = info->gc_depths;
+    } while (_cairo_atomic_int_cmpxchg (&info->gc_depths, old, 0) != old);
+#else
+    old = info->gc_depths;
+#endif
+
     for (i = 0; i < ARRAY_LENGTH (info->gc); i++) {
-	if (info->gc[i] != NULL) {
-	    XFreeGC (info->display->display, info->gc[i]);
-	    info->gc[i] = NULL;
-	}
+	if (old >> (8*i) & 0x7f)
+	    XFreeGC (dpy, info->gc[i]);
     }
 
     visuals = _cairo_array_index (&info->visuals, 0);
     for (i = 0; i < _cairo_array_num_elements (&info->visuals); i++)
-	_cairo_xlib_visual_info_destroy (info->display->display, visuals[i]);
+	_cairo_xlib_visual_info_destroy (dpy, visuals[i]);
     _cairo_array_truncate (&info->visuals, 0);
 
     CAIRO_MUTEX_UNLOCK (info->mutex);
@@ -358,8 +368,8 @@ _cairo_xlib_screen_info_get (cairo_xlib_display_t *display,
 	info->screen = screen;
 	info->has_render = FALSE;
 	info->has_font_options = FALSE;
+	info->gc_depths = 0;
 	memset (info->gc, 0, sizeof (info->gc));
-	info->gc_needs_clip_reset = 0;
 
 	_cairo_array_init (&info->visuals,
 			   sizeof (cairo_xlib_visual_info_t*));
@@ -386,71 +396,165 @@ _cairo_xlib_screen_info_get (cairo_xlib_display_t *display,
     return CAIRO_STATUS_SUCCESS;
 }
 
-static int
-depth_to_index (int depth)
+#if HAS_ATOMIC_OPS
+GC
+_cairo_xlib_screen_get_gc (cairo_xlib_screen_info_t *info,
+			   unsigned int depth,
+			   Drawable drawable,
+			   unsigned int *dirty)
 {
-    switch(depth){
-	case 1:  return 1;
-	case 8:  return 2;
-	case 12: return 3;
-	case 15: return 4;
-	case 16: return 5;
-	case 24: return 6;
-	case 30: return 7;
-	case 32: return 8;
+    XGCValues gcv;
+    int i, new, old;
+    GC gc;
+
+    do {
+	gc = NULL;
+	old = info->gc_depths;
+	if (old == 0)
+	    break;
+
+	if (((old >> 0) & 0x7f) == depth)
+	    i = 0;
+	else if (((old >> 8) & 0x7f) == depth)
+	    i = 1;
+	else if (((old >> 16) & 0x7f) == depth)
+	    i = 2;
+	else if (((old >> 24) & 0x7f) == depth)
+	    i = 3;
+	else
+	    break;
+
+	gc = info->gc[i];
+	new = old & ~(0xff << (8*i));
+    } while (_cairo_atomic_int_cmpxchg (&info->gc_depths, old, new) != old);
+
+    if (likely (gc != NULL)) {
+	(void) _cairo_atomic_ptr_cmpxchg (&info->gc[i], gc, NULL);
+
+	if (old & 0x80 << (8 * i))
+	    *dirty |= CAIRO_XLIB_SURFACE_CLIP_DIRTY_GC;
+
+	return gc;
     }
-    return 0;
+
+    gcv.graphics_exposures = False;
+    return XCreateGC (info->display->display, drawable,
+		      GCGraphicsExposures, &gcv);
 }
 
+void
+_cairo_xlib_screen_put_gc (cairo_xlib_screen_info_t *info,
+			   unsigned int depth,
+			   GC gc,
+			   cairo_bool_t reset_clip)
+{
+    int i, old, new;
+
+    depth |= reset_clip ? 0x80 : 0;
+    do {
+	do {
+	    i = -1;
+	    old = info->gc_depths;
+
+	    if (((old >> 0) & 0x7f) == 0)
+		i = 0;
+	    else if (((old >> 8) & 0x7f) == 0)
+		i = 1;
+	    else if (((old >> 16) & 0x7f) == 0)
+		i = 2;
+	    else if (((old >> 24) & 0x7f) == 0)
+		i = 3;
+	    else
+		goto out;
+
+	    new = old | (depth << (8*i));
+	} while (_cairo_atomic_ptr_cmpxchg (&info->gc[i], NULL, gc) != NULL);
+    } while (_cairo_atomic_int_cmpxchg (&info->gc_depths, old, new) != old);
+
+    return;
+
+out:
+    if (unlikely (_cairo_xlib_display_queue_work (info->display,
+				(cairo_xlib_notify_func) XFreeGC,
+				gc,
+				NULL)))
+    {
+	/* leak the server side resource... */
+	XFree ((char *) gc);
+    }
+}
+#else
 GC
 _cairo_xlib_screen_get_gc (cairo_xlib_screen_info_t *info,
-			   int depth,
+			   unsigned int depth,
+			   Drawable drawable,
 			   unsigned int *dirty)
 {
-    GC gc;
-    cairo_bool_t needs_reset;
-
-    depth = depth_to_index (depth);
+    GC gc = NULL;
+    int i;
 
     CAIRO_MUTEX_LOCK (info->mutex);
-    gc = info->gc[depth];
-    info->gc[depth] = NULL;
-    needs_reset = info->gc_needs_clip_reset & (1 << depth);
-    info->gc_needs_clip_reset &= ~(1 << depth);
+    for (i = 0; i < ARRAY_LENGTH (info->gc); i++) {
+	if (((info->gc_depths >> (8*i)) & 0x7f) == depth) {
+	    if (info->gc_depths & 0x80 << (8*i))
+		*dirty |= CAIRO_XLIB_SURFACE_CLIP_DIRTY_GC;
+
+	    info->gc_depths &= ~(0xff << (8*i));
+	    gc = info->gc[i];
+	    break;
+	}
+    }
     CAIRO_MUTEX_UNLOCK (info->mutex);
 
-    if (needs_reset)
-	*dirty |= CAIRO_XLIB_SURFACE_CLIP_DIRTY_GC;
+    if (gc == NULL) {
+	XGCValues gcv;
+
+	gcv.graphics_exposures = False;
+	gc = XCreateGC (info->display->display, drawable,
+			GCGraphicsExposures, &gcv);
+    }
 
     return gc;
 }
 
-cairo_status_t
-_cairo_xlib_screen_put_gc (cairo_xlib_screen_info_t *info, int depth, GC gc, cairo_bool_t reset_clip)
+void
+_cairo_xlib_screen_put_gc (cairo_xlib_screen_info_t *info,
+			   unsigned int depth,
+			   GC gc,
+			   cairo_bool_t reset_clip)
 {
-    cairo_status_t status = CAIRO_STATUS_SUCCESS;
-    GC oldgc;
+    int i;
 
-    depth = depth_to_index (depth);
+    depth |= reset_clip ? 0x80 : 0;
 
     CAIRO_MUTEX_LOCK (info->mutex);
-    oldgc = info->gc[depth];
-    info->gc[depth] = gc;
-    if (reset_clip)
-	info->gc_needs_clip_reset |= 1 << depth;
-    else
-	info->gc_needs_clip_reset &= ~(1 << depth);
-    CAIRO_MUTEX_UNLOCK (info->mutex);
+    for (i = 0; i < ARRAY_LENGTH (info->gc); i++) {
+	if (((info->gc_depths >> (8*i)) & 0x7f) == 0)
+	    break;
+    }
 
-    if (oldgc != NULL) {
-	status = _cairo_xlib_display_queue_work (info->display,
-		                               (cairo_xlib_notify_func) XFreeGC,
-					       oldgc,
-					       NULL);
+    if (i == ARRAY_LENGTH (info->gc)) {
+	cairo_status_t status;
+
+	/* perform random substitution to ensure fair caching over depths */
+	i = rand () % ARRAY_LENGTH (info->gc);
+	status =
+	    _cairo_xlib_display_queue_work (info->display,
+					    (cairo_xlib_notify_func) XFreeGC,
+					    info->gc[i],
+					    NULL);
+	if (unlikely (status)) {
+	    /* leak the server side resource... */
+	    XFree ((char *) info->gc[i]);
+	}
     }
 
-    return status;
+    info->gc[i] = gc;
+    info->gc_depths &= ~(0xff << (8*i));
+    info->gc_depths |= depth << (8*i);
+    CAIRO_MUTEX_UNLOCK (info->mutex);
 }
+#endif
 
 cairo_status_t
 _cairo_xlib_screen_get_visual_info (cairo_xlib_screen_info_t *info,
diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 4fa5d2a7..b68e6ed6 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -68,6 +68,9 @@ static cairo_status_t
 _cairo_xlib_surface_ensure_gc (cairo_xlib_surface_t *surface);
 
 static void
+_cairo_xlib_surface_maybe_put_gc (cairo_xlib_surface_t *surface);
+
+static void
 _cairo_xlib_surface_ensure_src_picture (cairo_xlib_surface_t *surface);
 
 static void
@@ -305,14 +308,11 @@ _cairo_xlib_surface_finish (void *abstract_surface)
     }
 
     if (surface->gc != NULL) {
-	cairo_status_t status2;
-	status2 = _cairo_xlib_screen_put_gc (surface->screen_info,
-		                             surface->depth,
-				             surface->gc,
-				             surface->gc_has_clip_rects);
+	_cairo_xlib_screen_put_gc (surface->screen_info,
+				   surface->depth,
+				   surface->gc,
+				   surface->gc_has_clip_rects);
 	surface->gc = NULL;
-	if (status == CAIRO_STATUS_SUCCESS)
-	    status = status2;
     }
 
     if (surface->clip_rects != surface->embedded_clip_rects)
@@ -662,8 +662,7 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
 	ximage = NULL;
     }
 
-    if (!ximage)
-    {
+    if (!ximage) {
 
 	/* XGetImage from a window is dangerous because it can
 	 * produce errors if the window is unmapped or partially
@@ -695,9 +694,12 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
 
 	    XFreePixmap (surface->dpy, pixmap);
 	}
+
+	if (!ximage)
+	    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+
+	_cairo_xlib_surface_maybe_put_gc (surface);
     }
-    if (!ximage)
-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
 
     _swap_ximage_to_native (ximage);
 
@@ -902,19 +904,14 @@ _cairo_xlib_surface_ensure_dst_picture (cairo_xlib_surface_t    *surface)
 static cairo_status_t
 _cairo_xlib_surface_ensure_gc (cairo_xlib_surface_t *surface)
 {
-    XGCValues gcv;
 
     if (surface->gc == NULL) {
 	surface->gc = _cairo_xlib_screen_get_gc (surface->screen_info,
 						 surface->depth,
+						 surface->drawable,
 						 &surface->clip_dirty);
-	if (surface->gc == NULL) {
-	    gcv.graphics_exposures = False;
-	    surface->gc = XCreateGC (surface->dpy, surface->drawable,
-				     GCGraphicsExposures, &gcv);
-	    if (unlikely (surface->gc == NULL))
-		return _cairo_error (CAIRO_STATUS_NO_MEMORY);
-	}
+	if (unlikely (surface->gc == NULL))
+	    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
     }
 
     if (surface->clip_dirty & CAIRO_XLIB_SURFACE_CLIP_DIRTY_GC)
@@ -923,6 +920,21 @@ _cairo_xlib_surface_ensure_gc (cairo_xlib_surface_t *surface)
     return CAIRO_STATUS_SUCCESS;
 }
 
+static void
+_cairo_xlib_surface_maybe_put_gc (cairo_xlib_surface_t *surface)
+{
+    /* return the GC back to the common pool if clean */
+
+    if (surface->gc_has_clip_rects)
+	return;
+
+    _cairo_xlib_screen_put_gc (surface->screen_info,
+			       surface->depth,
+			       surface->gc,
+			       FALSE);
+    surface->gc = NULL;
+}
+
 static cairo_status_t
 _draw_image_surface (cairo_xlib_surface_t   *surface,
 		     cairo_image_surface_t  *image,
@@ -1081,6 +1093,8 @@ _draw_image_surface (cairo_xlib_surface_t   *surface,
 	      &ximage, src_x, src_y, dst_x, dst_y,
 	      width, height);
 
+    _cairo_xlib_surface_maybe_put_gc (surface);
+
   BAIL:
     if (own_data)
 	free (ximage.data);
@@ -1870,6 +1884,8 @@ _cairo_xlib_surface_composite (cairo_operator_t		op,
 		   src_y + src_attr.y_offset + ity,
 		   width, height,
 		   dst_x, dst_y);
+
+	_cairo_xlib_surface_maybe_put_gc (dst);
 	break;
 
     case DO_XTILE:
@@ -1896,6 +1912,8 @@ _cairo_xlib_surface_composite (cairo_operator_t		op,
 
 	XFillRectangle (dst->dpy, dst->drawable, dst->gc,
 			dst_x, dst_y, width, height);
+
+	_cairo_xlib_surface_maybe_put_gc (dst);
 	break;
 
     case DO_UNSUPPORTED:
@@ -1969,6 +1987,8 @@ _cairo_xlib_surface_solid_fill_rectangles (cairo_xlib_surface_t    *surface,
 			rects[i].width, rects[i].height);
     }
 
+    _cairo_xlib_surface_maybe_put_gc (surface);
+
   BAIL:
     _cairo_pattern_release_surface (&solid.base, solid_surface, &attrs);