[cairo] Atomic reference counting

Chris Wilson chris at chris-wilson.co.uk
Mon Apr 2 03:53:14 PDT 2007


As has been discussed numerous times on this list (ok, at least twice ;-)
Cairo is suitable to races in it reference counting. This was most
recently exposed by the flawed solid pattern cache since the shared
colours are destroyed and reference with a high frequency.

The solution as proposed on this list, and used elsewhere, is to implement
atomic reference counting along with a strict policy on when references
must be taken.

The first attached patch provides atomic reference counting using
assembly routines from GLib and a fallback mutex implementation. The
second patch uses an atomic pointer to avoid having to take a mutex for
the revived solid pattern cache.
--
Chris Wilson
-------------- next part --------------
>From 8cd9eea7b76bfd491f1f8fc47a2786476f56f3e7 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Mon, 2 Apr 2007 11:17:37 +0100
Subject: [PATCH] Atomic reference counting.

Implement a basic atomic adder and exchange using a global mutex. This
ensures that reference counting is race free on all platforms.

Copy assembly implementations from GLib for ia32 and x86_64 in order to
flesh out the optimized paths for a couple of common platforms.
---
 configure.in               |   38 ++++++++++
 src/Makefile.am            |    2 +
 src/cairo-atomic-private.h |  133 ++++++++++++++++++++++++++++++++++++
 src/cairo-atomic.c         |  162 ++++++++++++++++++++++++++++++++++++++++++++
 src/cairo-clip.c           |   10 ++-
 src/cairo-font.c           |   49 ++++++++------
 src/cairo-ft-font.c        |    3 +-
 src/cairo-mutex-list.h     |    2 +
 src/cairo-pattern.c        |   40 ++++++++---
 src/cairo-scaled-font.c    |   49 ++++++++++----
 src/cairo-surface.c        |   40 ++++++++---
 src/cairo.c                |   38 ++++++++---
 12 files changed, 497 insertions(+), 69 deletions(-)

diff --git a/configure.in b/configure.in
index e885b5c..23fa19e 100644
--- a/configure.in
+++ b/configure.in
@@ -781,6 +781,44 @@ dnl  PHP_ADD_MAKEFILE_FRAGMENT($abs_srcdir/Makefile.gcov, $abs_srcdir)
 fi
 
 dnl ===========================================================================
