[gegl] GeglTileBackendRam: Store tiles directly

[Daniel Sabo <daniels src gnome org>]

commit 0f71998737bc19e567b8f07c493e5dfec687f5ca
Author: Daniel Sabo <DanielSabo gmail com>
Date:   Fri Feb 1 23:49:25 2013 -0800

    GeglTileBackendRam: Store tiles directly
    
    Store GeglTile structs instead of internal buffers, this gets
    rid of the significant duplication of data when the tile
    cache attempts to flush.

 gegl/buffer/gegl-tile-backend-ram.c |  333 +++++++++++++++++------------------
 1 files changed, 159 insertions(+), 174 deletions(-)
---
diff --git a/gegl/buffer/gegl-tile-backend-ram.c b/gegl/buffer/gegl-tile-backend-ram.c
index 4f5346b..4c18fc7 100644
--- a/gegl/buffer/gegl-tile-backend-ram.c
+++ b/gegl/buffer/gegl-tile-backend-ram.c
@@ -14,20 +14,19 @@
  * License along with GEGL; if not, see <http://www.gnu.org/licenses/>.
  *
  * Copyright 2006,2007 Øyvind Kolås <pippin gimp org>
+ * Copyright 2013 Daniel Sabo <DanielSabo gmail com>
  */
 
 #include "config.h"
 
-#include <string.h>
-
 #include <glib-object.h>
 
 #include "gegl-buffer-backend.h"
 #include "gegl-tile-backend.h"
 #include "gegl-tile-backend-ram.h"
 
-static void dbg_alloc (int size);
-static void dbg_dealloc (int size);
+/* We need the private header so we can check the ref_count of tiles */
+#include "gegl-buffer-private.h"
 
 typedef struct _RamEntry RamEntry;
 
@@ -35,169 +34,149 @@ struct _RamEntry
 {
   gint    x;
   gint    y;
-  gint    z;
-  guchar *offset;
+  GeglTile *tile;
 };
 
-static inline void
-ram_entry_read (GeglTileBackendRam *ram,
-                RamEntry           *entry,
-                guchar             *dest)
-{
-  gint tile_size = gegl_tile_backend_get_tile_size (GEGL_TILE_BACKEND (ram));
-
-  memcpy (dest, entry->offset, tile_size);
-}
-
-static inline void
-ram_entry_write (GeglTileBackendRam *ram,
-                 RamEntry           *entry,
-                 guchar             *source)
-{
-  gint tile_size = gegl_tile_backend_get_tile_size (GEGL_TILE_BACKEND (ram));
-
-  memcpy (entry->offset, source, tile_size);
-}
-
-static inline RamEntry *
-ram_entry_new (GeglTileBackendRam *ram)
-{
-  RamEntry *self = g_slice_new (RamEntry);
-  gint tile_size = gegl_tile_backend_get_tile_size (GEGL_TILE_BACKEND (ram));
-
-  self->offset = g_malloc (tile_size);
-  dbg_alloc (tile_size);
-  return self;
-}
-
-static inline void
-ram_entry_destroy (RamEntry           *entry,
-                   GeglTileBackendRam *ram)
-{
-  gint tile_size = gegl_tile_backend_get_tile_size (GEGL_TILE_BACKEND (ram));
-  g_free (entry->offset);
-  g_hash_table_remove (ram->entries, entry);
-
-  dbg_dealloc (tile_size);
-  g_slice_free (RamEntry, entry);
-}
-
-
 G_DEFINE_TYPE (GeglTileBackendRam, gegl_tile_backend_ram, GEGL_TYPE_TILE_BACKEND)
