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

"yueyihua@xxxxxxxxxxx" <yueyihua@xxxxxxxxxxx> · Wed, 18 Jul 2018 16:02:43 +0800

Oh, I am sorry, it is a mistake, should refcount but not refconut.


乐义华
系统开发工程师 Tel：13545059071
E-mail：yueyihua@xxxxxxxxxxx
QQ：113061543
武汉噢易云计算股份有限公司
总部地址：中国·光谷武汉市关山大道465号光谷创意大厦C座17层
客服热线：4001-027-580
客服邮箱：market@xxxxxxxxxxx
官方网站：www.os-easy.com
 
From: Frediano Ziglio
Date: 2018-07-18 15:57
To: yueyihua
CC: Christophe Fergeau; spice-devel
Subject: Re: [Spice-devel] [PATCH spice-gtk] If replace me, should not do refcount plus one


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.

Was the removal of refcount copy on purpose or a mistake?

typo: refconut -> refcount

Frediano

---
 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: [Spice-devel] [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


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