+dnl Checking what method to use for atomic ops
+AC_MSG_CHECKING([whether to use assembler code for atomic operations])
+if test x"$GCC" = "xyes"; then
+    have_atomic_ops=no
+    need_mb=yes
+    case $host_cpu in
+	i386)
+	    AC_MSG_RESULT([i386 - this processor does not support atomic ops])
+	    need_mb=no
+	    ;;
+	i?86)
+	    AC_MSG_RESULT([ia32])
+	    AC_DEFINE([CAIRO_IA32_ATOMIC_OPS], 1, [ia32 atomic implementation])
+	    have_atomic_ops=yes
+	    need_mb=no
+	    ;;
+	x86_64)
+	    AC_MSG_RESULT([x86_64])
+	    AC_DEFINE([CAIRO_X86_64_ATOMIC_OPS], 1, [x86_64 atomic implementation])
+	    have_atomic_ops=yes
+	    need_mb=no
+	    ;;
+	*)
+	    AC_MSG_RESULT([none])
+	    ;;
+    esac
+
+    if test $have_atomic_ops != no; then
+	AC_DEFINE([CAIRO_HAS_ATOMIC_OPS], 1, [whether Cairo can use atomic ops])
+	AC_DEFINE([CAIRO_INLINE_ATOMIC_OPS], 1, [whether to inline atomic ops])
+    fi
+    if test $need_mb != no; then
+	AC_DEFINE([CAIRO_ATOMIC_OPS_NEED_MB], 1, [whether atomic ops require an explicit memory barrier])
+    fi
+fi
+        
+
+dnl ===========================================================================
 
 AC_ARG_ENABLE(test-surfaces,
   AS_HELP_STRING([--enable-test-surfaces],
diff --git a/src/Makefile.am b/src/Makefile.am
index 73420d4..0f6330a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -166,6 +166,8 @@ libcairo_la_SOURCES =				\
 	cairo-arc.c				\
 	cairo-arc-private.h			\
 	cairo-array.c				\
+	cairo-atomic-private.h			\
+	cairo-atomic.c				\
 	cairo-base85-stream.c			\
 	cairo-bentley-ottmann.c			\
 	cairo-cache.c				\
diff --git a/src/cairo-atomic-private.h b/src/cairo-atomic-private.h
new file mode 100644
index 0000000..7c86769
--- /dev/null
+++ b/src/cairo-atomic-private.h
@@ -0,0 +1,133 @@
+/* -*- Mode: c; c-basic-offset: 4; indent-tabs-mode: t; tab-width: 8; -*- */
+/* cairo - a vector graphics library with display and print output
+ *
+ * This file is adapted from routines taken from GLib, glib/gatomic.c.
+ * Copyright (C) 2003 Sebastian Wilhelmi
+ *
+ * Cairo integration and additional routines,
+ * Copyright (C) 2007 Chris Wilson
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *           
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *  
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ */
+
+#ifndef CAIRO_ATOMIC_PRIVATE_H
+#define CAIRO_ATOMIC_PRIVATE_H
+
+#include "cairoint.h"
+
+#ifndef CAIRO_INLINE_ATOMIC_OPS
+cairo_private int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val);
+
+cairo_private void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val);
+
+cairo_private void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval);
+#elif defined (CAIRO_IA32_ATOMIC_OPS)
+cairo_static __inline__ int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val)
+{
+  int result;
+
+  __asm__ __volatile__ ("lock; xaddl %0,%1"
+                        : "=r" (result), "=m" (*atomic) 
+			: "0" (val), "m" (*atomic));
+  return result;
+}
+ 
+cairo_static __inline__ void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val)
+{
+  __asm__ __volatile__ ("lock; addl %1,%0"
+			: "=m" (*atomic) 
+			: "ir" (val), "m" (*atomic));
+}
+
+cairo_static __inline__ void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval)
+{
+  void *result;
+ 
+  __asm__ __volatile__ ("lock; xchgl %2, %1"
+			: "=a" (result), "=m" (*atomic)
+			: "r" (newval), "m" (*atomic)); 
+
+  return result;
+}
+#elif defined (CAIRO_X86_64_ATOMIC_OPS)
+cairo_static __inline__ int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val)
+{
+  int result;
+
+  __asm__ __volatile__ ("lock; xaddl %0,%1"
+                        : "=r" (result), "=m" (*atomic) 
+			: "0" (val), "m" (*atomic));
+  return result;
+}
+ 
+cairo_static __inline__ void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val)
+{
+  __asm__ __volatile__ ("lock; addl %1,%0"
+			: "=m" (*atomic) 
+			: "ir" (val), "m" (*atomic));
+}
+
+cairo_static __inline__ void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval)
+{
+  void *result;
+ 
+  __asm__ __volatile__ ("lock; xchgq %2, %1"
+			: "=a" (result), "=m" (*atomic)
+			: "r" (newval), "m" (*atomic)); 
+
+  return result;
+}
+#endif
+
+#ifndef CAIRO_ATOMIC_OPS_NEED_MB
+#define cairo_atomic_int_get(atomic)			(*(atomic))
+#define cairo_atomic_int_set(atomic, newval)		(*(atomic) = (newval))
+#else
+cairo_private int
+cairo_atomic_int_get (volatile int *atomic);
+
+cairo_private void
+cairo_atomic_int_set (volatile int *atomic, int newval);
+#endif
+
+#define cairo_ref_count_get(atomic)					\
+    ((unsigned int) cairo_atomic_int_get ((volatile int *)(atomic)))
+#define cairo_ref_count_inc(atomic)					\
+    (cairo_atomic_int_add ((volatile int *)(atomic), 1))
+#define cairo_ref_count_dec(atomic)				\
+    (cairo_atomic_int_exchange_and_add ((volatile int *)(atomic), -1) == 1)
+
+
+#endif /* CAIRO_ATOMIC_PRIVATE_H */
diff --git a/src/cairo-atomic.c b/src/cairo-atomic.c
new file mode 100644
index 0000000..34629b6
--- /dev/null
+++ b/src/cairo-atomic.c
@@ -0,0 +1,162 @@
+/* -*- Mode: c; c-basic-offset: 4; indent-tabs-mode: t; tab-width: 8; -*- */
+/* cairo - a vector graphics library with display and print output
+ *
+ * This file is composed of routines taken from GLib, glib/gatomic.c.
+ * Copyright (C) 2003 Sebastian Wilhelmi
+ *
+ * Cairo integration and additional routines,
+ * Copyright (C) 2007 Chris Wilson
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *           
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *  
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ *
+ */
+
+#include "cairoint.h"
+#include "cairo-atomic-private.h"
+
+/* start by defining generic atomic ops that take a mutex */
+
+#ifndef CAIRO_INLINE_ATOMIC_OPS
+#if defined(CAIRO_IA32_ATOMIC_OPS)
+int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val)
+{
+  int result;
+
+  __asm__ __volatile__ ("lock; xaddl %0,%1"
+                        : "=r" (result), "=m" (*atomic) 
+			: "0" (val), "m" (*atomic));
+  return result;
+}
+ 
+void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val)
+{
+  __asm__ __volatile__ ("lock; addl %1,%0"
+			: "=m" (*atomic) 
+			: "ir" (val), "m" (*atomic));
+}
+
+void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval)
+{
+  void *result;
+ 
+  __asm__ __volatile__ ("lock; xchgl %2, %1"
+			: "=a" (result), "=m" (*atomic)
+			: "r" (newval), "m" (*atomic)); 
+
+  return result;
+}
+#elif defined (CAIRO_X86_64_ATOMIC_OPS)
+int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val)
+{
+  int result;
+
+  __asm__ __volatile__ ("lock; xaddl %0,%1"
+                        : "=r" (result), "=m" (*atomic) 
+			: "0" (val), "m" (*atomic));
+  return result;
+}
+ 
+void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val)
+{
+  __asm__ __volatile__ ("lock; addl %1,%0"
+			: "=m" (*atomic) 
+			: "ir" (val), "m" (*atomic));
+}
+
+void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval)
+{
+  void *result;
+ 
+  __asm__ __volatile__ ("lock; xchgq %2, %1"
+			: "=a" (result), "=m" (*atomic)
+			: "r" (newval), "m" (*atomic)); 
+
+  return result;
+}
+#else /* fallback: use a mutex */
+int
+cairo_atomic_int_exchange_and_add (volatile int  *atomic,
+				   int            val)
+{
+    int result;
+
+    CAIRO_MUTEX_LOCK (_cairo_atomic_lock);
+    result = *atomic;
+    *atomic += val;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_lock);
+
+    return result;
+}
+ 
+void
+cairo_atomic_int_add (volatile int  *atomic, 
+		      int            val)
+{
+    CAIRO_MUTEX_LOCK (_cairo_atomic_lock);
+    *atomic += val;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_lock);
+}
+
+
+void *
+cairo_atomic_pointer_exchange (volatile void  **atomic, 
+			       void            *newval)
+{
+    void *result;
+
+    CAIRO_MUTEX_LOCK (_cairo_atomic_lock);
+    result = *atomic;
+    *atomic = newval;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_lock);
+
+    return result;
+}
+#endif
+#endif
+
+#ifdef CAIRO_ATOMIC_OPS_NEED_MB
+int
+cairo_atomic_int_get (volatile int *atomic)
+{
+    int result;
+
+    CAIRO_MUTEX_LOCK (_cairo_atomic_lock);
+    result = *atomic;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_lock);
+
+    return result;
+}
+
+void
+cairo_atomic_int_set (volatile int *atomic, int newval)
+{
+    CAIRO_MUTEX_LOCK (_cairo_atomic_lock);
+    *atomic = newval;
+    CAIRO_MUTEX_UNLOCK (_cairo_atomic_lock);
+}
+#endif
diff --git a/src/cairo-clip.c b/src/cairo-clip.c
index f425e2b..dc7ba71 100644
--- a/src/cairo-clip.c
+++ b/src/cairo-clip.c
@@ -37,6 +37,7 @@
  */
 
 #include "cairoint.h"