-static GObjectClass * parent_class = NULL;
-
-
-static gint allocs        = 0;
-static gint ram_size      = 0;
-static gint peak_allocs   = 0;
-static gint peak_ram_size = 0;
+#define parent_class gegl_tile_backend_ram_parent_class
+
+static GeglTile *get_tile   (GeglTileSource *tile_store,
+                             gint             x,
+                             gint             y,
+                             gint             z);
+static gboolean  set_tile   (GeglTileSource *store,
+                             GeglTile       *tile,
+                             gint            x,
+                             gint            y,
+                             gint            z);
+static gboolean  exist_tile (GeglTileSource *store,
+                             GeglTile       *tile,
+                             gint            x,
+                             gint            y,
+                             gint            z);
+static gboolean  void_tile  (GeglTileSource *store,
+                             GeglTile       *tile,
+                             gint            x,
+                             gint            y,
+                             gint            z);
 
 void gegl_tile_backend_ram_stats (void)
 {
-  g_warning ("leaked: %i chunks (%f mb)  peak: %i (%i bytes %fmb))",
-             allocs, ram_size / 1024 / 1024.0, peak_allocs, peak_ram_size, peak_ram_size / 1024 / 1024.0);
-}
-
-static void dbg_alloc (gint size)
-{
-  allocs++;
-  ram_size += size;
-  if (allocs > peak_allocs)
-    peak_allocs = allocs;
-  if (ram_size > peak_ram_size)
-    peak_ram_size = ram_size;
-}
-
-static void dbg_dealloc (gint size)
-{
-  allocs--;
-  ram_size -= size;
+  /* FIXME: Stats disabled for now as there's nothing meaningful to report */
 }
 
 static inline RamEntry *
 lookup_entry (GeglTileBackendRam *self,
-              gint         x,
-              gint         y,
-              gint         z)
+              gint                x,
+              gint                y)
 {
   RamEntry key;
 
   key.x = x;
   key.y = y;
-  key.z = z;
-  key.offset = 0;
+  key.tile = NULL;
 
   return g_hash_table_lookup (self->entries, &key);
 }
 
-/* this is the only place that actually should
- * instantiate tiles, when the cache is large enough
- * that should make sure we don't hit this function
- * too often.
- */
 static GeglTile *
 get_tile (GeglTileSource *tile_store,
           gint        x,
           gint        y,
           gint        z)
 {
-  GeglTileBackendRam     *tile_backend_ram = GEGL_TILE_BACKEND_RAM (tile_store);
-  GeglTileBackend *backend  = GEGL_TILE_BACKEND (tile_store);
-  GeglTile        *tile     = NULL;
-  gint tile_size = gegl_tile_backend_get_tile_size (backend);
-
-  {
-    RamEntry *entry = lookup_entry (tile_backend_ram, x, y, z);
+  GeglTileBackendRam *tile_backend_ram = GEGL_TILE_BACKEND_RAM (tile_store);
 
-    if (!entry)
-      return NULL;
-
-    tile = gegl_tile_new (tile_size);
+  if (z == 0)
+    {
+      RamEntry *entry = lookup_entry (tile_backend_ram, x, y);
+      if (entry)
+        return gegl_tile_ref (entry->tile);
+    }
 
-    ram_entry_read (tile_backend_ram, entry, gegl_tile_get_data (tile));
-  }
-  return tile;
+  return NULL;
 }
 
 static
 gboolean set_tile (GeglTileSource *store,
-                   GeglTile   *tile,
-                   gint        x,
-                   gint        y,
-                   gint        z)
+                   GeglTile       *tile,
+                   gint            x,
+                   gint            y,
+                   gint            z)
 {
-  GeglTileBackend *backend  = GEGL_TILE_BACKEND (store);
-  GeglTileBackendRam     *tile_backend_ram = GEGL_TILE_BACKEND_RAM (backend);
+  GeglTileBackendRam *tile_backend_ram = GEGL_TILE_BACKEND_RAM (store);
+  RamEntry           *entry  = NULL;
+  gboolean            is_dup = FALSE;
+
+  if (z != 0)
+    return FALSE;
 
-  RamEntry        *entry = lookup_entry (tile_backend_ram, x, y, z);
+  entry = lookup_entry (tile_backend_ram, x, y);
+
+  if (tile->ref_count == 0)
+    {
+      /* We've been handed a dead tile to store, this happens
+       * when tile_unref is called on a tile that had never
+       * been stored.
+       * At this stage in tile_unref it's still safe to duplicate
+       * the tile, which will prevent it's data from actually
+       * being free'd.
+       */
+       tile = gegl_tile_dup (tile);
+
+       /* The x,y,z values aren't copied by gegl_tile_dup */
+       tile->x = x;
+       tile->y = y;
+       tile->z = z;
+
+       is_dup = TRUE;
+    }
 
-  if (entry == NULL)
+  if (!entry)
     {
-      entry    = ram_entry_new (tile_backend_ram);
+      entry = g_slice_new (RamEntry);
       entry->x = x;
       entry->y = y;
-      entry->z = z;
+      entry->tile = NULL;
       g_hash_table_insert (tile_backend_ram->entries, entry, entry);
     }
-  ram_entry_write (tile_backend_ram, entry, gegl_tile_get_data (tile));
-  gegl_tile_mark_as_stored (tile);
+  else if (entry->tile == tile)
+    {
+      gegl_tile_mark_as_stored (tile);
+      return TRUE;
+    }
+  else
+    {
+      /* Mark as stored to prevent a recursive attempt to store by tile_unref */
+      gegl_tile_mark_as_stored (entry->tile);
+      gegl_tile_unref (entry->tile);
+    }
+
+  entry->tile = tile;
+
+  if (!is_dup)
+    gegl_tile_ref (entry->tile);
+
+  gegl_tile_mark_as_stored (entry->tile);
+
   return TRUE;
 }
 
 static
 gboolean void_tile (GeglTileSource *store,
-                    GeglTile   *tile,
-                    gint        x,
-                    gint        y,
-                    gint        z)
+                    GeglTile       *tile,
+                    gint            x,
+                    gint            y,
+                    gint            z)
 {
-  GeglTileBackend *backend  = GEGL_TILE_BACKEND (store);
-  GeglTileBackendRam     *tile_backend_ram = GEGL_TILE_BACKEND_RAM (backend);
-  RamEntry        *entry    = lookup_entry (tile_backend_ram, x, y, z);
+  GeglTileBackendRam *tile_backend_ram = GEGL_TILE_BACKEND_RAM (store);
 
-  if (entry != NULL)
+  if (z == 0)
     {
-      ram_entry_destroy (entry, tile_backend_ram);
+      RamEntry *entry = lookup_entry (tile_backend_ram, x, y);
+
+      if (entry != NULL)
+        g_hash_table_remove (tile_backend_ram->entries, entry);
     }
 
   return TRUE;
@@ -205,16 +184,17 @@ gboolean void_tile (GeglTileSource *store,
 
 static
 gboolean exist_tile (GeglTileSource *store,
-                     GeglTile   *tile,
-                     gint        x,
-                     gint        y,
-                     gint        z)
+                     GeglTile       *tile,
+                     gint            x,
+                     gint            y,
+                     gint            z)
 {
-  GeglTileBackend *backend  = GEGL_TILE_BACKEND (store);
-  GeglTileBackendRam     *tile_backend_ram = GEGL_TILE_BACKEND_RAM (backend);
-  RamEntry        *entry    = lookup_entry (tile_backend_ram, x, y, z);
+  GeglTileBackendRam *tile_backend_ram = GEGL_TILE_BACKEND_RAM (store);
 
-  return entry != NULL;
+  if (z != 0)
+    return FALSE;
+
+  return lookup_entry (tile_backend_ram, x, y) != NULL;
 }
 
 
@@ -224,12 +204,12 @@ enum
 };
 
 static gpointer
