[Spice-devel] [PATCH spice-gtk 08/10] gtk: use GHashTable in display_cache

Marc-André Lureau marcandre.lureau at gmail.com
Tue Sep 10 07:44:24 PDT 2013


On Mon, Sep 9, 2013 at 9:19 PM, Yonit Halperin <yhalperi at redhat.com> wrote:
> Hi,
>
> In general it looks good, but I have several comments:
>
> (1) the palette cache shouldn't be shared among the display channels. I.e.,
> there should be one instance per channel, and not one instance in
> spice-session.

Ok, I thought display cache and palette cache were directly related,
but it seems the display cache contains images without palette. I will
address this separately in a second patch.

> (2) I think you changed the ref_count logic of cache item. Now each cache
> value is referenced by the cache only once.
> While multiple insertions and deletions from the cache shouldn't occur, I
> believe that the previous ref counting may be the reason that even though
> the palette cache was shared by the channels (while it shouldn't have been),
> deleting palettes with ref_count > 1, didn't cause errors.

Could be, but I don't think the item ref count is necessary. This
change shouldn't change the behaviour, since the cache was updated
anyway, and the refcnt isn't used (there is no long lived ref on
palette).

> (3) hash function: the unique id can serve as the hash key, so you can just
> use the identity function instead of g_int64_hash

Well, sadly not on x32. If we would use uint32, yes... (my initial
patch was using direct hash)