+#include "cairo-atomic-private.h"
 #include "cairo-clip-private.h"
 
 static cairo_clip_path_t *
@@ -324,7 +325,9 @@ _cairo_clip_path_reference (cairo_clip_path_t *clip_path)
     if (clip_path == NULL)
 	return NULL;
 
-    clip_path->ref_count++;
+    assert (cairo_atomic_int_get (&clip_path->ref_count) > 0);
+
+    cairo_ref_count_inc (&clip_path->ref_count);
 
     return clip_path;
 }
@@ -335,8 +338,9 @@ _cairo_clip_path_destroy (cairo_clip_path_t *clip_path)
     if (clip_path == NULL)
 	return;
 
-    clip_path->ref_count--;
-    if (clip_path->ref_count)
+    assert (cairo_atomic_int_get (&clip_path->ref_count) > 0);
+
+    if (! cairo_ref_count_dec (&clip_path->ref_count))
 	return;
 
     _cairo_path_fixed_fini (&clip_path->path);
diff --git a/src/cairo-font.c b/src/cairo-font.c
index f254c3b..8b1baab 100644
--- a/src/cairo-font.c
+++ b/src/cairo-font.c
@@ -39,6 +39,7 @@
  */
 
 #include "cairoint.h"
+#include "cairo-atomic-private.h"
 
 /* Forward declare so we can use it as an arbitrary backend for
  * _cairo_font_face_nil.
@@ -85,18 +86,20 @@ _cairo_font_face_init (cairo_font_face_t               *font_face,
 cairo_font_face_t *
 cairo_font_face_reference (cairo_font_face_t *font_face)
 {
-    if (font_face == NULL || font_face->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned ref_count;
+
+    if (font_face == NULL)
 	return font_face;
 
-    CAIRO_MUTEX_LOCK (_cairo_font_face_mutex);
+    ref_count = cairo_ref_count_get (&font_face->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
+	return font_face;
 
     /* We would normally assert (font_face->ref_count >0) here but we
      * can't get away with that due to the zombie case as documented
      * in _cairo_ft_font_face_destroy. */
 
