Re: [PATCH spice-gtk] If replace me, should not do refcount plus one

"yueyihua@xxxxxxxxxxx" <yueyihua@xxxxxxxxxxx> · Tue, 17 Jul 2018 18:56:21 +0800

OK, I have modified it.
 From 87be8732bf36038c9ba9faabb1aae58f7b830d05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E4=B9=90=E4=B9=89=E5=8D=8E?= <yueyihua@xxxxxxxxxxx>
Date: Thu, 12 Jul 2018 13:16:53 +0800
Subject: [PATCH] Fix image cache memory usage

When image flag is SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME, and if found it
in hash table, we will do refcount increase, but this image will only
destroy once, so the cache will usage more and more memory. In this
circumstance, we should not increase refconut.
---
 src/channel-display.c     | 5 ++++-
 src/spice-channel-cache.h | 8 ++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/channel-display.c b/src/channel-display.c
index 7c14c76..ebb8695 100755
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -659,7 +659,10 @@ static void image_put_lossy(SpiceImageCache *cache, uint64_t id,
 static void image_replace_lossy(SpiceImageCache *cache, uint64_t id,
                                 pixman_image_t *surface)
 {
-    image_put(cache, id, surface);
+    SpiceDisplayChannelPrivate *c =
+        SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate, image_cache);
+
+    cache_replace_lossy(c->images, id, pixman_image_ref(surface), FALSE);
 }
 
 static pixman_image_t* image_get_lossless(SpiceImageCache *cache, uint64_t id)
diff --git a/src/spice-channel-cache.h b/src/spice-channel-cache.h
index ca1fd5d..da88a39 100755
--- a/src/spice-channel-cache.h
+++ b/src/spice-channel-cache.h
@@ -101,6 +101,14 @@ static inline void cache_add_lossy(display_cache *cache, uint64_t id,
     g_hash_table_replace(cache->table, item, value);
 }
 
+static inline void cache_replace_lossy(display_cache *cache, uint64_t id,
+                                   gpointer value, gboolean lossy)
+{
+    display_cache_item *item = cache_item_new(id, lossy);
+
+    g_hash_table_replace(cache->table, item, value);
+}
+
 static inline void cache_add(display_cache *cache, uint64_t id, gpointer value)
 {
     cache_add_lossy(cache, id, value, FALSE);
-- 
2.5.5

From: Christophe Fergeau
Date: 2018-07-17 18:07
To: Frediano Ziglio
CC: spice-devel; 乐义华
Subject: Re:  [PATCH spice-gtk] If replace me, should not do refcount plus one
On Tue, Jul 17, 2018 at 08:17:53AM +0100, Frediano Ziglio wrote:
> From: 乐义华 <yueyihua@xxxxxxxxxxx>
> 
> When an image with SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME is received the
> reference counter should stay the same to avoid the cache to grow
> continously.
> 
> This fixes https://gitlab.freedesktop.org/spice/spice-gtk/issues/72
 
Can you reword the shortlog a bit?
 
Christophe
 
> 
> Signed-off-by: 乐义华 <yueyihua@xxxxxxxxxxx>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
>  src/channel-display.c     |  5 ++++-
>  src/spice-channel-cache.h | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 44ba0439..138cd8ce 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -788,7 +788,10 @@ static void image_put_lossy(SpiceImageCache *cache, uint64_t id,
>  static void image_replace_lossy(SpiceImageCache *cache, uint64_t id,
>                                  pixman_image_t *surface)
>  {
> -    image_put(cache, id, surface);
> +    SpiceDisplayChannelPrivate *c =
> +        SPICE_CONTAINEROF(cache, SpiceDisplayChannelPrivate, image_cache);
> +
> +    cache_replace_lossy(c->images, id, pixman_image_ref(surface), FALSE);
>  }
>  
>  static pixman_image_t* image_get_lossless(SpiceImageCache *cache, uint64_t id)
> diff --git a/src/spice-channel-cache.h b/src/spice-channel-cache.h
> index 75cc2cd8..fc25c354 100644
> --- a/src/spice-channel-cache.h
> +++ b/src/spice-channel-cache.h
> @@ -101,6 +101,23 @@ static inline void cache_add_lossy(display_cache *cache, uint64_t id,
>      g_hash_table_replace(cache->table, item, value);
>  }
>  
> +static inline void cache_replace_lossy(display_cache *cache, uint64_t id,
> +                                       gpointer value, gboolean lossy)
> +{
> +    display_cache_item *item = cache_item_new(id, lossy);
> +
> +    // If image is currently in the table consider its reference count before replacing it
> +    if (cache->ref_counted) {
> +        display_cache_item *current_item;
> +        gpointer current_image;
> +        if (g_hash_table_lookup_extended(cache->table, &id, (gpointer*) &current_item,
> +                                         &current_image)) {
> +            item->ref_count = current_item->ref_count;
> +        }
> +    }
> +    g_hash_table_replace(cache->table, item, value);
> +}
> +
>  static inline void cache_add(display_cache *cache, uint64_t id, gpointer value)
>  {
>      cache_add_lossy(cache, id, value, FALSE);
> -- 
> 2.17.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment:
0001-Fix-image-cache-memory-usage.patch

Description: Binary data
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