>
> Best,
> Yonit.
>
>
> On 09/08/2013 02:59 PM, Marc-André Lureau wrote:
>>
>> From: Marc-André Lureau <marcandre.lureau at gmail.com>
>>
>> The cache code isn't very quick, it shows up in profilers.  Using
>> GHashTable allows to simplify the code while making it faster.
>> ---
>>   gtk/channel-cursor.c      |  56 +++++++----------------
>>   gtk/channel-display.c     |  93 +++++++++++---------------------------
>>   gtk/spice-channel-cache.h | 111
>> +++++++++++++++++-----------------------------
>>   gtk/spice-session-priv.h  |   4 +-
>>   gtk/spice-session.c       |  58 +++++++-----------------
>>   spice-common              |   2 +-
>>   6 files changed, 103 insertions(+), 221 deletions(-)
>>
>> diff --git a/gtk/channel-cursor.c b/gtk/channel-cursor.c
>> index e4a996b..eb53843 100644
>> --- a/gtk/channel-cursor.c
>> +++ b/gtk/channel-cursor.c
>> @@ -51,7 +51,7 @@ struct display_cursor {
>>   };
>>
>>   struct _SpiceCursorChannelPrivate {
>> -    display_cache               cursors;
>> +    display_cache               *cursors;
>>       gboolean                    init_done;
>>   };
>>
>> @@ -67,7 +67,6 @@ enum {
>>   static guint signals[SPICE_CURSOR_LAST_SIGNAL];
>>
>>   static void spice_cursor_handle_msg(SpiceChannel *channel, SpiceMsgIn
>> *msg);
>> -static void delete_cursor_all(SpiceChannel *channel);
>>   static display_cursor * display_cursor_ref(display_cursor *cursor);
>>   static void display_cursor_unref(display_cursor *cursor);
>>
>> @@ -81,12 +80,14 @@ static void
>> spice_cursor_channel_init(SpiceCursorChannel *channel)
>>
>>       c = channel->priv = SPICE_CURSOR_CHANNEL_GET_PRIVATE(channel);
>>
>> -    cache_init(&c->cursors, "cursor");
>> +    c->cursors = cache_new((GDestroyNotify)display_cursor_unref);
>>   }
>>
>>   static void spice_cursor_channel_finalize(GObject *obj)
>>   {
>> -    delete_cursor_all(SPICE_CHANNEL(obj));
>> +    SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL_GET_PRIVATE(obj);
>> +
>> +    g_clear_pointer(&c->cursors, cache_unref);
>>
>>       if (G_OBJECT_CLASS(spice_cursor_channel_parent_class)->finalize)
>>
>> G_OBJECT_CLASS(spice_cursor_channel_parent_class)->finalize(obj);
>> @@ -97,7 +98,7 @@ static void spice_cursor_channel_reset(SpiceChannel
>> *channel, gboolean migrating
>>   {
>>       SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>>
>> -    delete_cursor_all(channel);
>> +    cache_clear(c->cursors);
>>       c->init_done = FALSE;
>>
>>
>> SPICE_CHANNEL_CLASS(spice_cursor_channel_parent_class)->channel_reset(channel,
>> migrating);
>> @@ -359,7 +360,6 @@ static display_cursor *set_cursor(SpiceChannel
>> *channel, SpiceCursor *scursor)
>>   {
>>       SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>>       SpiceCursorHeader *hdr = &scursor->header;
>> -    display_cache_item *item;
>>       display_cursor *cursor;
>>       size_t size;
>>       gint i, pix_mask, pix;
>> @@ -378,9 +378,9 @@ static display_cursor *set_cursor(SpiceChannel
>> *channel, SpiceCursor *scursor)
>>                     hdr->width, hdr->height);
>>
>>       if (scursor->flags & SPICE_CURSOR_FLAGS_FROM_CACHE) {
>> -        item = cache_find(&c->cursors, hdr->unique);
>> -        g_return_val_if_fail(item != NULL, NULL);
>> -        return display_cursor_ref(item->ptr);
>> +        cursor = cache_find(c->cursors, hdr->unique);
>> +        g_return_val_if_fail(cursor != NULL, NULL);
>> +        return display_cursor_ref(cursor);
>>       }
>>
>>       g_return_val_if_fail(scursor->data_size != 0, NULL);
>> @@ -453,36 +453,12 @@ static display_cursor *set_cursor(SpiceChannel
>> *channel, SpiceCursor *scursor)
>>
>>   cache_add:
>>       if (cursor && (scursor->flags & SPICE_CURSOR_FLAGS_CACHE_ME)) {
>> -        display_cursor_ref(cursor);
>> -        item = cache_add(&c->cursors, hdr->unique);
>> -        item->ptr = cursor;
>> +        cache_add(c->cursors, hdr->unique, display_cursor_ref(cursor));
>>       }
>>
>>       return cursor;
>>   }
>>
>> -static void delete_cursor_one(SpiceChannel *channel, display_cache_item
>> *item)
>> -{
>> -    SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>> -
>> -    display_cursor_unref((display_cursor*)item->ptr);
>> -    cache_del(&c->cursors, item);
>> -}
>> -
>> -static void delete_cursor_all(SpiceChannel *channel)
>> -{
>> -    SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>> -    display_cache_item *item;
>> -
>> -    for (;;) {
>> -        item = cache_get_lru(&c->cursors);
>> -        if (item == NULL) {
>> -            return;
>> -        }
>> -        delete_cursor_one(channel, item);
>> -    }
>> -}
>> -
>>   /* coroutine context */
>>   static void emit_cursor_set(SpiceChannel *channel, display_cursor
>> *cursor)
>>   {
>> @@ -502,7 +478,7 @@ static void cursor_handle_init(SpiceChannel *channel,
>> SpiceMsgIn *in)
>>
>>       g_return_if_fail(c->init_done == FALSE);
>>
>> -    delete_cursor_all(channel);
>> +    cache_clear(c->cursors);
>>       cursor = set_cursor(channel, &init->cursor);
>>       c->init_done = TRUE;
>>       if (cursor)
>> @@ -520,7 +496,7 @@ static void cursor_handle_reset(SpiceChannel *channel,
>> SpiceMsgIn *in)
>>
>>       CHANNEL_DEBUG(channel, "%s, init_done: %d", __FUNCTION__,
>> c->init_done);
>>
>> -    delete_cursor_all(channel);
>> +    cache_clear(c->cursors);
>>       emit_main_context(channel, SPICE_CURSOR_RESET);
>>       c->init_done = FALSE;
>>   }
>> @@ -584,18 +560,18 @@ static void cursor_handle_inval_one(SpiceChannel
>> *channel, SpiceMsgIn *in)
>>   {
>>       SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>>       SpiceMsgDisplayInvalOne *zap = spice_msg_in_parsed(in);
>> -    display_cache_item *item;
>>
>>       g_return_if_fail(c->init_done == TRUE);
>>
>> -    item = cache_find(&c->cursors, zap->id);
>> -    delete_cursor_one(channel, item);
>> +    cache_remove(c->cursors, zap->id);
>>   }
>>
>>   /* coroutine context */
>>   static void cursor_handle_inval_all(SpiceChannel *channel, SpiceMsgIn
>> *in)
>>   {
>> -    delete_cursor_all(channel);
>> +    SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(channel)->priv;
>> +
>> +    cache_clear(c->cursors);
>>   }
>>
>>   static const spice_msg_handler cursor_handlers[] = {
>> diff --git a/gtk/channel-display.c b/gtk/channel-display.c
>> index 03ae535..a69f14f 100644
>> --- a/gtk/channel-display.c
>> +++ b/gtk/channel-display.c
>> @@ -486,16 +486,8 @@ static void image_put(SpiceImageCache *cache,
>> uint64_t id, pixman_image_t *image
>>   {
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> image_cache);
>> -    display_cache_item *item;
>>
>> -    item = cache_find(c->images, id);
>> -    if (item) {
>> -        cache_ref(item);
>> -        return;
>> -    }
>> -
>> -    item = cache_add(c->images, id);
>> -    item->ptr = pixman_image_ref(image);
>> +    cache_add(c->images, id, pixman_image_ref(image));
>>   }
>>
>>   typedef struct _WaitImageData
>> @@ -508,18 +500,16 @@ typedef struct _WaitImageData
>>
>>   static gboolean wait_image(gpointer data)
>>   {
>> -    display_cache_item *item;
>> +    gboolean lossy;
>>       WaitImageData *wait = data;
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(wait->cache, SpiceDisplayChannelPrivate,
>> image_cache);
>> +    pixman_image_t *image = cache_find_lossy(c->images, wait->id,
>> &lossy);
>>
>> -    item = cache_find(c->images, wait->id);
>> -    if (item == NULL ||
>> -        (item->lossy && !wait->lossy))
>> +    if (!image || (lossy && !wait->lossy))
>>           return FALSE;
>>
>> -    cache_used(c->images, item);
>> -    wait->image = pixman_image_ref(item->ptr);
>> +    wait->image = pixman_image_ref(image);
>>
>>       return TRUE;
>>   }
>> @@ -538,58 +528,33 @@ static pixman_image_t *image_get(SpiceImageCache
>> *cache, uint64_t id)
>>       return wait.image;
>>   }
>>
>> -static void image_remove(SpiceImageCache *cache, uint64_t id)
>> -{
>> -    SpiceDisplayChannelPrivate *c =
>> -        SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> image_cache);
>> -    display_cache_item *item;
>> -
>> -    item = cache_find(c->images, id);
>> -    g_return_if_fail(item != NULL);
>> -    if (cache_unref(item)) {
>> -        pixman_image_unref(item->ptr);
>> -        cache_del(c->images, item);
>> -    }
>> -}
>> -
>>   static void palette_put(SpicePaletteCache *cache, SpicePalette *palette)
>>   {
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> palette_cache);
>> -    display_cache_item *item;
>>
>> -    item = cache_add(c->palettes, palette->unique);
>> -    item->ptr = g_memdup(palette, sizeof(SpicePalette) +
>> -                         palette->num_ents * sizeof(palette->ents[0]));
>> +    cache_add(c->palettes, palette->unique,
>> +              g_memdup(palette, sizeof(SpicePalette) +
>> +                       palette->num_ents * sizeof(palette->ents[0])));
>>   }
>>
>>   static SpicePalette *palette_get(SpicePaletteCache *cache, uint64_t id)
>>   {
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> palette_cache);
>> -    display_cache_item *item;
>>
>> -    item = cache_find(c->palettes, id);
>> -    if (item) {
>> -        cache_ref(item);
>> -        return item->ptr;
>> -    }
>> -    return NULL;
>> +    /* here the returned pointer is weak, no ref given to caller.  it
>> +     * seems spice canvas usage is exclusively temporary, so it's ok
>> +     * (for now) */
>> +    return cache_find(c->palettes, id);
>>   }
>>
>>   static void palette_remove(SpicePaletteCache *cache, uint32_t id)
>>   {
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> palette_cache);
>> -    display_cache_item *item;
>>
>> -    item = cache_find(c->palettes, id);
>> -    if (item) {
>> -        if (cache_unref(item)) {
>> -            g_free(item->ptr);
>> -            cache_del(c->palettes, item);
>> -        }
>> -    }
>> +    cache_remove(c->palettes, id);
>>   }
>>
>>   static void palette_release(SpicePaletteCache *cache, SpicePalette
>> *palette)
>> @@ -603,30 +568,18 @@ static void image_put_lossy(SpiceImageCache *cache,
>> uint64_t id,
>>   {
>>       SpiceDisplayChannelPrivate *c =
>>           SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> image_cache);
>> -    display_cache_item *item;
>>
>>   #ifndef NDEBUG
>>       g_warn_if_fail(cache_find(c->images, id) == NULL);
>>   #endif
>>
>> -    item = cache_add(c->images, id);
>> -    item->ptr = pixman_image_ref(surface);
>> -    item->lossy = TRUE;
>> +    cache_add_lossy(c->images, id, pixman_image_ref(surface), TRUE);
>>   }
>>
>>   static void image_replace_lossy(SpiceImageCache *cache, uint64_t id,
>>                                   pixman_image_t *surface)
>>   {
>> -    SpiceDisplayChannelPrivate *c =
>> -        SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate,
>> image_cache);
>> -    display_cache_item *item;
>> -
>> -    item = cache_find(c->images, id);
>> -    g_return_if_fail(item != NULL);
>> -
>> -    pixman_image_unref(item->ptr);
>> -    item->ptr = pixman_image_ref(surface);
>> -    item->lossy = FALSE;
>> +    image_put(cache, id, surface);
>>   }
>>
>>   static pixman_image_t* image_get_lossless(SpiceImageCache *cache,
>> uint64_t id)
>> @@ -968,7 +921,7 @@ static void display_handle_reset(SpiceChannel
>> *channel, SpiceMsgIn *in)
>>       if (surface != NULL)
>>           surface->canvas->ops->clear(surface->canvas);
>>
>> -    spice_session_palettes_clear(spice_channel_get_session(channel));
>> +    cache_clear(c->palettes);
>>
>>       c->mark = FALSE;
>>       emit_main_context(channel, SPICE_DISPLAY_MARK, FALSE);
>> @@ -997,9 +950,12 @@ static void display_handle_inv_list(SpiceChannel
>> *channel, SpiceMsgIn *in)
>>       int i;
>>
>>       for (i = 0; i < list->count; i++) {
>> +        guint64 id = list->resources[i].id;
>> +
>>           switch (list->resources[i].type) {
>>           case SPICE_RES_TYPE_PIXMAP:
>> -            image_remove(&c->image_cache, list->resources[i].id);
>> +            if (!cache_remove(c->images, id))
>> +                SPICE_DEBUG("fail to remove image %" G_GUINT64_FORMAT,
>> id);
>>               break;
>>           default:
>>               g_return_if_reached();
>> @@ -1011,9 +967,10 @@ static void display_handle_inv_list(SpiceChannel
>> *channel, SpiceMsgIn *in)
>>   /* coroutine context */
>>   static void display_handle_inv_pixmap_all(SpiceChannel *channel,
>> SpiceMsgIn *in)
>>   {
>> -    spice_channel_handle_wait_for_channels(channel, in);
>> +    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>>
>> -    spice_session_images_clear(spice_channel_get_session(channel));
>> +    spice_channel_handle_wait_for_channels(channel, in);
>> +    cache_clear(c->images);
>>   }
>>
>>   /* coroutine context */
>> @@ -1028,7 +985,9 @@ static void display_handle_inv_palette(SpiceChannel
>> *channel, SpiceMsgIn *in)
>>   /* coroutine context */
>>   static void display_handle_inv_palette_all(SpiceChannel *channel,
>> SpiceMsgIn *in)
>>   {
>> -    spice_session_palettes_clear(spice_channel_get_session(channel));
>> +    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>> +
>> +    cache_clear(c->palettes);
>>   }
>>
>>   /* ------------------------------------------------------------------ */
>> diff --git a/gtk/spice-channel-cache.h b/gtk/spice-channel-cache.h
>> index 3f39877..17775e6 100644
>> --- a/gtk/spice-channel-cache.h
>> +++ b/gtk/spice-channel-cache.h
>> @@ -25,109 +25,80 @@
>>   G_BEGIN_DECLS
>>
>>   typedef struct display_cache_item {
>> -    RingItem                    hash_link;
>> -    RingItem                    lru_link;
>> -    uint64_t                    id;
>> -    uint32_t                    refcount;
>> -    void                        *ptr;
>> +    guint64                     id;
>>       gboolean                    lossy;
>>   } display_cache_item;
>>
>> -typedef struct display_cache {
>> -    const char                  *name;
>> -    Ring                        hash[64];
>> -    Ring                        lru;
>> -    int                         nitems;
>> -} display_cache;
>> +typedef GHashTable display_cache;
>>
>> -static inline void cache_init(display_cache *cache, const char *name)
>> +static inline display_cache_item* cache_item_new(guint64 id, gboolean
>> lossy)
>>   {
>> -    int i;
>> +    display_cache_item *self = g_slice_new(display_cache_item);
>> +    self->id = id;
>> +    self->lossy = lossy;
>> +    return self;
>> +}
>>
>> -    cache->name = name;
>> -    ring_init(&cache->lru);
>> -    for (i = 0; i < SPICE_N_ELEMENTS(cache->hash); i++) {
>> -        ring_init(&cache->hash[i]);
>> -    }
>> +static inline void cache_item_free(display_cache_item *self)
>> +{
>> +    g_slice_free(display_cache_item, self);
>>   }
>>
>> -static inline Ring *cache_head(display_cache *cache, uint64_t id)
>> +static inline display_cache* cache_new(GDestroyNotify value_destroy)
>>   {
>> -    return &cache->hash[id % SPICE_N_ELEMENTS(cache->hash)];
>> +    GHashTable* self;
>> +
>> +    self = g_hash_table_new_full(g_int64_hash, g_int64_equal,
>> +                                 (GDestroyNotify)cache_item_free,
>> +                                 value_destroy);
>> +
>> +    return self;
>>   }
>>
>> -static inline void cache_used(display_cache *cache, display_cache_item
>> *item)
>> +static inline gpointer cache_find(display_cache *cache, uint64_t id)
>>   {
>> -    ring_remove(&item->lru_link);
>> -    ring_add(&cache->lru, &item->lru_link);
>> +    return g_hash_table_lookup(cache, &id);
>>   }
>>
>> -static inline display_cache_item *cache_get_lru(display_cache *cache)
>> +static inline gpointer cache_find_lossy(display_cache *cache, uint64_t
>> id, gboolean *lossy)
>>   {
>> +    gpointer value;
>>       display_cache_item *item;
>> -    RingItem *ring;
>>
>> -    if (ring_is_empty(&cache->lru))
>> +    if (!g_hash_table_lookup_extended(cache, &id, (gpointer*)&item,
>> &value))
>>           return NULL;
>> -    ring = ring_get_tail(&cache->lru);
>> -    item = SPICE_CONTAINEROF(ring, display_cache_item, lru_link);
>> -    return item;
>> -}
>>
>> -static inline display_cache_item *cache_find(display_cache *cache,
>> uint64_t id)
>> -{
>> -    display_cache_item *item;
>> -    RingItem *ring;
>> -    Ring *head;
>> -
>> -    head = cache_head(cache, id);
>> -    for (ring = ring_get_head(head); ring != NULL; ring = ring_next(head,
>> ring)) {
>> -        item = SPICE_CONTAINEROF(ring, display_cache_item, hash_link);
>> -        if (item->id == id) {
>> -            return item;
>> -        }
>> -    }
>> -
>> -    SPICE_DEBUG("%s: %s %" PRIx64 " [not found]", __FUNCTION__,
>> -            cache->name, id);
>> -    return NULL;
>> +    *lossy = item->lossy;
>> +
>> +    return value;
>>   }
>>
>> -static inline display_cache_item *cache_add(display_cache *cache,
>> uint64_t id)
>> +static inline void cache_add_lossy(display_cache *cache, uint64_t id,
>> +                                   gpointer value, gboolean lossy)
>>   {
>> -    display_cache_item *item;
>> +    display_cache_item *item = cache_item_new(id, lossy);
>>
>> -    item = spice_new0(display_cache_item, 1);
>> -    item->id = id;
>> -    item->refcount = 1;
>> -    ring_add(cache_head(cache, item->id), &item->hash_link);
>> -    ring_add(&cache->lru, &item->lru_link);
>> -    cache->nitems++;
>> +    g_hash_table_replace(cache, item, value);
>> +}
>>
>> -    SPICE_DEBUG("%s: %s %" PRIx64 " (%d)", __FUNCTION__,
>> -            cache->name, id, cache->nitems);
>> -    return item;
>> +static inline void cache_add(display_cache *cache, uint64_t id, gpointer
>> value)
>> +{
>> +    cache_add_lossy(cache, id, value, FALSE);
>>   }
>>
>> -static inline void cache_del(display_cache *cache, display_cache_item
>> *item)
>> +static inline gboolean cache_remove(display_cache *cache, uint64_t id)
>>   {
>> -    SPICE_DEBUG("%s: %s %" PRIx64, __FUNCTION__,
>> -            cache->name, item->id);
>> -    ring_remove(&item->hash_link);
>> -    ring_remove(&item->lru_link);
>> -    free(item);
>> -    cache->nitems--;
>> +    return g_hash_table_remove(cache, &id);
>>   }
>>
>> -static inline void cache_ref(display_cache_item *item)
>> +static inline void cache_clear(display_cache *cache)
>>   {
>> -    item->refcount++;
>> +    g_hash_table_remove_all(cache);
>>   }
>>
>> -static inline int cache_unref(display_cache_item *item)
>> +static inline void cache_unref(display_cache *cache)
>>   {
>> -    item->refcount--;
>> -    return item->refcount == 0;
>> +    g_hash_table_unref(cache);
>>   }
>>
>>   G_END_DECLS
>> diff --git a/gtk/spice-session-priv.h b/gtk/spice-session-priv.h
>> index 5ed48dd..e175281 100644
>> --- a/gtk/spice-session-priv.h
>> +++ b/gtk/spice-session-priv.h
>> @@ -92,8 +92,8 @@ struct _SpiceSessionPrivate {
>>       guint             after_main_init;
>>       gboolean          migration_copy;
>>
>> -    display_cache     images;
>> -    display_cache     palettes;
>> +    display_cache     *images;
>> +    display_cache     *palettes;
>>       SpiceGlzDecoderWindow *glz_window;
>>       int               images_cache_size;
>>       int               glz_window_size;
>> diff --git a/gtk/spice-session.c b/gtk/spice-session.c
>> index a9435f4..c050266 100644
>> --- a/gtk/spice-session.c
>> +++ b/gtk/spice-session.c
>> @@ -176,8 +176,8 @@ static void spice_session_init(SpiceSession *session)
>>       g_free(channels);
>>
>>       ring_init(&s->channels);
>> -    cache_init(&s->images, "image");
>> -    cache_init(&s->palettes, "palette");
>> +    s->images = cache_new((GDestroyNotify)pixman_image_unref);
>> +    s->palettes = cache_new(g_free);
>>       s->glz_window = glz_decoder_window_new();
>>       update_proxy(session, NULL);
>>   }
>> @@ -219,35 +219,6 @@ spice_session_dispose(GObject *gobject)
>>           G_OBJECT_CLASS(spice_session_parent_class)->dispose(gobject);
>>   }
>>
>> -G_GNUC_INTERNAL
>> -void spice_session_palettes_clear(SpiceSession *session)
>> -{
>> -    SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
>> -    g_return_if_fail(s != NULL);
>> -
>> -    for (;;) {
>> -        display_cache_item *item = cache_get_lru(&s->palettes);
>> -        if (item == NULL)
>> -            break;
>> -        cache_del(&s->palettes, item);
>> -    }
>> -}
>> -
>> -G_GNUC_INTERNAL
>> -void spice_session_images_clear(SpiceSession *session)
>> -{
>> -    SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(session);
>> -    g_return_if_fail(s != NULL);
>> -
>> -    for (;;) {
>> -        display_cache_item *item = cache_get_lru(&s->images);
>> -        if (item == NULL)
>> -            break;
>> -        pixman_image_unref(item->ptr);
>> -        cache_del(&s->images, item);
>> -    }
>> -}
>> -
>>   static void
>>   spice_session_finalize(GObject *gobject)
>>   {
>> @@ -267,8 +238,8 @@ spice_session_finalize(GObject *gobject)
>>       g_strfreev(s->disable_effects);
>>       g_strfreev(s->secure_channels);
>>
>> -    spice_session_palettes_clear(session);
>> -    spice_session_images_clear(session);
>> +    g_clear_pointer(&s->images, cache_unref);
>> +    g_clear_pointer(&s->palettes, cache_unref);
>>       glz_decoder_window_destroy(s->glz_window);
>>
>>       g_clear_pointer(&s->pubkey, g_byte_array_unref);
>> @@ -1332,6 +1303,15 @@ gboolean
>> spice_session_get_client_provided_socket(SpiceSession *session)
>>       return s->client_provided_sockets;
>>   }
>>
>> +static void cache_clear_all(SpiceSession *self)
>> +{
>> +    SpiceSessionPrivate *s = SPICE_SESSION_GET_PRIVATE(self);
>> +
>> +    cache_clear(s->images);
>> +    cache_clear(s->palettes);
>> +    glz_decoder_window_clear(s->glz_window);
>> +}
>> +
>>   G_GNUC_INTERNAL
>>   void spice_session_switching_disconnect(SpiceSession *self)
>>   {
>> @@ -1353,9 +1333,7 @@ void spice_session_switching_disconnect(SpiceSession
>> *self)
>>
>>       g_warn_if_fail(!ring_is_empty(&s->channels)); /* ring_get_length()
>> == 1 */
>>
>> -    spice_session_palettes_clear(self);
>> -    spice_session_images_clear(self);
>> -    glz_decoder_window_clear(s->glz_window);
>> +    cache_clear_all(self);
>>   }
>>
>>   G_GNUC_INTERNAL
>> @@ -1561,9 +1539,7 @@ void spice_session_migrate_end(SpiceSession *self)
>>           }
>>       }
>>
>> -    spice_session_palettes_clear(self);
>> -    spice_session_images_clear(self);
>> -    glz_decoder_window_clear(s->glz_window);
>> +    cache_clear_all(self);
>>
>>       /* send MIGRATE_END to target */
>>       out = spice_msg_out_new(s->cmain, SPICE_MSGC_MAIN_MIGRATE_END);
>> @@ -2119,9 +2095,9 @@ void spice_session_get_caches(SpiceSession *session,
>>       g_return_if_fail(s != NULL);
>>
>>       if (images)
>> -        *images = &s->images;
>> +        *images = s->images;
>>       if (palettes)
>> -        *palettes = &s->palettes;
>> +        *palettes = s->palettes;
>>       if (glz_window)
>>           *glz_window = s->glz_window;
>>   }
>> diff --git a/spice-common b/spice-common
>> index fc27fb2..7aef065 160000
>> --- a/spice-common
>> +++ b/spice-common
>> @@ -1 +1 @@
>> -Subproject commit fc27fb20b8ecd3e07809aec884f99f2121712bc9
>> +Subproject commit 7aef06577e47965bbe93e0a054857a562d2fde5a
>>
>



-- 
Marc-André Lureau


More information about the Spice-devel mailing list