-    font_face->ref_count++;
-
-    CAIRO_MUTEX_UNLOCK (_cairo_font_face_mutex);
+    cairo_ref_count_inc (&font_face->ref_count);
 
     return font_face;
 }
@@ -113,19 +116,19 @@ slim_hidden_def (cairo_font_face_reference);
 void
 cairo_font_face_destroy (cairo_font_face_t *font_face)
 {
-    if (font_face == NULL || font_face->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned ref_count;
+
+    if (font_face == NULL)
 	return;
 
-    CAIRO_MUTEX_LOCK (_cairo_font_face_mutex);
+    ref_count = cairo_ref_count_get (&font_face->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
+	return;
 
-    assert (font_face->ref_count > 0);
+    assert (ref_count > 0);
 
-    if (--(font_face->ref_count) > 0) {
-        CAIRO_MUTEX_UNLOCK (_cairo_font_face_mutex);
+    if (! cairo_ref_count_dec (&font_face->ref_count))
 	return;
-    }
-
-    CAIRO_MUTEX_UNLOCK (_cairo_font_face_mutex);
 
     font_face->backend->destroy (font_face);
 
@@ -133,7 +136,7 @@ cairo_font_face_destroy (cairo_font_face_t *font_face)
      * FreeType backend where cairo_ft_font_face_t and cairo_ft_unscaled_font_t
      * need to effectively mutually reference each other
      */
-    if (font_face->ref_count > 0)
+    if (cairo_ref_count_get (&font_face->ref_count) > 0)
 	return;
 
     _cairo_user_data_array_fini (&font_face->user_data);
@@ -173,10 +176,16 @@ cairo_font_face_get_type (cairo_font_face_t *font_face)
 unsigned int
 cairo_font_face_get_reference_count (cairo_font_face_t *font_face)
 {
-    if (font_face == NULL || font_face->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (font_face == NULL)
+	return 0;
+
+    ref_count = cairo_ref_count_get (&font_face->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return 0;
 
-    return font_face->ref_count;
+    return ref_count;
 }
 
 /**
@@ -237,7 +246,7 @@ cairo_font_face_set_user_data (cairo_font_face_t	   *font_face,
 			       void			   *user_data,
 			       cairo_destroy_func_t	    destroy)
 {
-    if (font_face->ref_count == CAIRO_REF_COUNT_INVALID)
+    if (cairo_ref_count_get (&font_face->ref_count) == CAIRO_REF_COUNT_INVALID)
 	return CAIRO_STATUS_NO_MEMORY;
 
     return _cairo_user_data_array_set_data (&font_face->user_data,
@@ -395,7 +404,7 @@ _cairo_toy_font_face_create (const char          *family,
     {
 	/* We increment the reference count here manually to avoid
 	   double-locking. */
-	font_face->base.ref_count++;
+	cairo_ref_count_inc (&font_face->base.ref_count);
 	_cairo_toy_font_face_hash_table_unlock ();
 	return &font_face->base;
     }
@@ -480,7 +489,7 @@ _cairo_unscaled_font_reference (cairo_unscaled_font_t *unscaled_font)
     if (unscaled_font == NULL)
 	return NULL;
 
-    unscaled_font->ref_count++;
+    cairo_ref_count_inc (&unscaled_font->ref_count);
 
     return unscaled_font;
 }
@@ -491,7 +500,7 @@ _cairo_unscaled_font_destroy (cairo_unscaled_font_t *unscaled_font)
     if (unscaled_font == NULL)
 	return;
 
-    if (--(unscaled_font->ref_count) > 0)
+    if (! cairo_ref_count_dec (&unscaled_font->ref_count))
 	return;
 
     unscaled_font->backend->destroy (unscaled_font);
diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
index 9c6ed0b..7035e32 100644
--- a/src/cairo-ft-font.c
+++ b/src/cairo-ft-font.c
@@ -39,6 +39,7 @@
 
 #include <float.h>
 
+#include "cairo-atomic-private.h"
 #include "cairo-ft-private.h"
 
 #include <fontconfig/fontconfig.h>
@@ -2169,7 +2170,7 @@ _cairo_ft_font_face_destroy (void *abstract_face)
 
     if (font_face->unscaled &&
 	font_face->unscaled->from_face &&
-	font_face->unscaled->base.ref_count > 1)
+	cairo_ref_count_get (&font_face->unscaled->base.ref_count) > 1)
     {
 	cairo_font_face_reference (&font_face->base);
 
diff --git a/src/cairo-mutex-list.h b/src/cairo-mutex-list.h
index c054e11..97ae3aa 100644
--- a/src/cairo-mutex-list.h
+++ b/src/cairo-mutex-list.h
@@ -34,6 +34,8 @@
 #ifndef CAIRO_MUTEX_LIST_H
 #define CAIRO_MUTEX_LIST_H
 
+CAIRO_MUTEX_DECLARE (_cairo_atomic_lock);
+
 CAIRO_MUTEX_DECLARE (_cairo_pattern_solid_cache_lock);
 
 CAIRO_MUTEX_DECLARE (_cairo_font_face_mutex);
diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index 570527d..fdafc3e 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -29,6 +29,7 @@
  */
 
 #include "cairoint.h"
+#include "cairo-atomic-private.h"
 
 const cairo_solid_pattern_t cairo_pattern_nil = {
     { CAIRO_PATTERN_TYPE_SOLID, 	/* type */
@@ -280,7 +281,7 @@ static struct {
 static cairo_pattern_t *
 _cairo_pattern_create_solid_from_cache (const cairo_color_t *color)
 {
-    cairo_solid_pattern_t *pattern = NULL;
+    cairo_solid_pattern_t *pattern;
 
     CAIRO_MUTEX_LOCK (_cairo_pattern_solid_cache_lock);
 
@@ -569,12 +570,18 @@ cairo_pattern_create_radial (double cx0, double cy0, double radius0,
 cairo_pattern_t *
 cairo_pattern_reference (cairo_pattern_t *pattern)
 {
-    if (pattern == NULL || pattern->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (pattern == NULL)
+	return pattern;
+
+    ref_count = cairo_ref_count_get (&pattern->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return pattern;
 
-    assert (pattern->ref_count > 0);
+    assert (ref_count > 0);
 
-    pattern->ref_count++;
+    cairo_ref_count_inc (&pattern->ref_count);
 
     return pattern;
 }
@@ -626,13 +633,18 @@ slim_hidden_def (cairo_pattern_status);
 void
 cairo_pattern_destroy (cairo_pattern_t *pattern)
 {
-    if (pattern == NULL || pattern->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (pattern == NULL)
 	return;
 
-    assert (pattern->ref_count > 0);
+    ref_count = cairo_ref_count_get (&pattern->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
+	return;
+
+    assert (ref_count > 0);
 
-    pattern->ref_count--;
-    if (pattern->ref_count)
+    if (! cairo_ref_count_dec (&pattern->ref_count))
 	return;
 
     _cairo_pattern_fini (pattern);
@@ -672,10 +684,16 @@ slim_hidden_def (cairo_pattern_destroy);
 unsigned int
 cairo_pattern_get_reference_count (cairo_pattern_t *pattern)
 {
-    if (pattern == NULL || pattern->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (pattern == NULL)
+	return 0;
+
+    ref_count = cairo_ref_count_get (&pattern->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return 0;
 
-    return pattern->ref_count;
+    return ref_count;
 }
 
 /**
@@ -724,7 +742,7 @@ cairo_pattern_set_user_data (cairo_pattern_t		 *pattern,
 			     void			 *user_data,
 			     cairo_destroy_func_t	  destroy)
 {
-    if (pattern->ref_count == CAIRO_REF_COUNT_INVALID)
+    if (cairo_ref_count_get (&pattern->ref_count) == CAIRO_REF_COUNT_INVALID)
 	return CAIRO_STATUS_NO_MEMORY;
 
     return _cairo_user_data_array_set_data (&pattern->user_data,
diff --git a/src/cairo-scaled-font.c b/src/cairo-scaled-font.c
index b904b85..7ecfbe9 100644
--- a/src/cairo-scaled-font.c
+++ b/src/cairo-scaled-font.c
@@ -37,6 +37,7 @@
  */
 
 #include "cairoint.h"
+#include "cairo-atomic-private.h"
 #include "cairo-scaled-font-test.h"
 
 static cairo_bool_t
@@ -242,7 +243,7 @@ _cairo_scaled_font_map_destroy (void)
 	/* We should only get here through the reset_static_data path
 	 * and there had better not be any active references at that
 	 * point. */
-	assert (scaled_font->ref_count == 0);
+	assert (cairo_ref_count_get (&scaled_font->ref_count) == 0);
 	_cairo_hash_table_remove (font_map->hash_table,
 				  &scaled_font->hash_entry);
 	_cairo_scaled_font_fini (scaled_font);
@@ -496,7 +497,7 @@ cairo_scaled_font_create (cairo_font_face_t          *font_face,
 	 * been found in font_map->holdovers, (which means this caching is
 	 * actually working). So now we remove it from the holdovers
 	 * array. */
-	if (scaled_font->ref_count == 0) {
+	if (cairo_ref_count_get (&scaled_font->ref_count) == 0) {
 	    int i;
 
 	    for (i = 0; i < font_map->num_holdovers; i++)
@@ -520,7 +521,7 @@ cairo_scaled_font_create (cairo_font_face_t          *font_face,
 	 * than calling into cairo_scaled_font_reference), since we
 	 * must modify the reference count while our lock is still
 	 * held. */
-	scaled_font->ref_count++;
+	cairo_ref_count_inc (&scaled_font->ref_count);
 	_cairo_scaled_font_map_unlock ();
     } else {
 create_font:
@@ -567,14 +568,21 @@ slim_hidden_def (cairo_scaled_font_create);
 cairo_scaled_font_t *
 cairo_scaled_font_reference (cairo_scaled_font_t *scaled_font)
 {
-    if (scaled_font == NULL || scaled_font->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (scaled_font == NULL)
+	return scaled_font;
+
+    ref_count = cairo_ref_count_get (&scaled_font->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return scaled_font;
 
     _cairo_scaled_font_map_lock ();
     {
-	assert (scaled_font->ref_count > 0);
+	/* double check after waiting for the lock */
+	assert (cairo_ref_count_get (&scaled_font->ref_count) > 0);
 
-	scaled_font->ref_count++;
+	cairo_ref_count_inc (&scaled_font->ref_count);
     }
     _cairo_scaled_font_map_unlock ();
 
@@ -595,17 +603,23 @@ cairo_scaled_font_destroy (cairo_scaled_font_t *scaled_font)
 {
     cairo_scaled_font_map_t *font_map;
     cairo_scaled_font_t *lru = NULL;
+    unsigned int ref_count;
+
+    if (scaled_font == NULL)
+	return;
 
-    if (scaled_font == NULL || scaled_font->ref_count == CAIRO_REF_COUNT_INVALID)
+    ref_count = cairo_ref_count_get (&scaled_font->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return;
 
     font_map = _cairo_scaled_font_map_lock ();
     {
 	assert (font_map != NULL);
 
-	assert (scaled_font->ref_count > 0);
+	/* double check after waiting for the lock */
+	assert (cairo_ref_count_get (&scaled_font->ref_count) > 0);
 
-	if (--(scaled_font->ref_count) == 0)
+	if (cairo_ref_count_dec (&scaled_font->ref_count))
 	{
 	    /* Rather than immediately destroying this object, we put it into
 	     * the font_map->holdovers array in case it will get used again
@@ -615,7 +629,7 @@ cairo_scaled_font_destroy (cairo_scaled_font_t *scaled_font)
 	    if (font_map->num_holdovers == CAIRO_SCALED_FONT_MAX_HOLDOVERS)
 	    {
 		lru = font_map->holdovers[0];
-		assert (lru->ref_count == 0);
+		assert (cairo_ref_count_get (&lru->ref_count) == 0);
 
 		_cairo_hash_table_remove (font_map->hash_table, &lru->hash_entry);
 
@@ -658,10 +672,16 @@ slim_hidden_def (cairo_scaled_font_destroy);
 unsigned int
 cairo_scaled_font_get_reference_count (cairo_scaled_font_t *scaled_font)
 {
-    if (scaled_font == NULL || scaled_font->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (scaled_font == NULL)
+	return 0;
+
+    ref_count = cairo_ref_count_get (&scaled_font->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return 0;
 
-    return scaled_font->ref_count;
+    return ref_count;
 }
 
 /**
@@ -710,7 +730,10 @@ cairo_scaled_font_set_user_data (cairo_scaled_font_t	     *scaled_font,
 				 void			     *user_data,
 				 cairo_destroy_func_t	      destroy)
 {
-    if (scaled_font->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    ref_count = cairo_ref_count_get (&scaled_font->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return CAIRO_STATUS_NO_MEMORY;
 
     return _cairo_user_data_array_set_data (&scaled_font->user_data,
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index aa4f3b8..0a497eb 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -39,6 +39,7 @@
 
 #include "cairoint.h"
 #include "cairo-surface-fallback-private.h"
+#include "cairo-atomic-private.h"
 #include "cairo-clip-private.h"
 
 #define DEFINE_NIL_SURFACE(status, name)			\
@@ -361,12 +362,18 @@ _cairo_surface_get_clip_mode (cairo_surface_t *surface)
 cairo_surface_t *
 cairo_surface_reference (cairo_surface_t *surface)
 {
-    if (surface == NULL || surface->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (surface == NULL)
+	return surface;
+
+    ref_count = cairo_ref_count_get (&surface->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return surface;
 
-    assert (surface->ref_count > 0);
+    assert (ref_count > 0);
 
-    surface->ref_count++;
+    cairo_ref_count_inc (&surface->ref_count);
 
     return surface;
 }
@@ -383,13 +390,18 @@ slim_hidden_def (cairo_surface_reference);
 void
 cairo_surface_destroy (cairo_surface_t *surface)
 {
-    if (surface == NULL || surface->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (surface == NULL)
 	return;
 
-    assert (surface->ref_count > 0);
+    ref_count = cairo_ref_count_get (&surface->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
+	return;
+
+    assert (ref_count > 0);
 
-    surface->ref_count--;
-    if (surface->ref_count)
+    if (! cairo_ref_count_dec (&surface->ref_count))
 	return;
 
     if (! surface->finished)
@@ -415,10 +427,16 @@ slim_hidden_def(cairo_surface_destroy);
 unsigned int
 cairo_surface_get_reference_count (cairo_surface_t *surface)
 {
-    if (surface == NULL || surface->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (surface == NULL)
+	return 0;
+
+    ref_count = cairo_ref_count_get (&surface->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return 0;
 
-    return surface->ref_count;
+    return ref_count;
 }
 
 /**
@@ -447,7 +465,7 @@ cairo_surface_finish (cairo_surface_t *surface)
     if (surface == NULL)
 	return;
 
-    if (surface->ref_count == CAIRO_REF_COUNT_INVALID)
+    if (cairo_ref_count_get (&surface->ref_count) == CAIRO_REF_COUNT_INVALID)
 	return;
 
     if (surface->finished) {
@@ -520,7 +538,7 @@ cairo_surface_set_user_data (cairo_surface_t		 *surface,
 			     void			 *user_data,
 			     cairo_destroy_func_t	 destroy)
 {
-    if (surface->ref_count == CAIRO_REF_COUNT_INVALID)
+    if (cairo_ref_count_get (&surface->ref_count) == CAIRO_REF_COUNT_INVALID)
 	return CAIRO_STATUS_NO_MEMORY;
 
     return _cairo_user_data_array_set_data (&surface->user_data,
diff --git a/src/cairo.c b/src/cairo.c
index 4455e8f..d9e1f83 100644
--- a/src/cairo.c
+++ b/src/cairo.c
@@ -40,6 +40,7 @@
 #include "cairo-private.h"
 
 #include "cairo-arc-private.h"
+#include "cairo-atomic-private.h"
 #include "cairo-path-private.h"
 
 #define CAIRO_TOLERANCE_MINIMUM	0.0002 /* We're limited by 16 bits of sub-pixel precision */
@@ -234,12 +235,18 @@ slim_hidden_def (cairo_create);
 cairo_t *
 cairo_reference (cairo_t *cr)
 {
-    if (cr == NULL || cr->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (cr == NULL)
+	return cr;
+
+    ref_count = cairo_ref_count_get (&cr->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return cr;
 
-    assert (cr->ref_count > 0);
+    assert (ref_count > 0);
 
-    cr->ref_count++;
+    cairo_ref_count_inc (&cr->ref_count);
 
     return cr;
 }
@@ -255,13 +262,18 @@ cairo_reference (cairo_t *cr)
 void
 cairo_destroy (cairo_t *cr)
 {
-    if (cr == NULL || cr->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (cr == NULL)
 	return;
 
-    assert (cr->ref_count > 0);
+    ref_count = cairo_ref_count_get (&cr->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
+	return;
+
+    assert (ref_count > 0);
 
-    cr->ref_count--;
-    if (cr->ref_count)
+    if (! cairo_ref_count_dec (&cr->ref_count))
 	return;
 
     while (cr->gstate != cr->gstate_tail) {
@@ -325,7 +337,7 @@ cairo_set_user_data (cairo_t			 *cr,
 		     void			 *user_data,
 		     cairo_destroy_func_t	 destroy)
 {
-    if (cr->ref_count == CAIRO_REF_COUNT_INVALID)
+    if (cairo_ref_count_get (&cr->ref_count) == CAIRO_REF_COUNT_INVALID)
 	return CAIRO_STATUS_NO_MEMORY;
 
     return _cairo_user_data_array_set_data (&cr->user_data,
@@ -346,10 +358,16 @@ cairo_set_user_data (cairo_t			 *cr,
 unsigned int
 cairo_get_reference_count (cairo_t *cr)
 {
-    if (cr == NULL || cr->ref_count == CAIRO_REF_COUNT_INVALID)
+    unsigned int ref_count;
+
+    if (cr == NULL)
+	return 0;
+
+    ref_count = cairo_ref_count_get (&cr->ref_count);
+    if (ref_count == CAIRO_REF_COUNT_INVALID)
 	return 0;
 
-    return cr->ref_count;
+    return ref_count;
 }
 
 /**
-- 
1.4.4.2

-------------- next part --------------
>From f592c0a5151a790349228a76071f7fc76f9a86e5 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris at chris-wilson.co.uk>
Date: Mon, 2 Apr 2007 11:37:43 +0100
Subject: [PATCH] Use a single atomic solid pattern cache.

Avoid taking a slow mutex for the common case of immediate pattern
reuse by keeping the MRU pattern in an atomic pointer.
---
 src/cairo-pattern.c |   41 +++++++++++++++++++++++++++++++----------
 1 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/cairo-pattern.c b/src/cairo-pattern.c
index fdafc3e..2224d79 100644
--- a/src/cairo-pattern.c
+++ b/src/cairo-pattern.c
@@ -277,12 +277,23 @@ static struct {
     cairo_solid_pattern_t *patterns[MAX_PATTERN_CACHE_SIZE];
     int size;
 } solid_pattern_cache;
+#ifdef CAIRO_HAS_ATOMIC_OPS
+static cairo_solid_pattern_t *last_solid_pattern;
+#define xchg_last_solid_pattern(newval) \
+    cairo_atomic_pointer_exchange ((volatile void **)&last_solid_pattern, newval)
+#else
+#define xchg_last_solid_pattern(newval) NULL
+#endif
 
 static cairo_pattern_t *
 _cairo_pattern_create_solid_from_cache (const cairo_color_t *color)
 {
     cairo_solid_pattern_t *pattern;
 
+    pattern = xchg_last_solid_pattern (NULL);
+    if (pattern != NULL)
+	goto DONE;
+
     CAIRO_MUTEX_LOCK (_cairo_pattern_solid_cache_lock);
 
     if (solid_pattern_cache.size) {
@@ -298,8 +309,10 @@ _cairo_pattern_create_solid_from_cache (const cairo_color_t *color)
 	/* None cached, need to create a new pattern. */
 	pattern = malloc (sizeof (cairo_solid_pattern_t));
     }
-    if (pattern != NULL)
+    if (pattern != NULL) {
+DONE:
 	_cairo_pattern_init_solid (pattern, color);
+    }
 
     return &pattern->base;
 }
@@ -319,6 +332,7 @@ _cairo_pattern_create_solid (const cairo_color_t *color)
 void
 _cairo_pattern_reset_static_data (void)
 {
+    cairo_solid_pattern_t *pattern;
     int i;
 
     CAIRO_MUTEX_LOCK (_cairo_pattern_solid_cache_lock);
@@ -330,6 +344,10 @@ _cairo_pattern_reset_static_data (void)
     solid_pattern_cache.size = 0;
 
     CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_cache_lock);
+
+    pattern = xchg_last_solid_pattern (NULL);
+    if (pattern != NULL)
+	free (pattern);
 }
 
 static const cairo_pattern_t *
@@ -651,19 +669,22 @@ cairo_pattern_destroy (cairo_pattern_t *pattern)
 
     /* maintain a small cache of freed patterns */
     if (pattern->type == CAIRO_PATTERN_TYPE_SOLID) {
-	int i;
+	pattern = xchg_last_solid_pattern (pattern);
+	if (pattern != NULL) {
+	    int i;
 
-	CAIRO_MUTEX_LOCK (_cairo_pattern_solid_cache_lock);
+	    CAIRO_MUTEX_LOCK (_cairo_pattern_solid_cache_lock);
 
-	i = solid_pattern_cache.size++ %
-	    ARRAY_LEN (solid_pattern_cache.patterns);
-	/* swap an old pattern for this 'cache-hot' pattern */
-	if (solid_pattern_cache.patterns[i])
-	    free (solid_pattern_cache.patterns[i]);
+	    i = solid_pattern_cache.size++ %
+		ARRAY_LEN (solid_pattern_cache.patterns);
+	    /* swap an old pattern for this 'cache-hot' pattern */
+	    if (solid_pattern_cache.patterns[i])
+		free (solid_pattern_cache.patterns[i]);
 
-	solid_pattern_cache.patterns[i] = (cairo_solid_pattern_t *) pattern;
+	    solid_pattern_cache.patterns[i] = (cairo_solid_pattern_t *) pattern;
 
-	CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_cache_lock);
+	    CAIRO_MUTEX_UNLOCK (_cairo_pattern_solid_cache_lock);
+	}
     } else {
 	free (pattern);
     }
-- 
1.4.4.2



More information about the cairo mailing list