-gegl_tile_backend_ram_command (GeglTileSource     *tile_store,
-                               GeglTileCommand command,
-                               gint            x,
-                               gint            y,
-                               gint            z,
-                               gpointer        data)
+gegl_tile_backend_ram_command (GeglTileSource  *tile_store,
+                               GeglTileCommand  command,
+                               gint             x,
+                               gint             y,
+                               gint             z,
+                               gpointer         data)
 {
   switch (command)
     {
@@ -248,19 +228,20 @@ gegl_tile_backend_ram_command (GeglTileSource     *tile_store,
         return NULL;
 
       case GEGL_TILE_EXIST:
-        return GINT_TO_POINTER(exist_tile (tile_store, data, x, y, z));
+        return GINT_TO_POINTER (exist_tile (tile_store, data, x, y, z));
 
       default:
         g_assert (command < GEGL_TILE_LAST_COMMAND &&
                   command >= 0);
     }
-  return FALSE;
+  return NULL;
 }
 
-static void set_property (GObject       *object,
-                            guint          property_id,
-                            const GValue *value,
-                            GParamSpec    *pspec)
+static void
+set_property (GObject       *object,
+              guint          property_id,
+              const GValue  *value,
+              GParamSpec    *pspec)
 {
   switch (property_id)
     {
@@ -270,10 +251,11 @@ static void set_property (GObject       *object,
     }
 }
 
-static void get_property (GObject    *object,
-                            guint       property_id,
-                            GValue     *value,
-                            GParamSpec *pspec)
+static void
+get_property (GObject    *object,
+              guint       property_id,
+              GValue     *value,
+              GParamSpec *pspec)
 {
   switch (property_id)
     {
@@ -284,79 +266,84 @@ static void get_property (GObject    *object,
 }
 
 static void
-for_each_free (RamEntry           *key,
-               RamEntry           *entry,
-               GeglTileBackendRam *ram)
-{
-  gint tile_size = gegl_tile_backend_get_tile_size (GEGL_TILE_BACKEND (ram));
-  g_free (entry->offset);
-  dbg_dealloc (tile_size);
-  g_slice_free (RamEntry, entry);
-}
-
-static void
-finalize (GObject *object)
+gegl_tile_backend_ram_finalize (GObject *object)
 {
   GeglTileBackendRam *self = GEGL_TILE_BACKEND_RAM (object);
 
-  g_hash_table_foreach (self->entries, (GHFunc)for_each_free, self);
   g_hash_table_unref (self->entries);
 
-  (*G_OBJECT_CLASS (parent_class)->finalize)(object);
+  G_OBJECT_CLASS (parent_class)->finalize (object);
 }
 
-static guint hashfunc (gconstpointer key)
+static guint
+ram_entry_hash_func (gconstpointer key)
 {
   const RamEntry *e = key;
   guint           hash;
   gint            i;
   gint            srcA = e->x;
   gint            srcB = e->y;
-  gint            srcC = e->z;
 
-  /* interleave the 10 least significant bits of all coordinates,
+  /* interleave the 16 least significant bits of the coordinates,
    * this gives us Z-order / morton order of the space and should
    * work well as a hash
    */
   hash = 0;
-  for (i = 9; i >= 0; i--)
+  for (i = 16; i >= 0; i--)
     {
 #define ADD_BIT(bit)    do { hash |= (((bit) != 0) ? 1 : 0); hash <<= 1; \
     } \
   while (0)
       ADD_BIT (srcA & (1 << i));
       ADD_BIT (srcB & (1 << i));
-      ADD_BIT (srcC & (1 << i));
 #undef ADD_BIT
     }
   return hash;
 }
 
-static gboolean equalfunc (gconstpointer a,
-                           gconstpointer b)
+static gboolean
+ram_entry_equal_func (gconstpointer a,
+                      gconstpointer b)
 {
   const RamEntry *ea = a;
   const RamEntry *eb = b;
 
   if (ea->x == eb->x &&
-      ea->y == eb->y &&
-      ea->z == eb->z)
+      ea->y == eb->y)
     return TRUE;
   return FALSE;
 }
 
+static void
+ram_entry_free_func (gpointer data)
+{
+  RamEntry *entry = (RamEntry *)data;
+
+  if (entry->tile)
+    {
+      /* Mark as stored to prevent an attempt to store by tile_unref */
+      gegl_tile_mark_as_stored (entry->tile);
+      gegl_tile_unref (entry->tile);
+      entry->tile = NULL;
+    }
+  g_slice_free (RamEntry, entry);
+}
+
 static GObject *
 gegl_tile_backend_ram_constructor (GType                  type,
                                    guint                  n_params,
                                    GObjectConstructParam *params)
 {
-  GObject     *object;
+  GObject            *object;
   GeglTileBackendRam *ram;
 
   object = G_OBJECT_CLASS (parent_class)->constructor (type, n_params, params);
   ram    = GEGL_TILE_BACKEND_RAM (object);
 
-  ram->entries = g_hash_table_new (hashfunc, equalfunc);
+  ram->entries = g_hash_table_new_full (ram_entry_hash_func,
+                                        ram_entry_equal_func,
+                                        NULL,
+                                        ram_entry_free_func);
 
   return object;
 }
@@ -364,19 +351,17 @@ gegl_tile_backend_ram_constructor (GType                  type,
 static void
 gegl_tile_backend_ram_class_init (GeglTileBackendRamClass *klass)
 {
-  GObjectClass    *gobject_class     = G_OBJECT_CLASS (klass);
-
-  parent_class = g_type_class_peek_parent (klass);
+  GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
 
   gobject_class->get_property = get_property;
   gobject_class->set_property = set_property;
   gobject_class->constructor  = gegl_tile_backend_ram_constructor;
-  gobject_class->finalize     = finalize;
+  gobject_class->finalize     = gegl_tile_backend_ram_finalize;
 }
 
 static void
 gegl_tile_backend_ram_init (GeglTileBackendRam *self)
 {
-  ((GeglTileSource*)self)->command = gegl_tile_backend_ram_command;
+  GEGL_TILE_SOURCE (self)->command = gegl_tile_backend_ram_command;
   self->entries = NULL;
 }