[Spice-commits] 3 commits - server/cache-item.h server/cache-item.tmpl.cpp server/common-graphics-channel.h server/cursor-channel.cpp server/dcc-send.cpp

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Jun 23 12:24:19 UTC 2020


 server/cache-item.h              |   13 ++---------
 server/cache-item.tmpl.cpp       |   46 ++++++++++++++++++++++++---------------
 server/common-graphics-channel.h |    6 +++++
 server/cursor-channel.cpp        |    9 ++-----
 server/dcc-send.cpp              |    9 ++-----
 5 files changed, 44 insertions(+), 39 deletions(-)

New commits:
commit dc65afb03a00eb2f3602ab9c7c900b6ac38d1a8f
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue Jun 9 04:03:48 2020 +0100

    common-graphics-channel: Use marshaller structure for RedCachePipeItem
    
    Allows to simplify a bit marshalling code in both CursorChannel
    and DisplayChannel as they use the same marshalling structure.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/cache-item.tmpl.cpp b/server/cache-item.tmpl.cpp
index 89e75144..2b4755f3 100644
--- a/server/cache-item.tmpl.cpp
+++ b/server/cache-item.tmpl.cpp
@@ -77,7 +77,7 @@ static void FUNC_NAME(remove)(CHANNELCLIENT *channel_client, RedCacheItem *item)
     RedCachePipeItem *pipe_item = reinterpret_cast<RedCachePipeItem*>(item);
 
     red_pipe_item_init(&pipe_item->base, RED_PIPE_ITEM_TYPE_INVAL_ONE);
-    pipe_item->id = id;
+    pipe_item->inval_one.id = id;
     channel_client->pipe_add_tail(&pipe_item->base); // for now
 }
 
diff --git a/server/common-graphics-channel.h b/server/common-graphics-channel.h
index c7c91faa..455e2d05 100644
--- a/server/common-graphics-channel.h
+++ b/server/common-graphics-channel.h
@@ -71,7 +71,7 @@ protected:
 /* pipe item used to release a specific cached item on the client */
 struct RedCachePipeItem {
     RedPipeItem base;
-    uint64_t id;
+    SpiceMsgDisplayInvalOne inval_one;
 };
 
 #include "pop-visibility.h"
diff --git a/server/cursor-channel.cpp b/server/cursor-channel.cpp
index cae2a72d..05b7228a 100644
--- a/server/cursor-channel.cpp
+++ b/server/cursor-channel.cpp
@@ -168,12 +168,9 @@ static inline void red_marshall_inval(RedChannelClient *rcc,
                                       SpiceMarshaller *base_marshaller,
                                       RedCachePipeItem *cache_item)
 {
-    SpiceMsgDisplayInvalOne inval_one;
-
     rcc->init_send_data(SPICE_MSG_CURSOR_INVAL_ONE);
-    inval_one.id = cache_item->id;
 
-    spice_marshall_msg_cursor_inval_one(base_marshaller, &inval_one);
+    spice_marshall_msg_cursor_inval_one(base_marshaller, &cache_item->inval_one);
 }
 
 void CursorChannelClient::send_item(RedPipeItem *pipe_item)
diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index b6b4d5b4..727507fb 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -1742,12 +1742,9 @@ static inline void marshall_inval_palette(RedChannelClient *rcc,
                                           SpiceMarshaller *base_marshaller,
                                           RedCachePipeItem *cache_item)
 {
-    SpiceMsgDisplayInvalOne inval_one;
-
     rcc->init_send_data(SPICE_MSG_DISPLAY_INVAL_PALETTE);
-    inval_one.id = cache_item->id;
 
-    spice_marshall_msg_display_inval_palette(base_marshaller, &inval_one);
+    spice_marshall_msg_display_inval_palette(base_marshaller, &cache_item->inval_one);
 
 }
 
commit e7cdaa668d42937760669dab924f8908c925353b
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Tue Jun 9 03:51:37 2020 +0100

    cache-item: Move RedCachePipeItem declaration to common-graphics-channel.h
    
    The pipe items are meant to be used by channel clients, so move
    declaration where channel clients is.
    This item is used by both CursorChannelClient and DisplayChannelClient
    so the CommonGraphicsChannel is the proper place.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/cache-item.h b/server/cache-item.h
index 610d1b8f..e0e1f4c1 100644
--- a/server/cache-item.h
+++ b/server/cache-item.h
@@ -23,12 +23,6 @@
 
 #include "red-pipe-item.h"
 
-/* pipe item used to release a specific cached item on the client */
-struct RedCachePipeItem {
-    RedPipeItem base;
-    uint64_t id;
-};
-
 struct RedCacheItem {
     RingItem lru_link;
     RedCacheItem *next;
diff --git a/server/common-graphics-channel.h b/server/common-graphics-channel.h
index fe235591..c7c91faa 100644
--- a/server/common-graphics-channel.h
+++ b/server/common-graphics-channel.h
@@ -68,6 +68,12 @@ protected:
     virtual bool config_socket() override;
 };
 
+/* pipe item used to release a specific cached item on the client */
+struct RedCachePipeItem {
+    RedPipeItem base;
+    uint64_t id;
+};
+
 #include "pop-visibility.h"
 
 #endif /* COMMON_GRAPHICS_CHANNEL_H_ */
commit 255f6b2fd34661e07cf9d58d5a2206878c367f2b
Author: Frediano Ziglio <freddy77 at gmail.com>
Date:   Fri Jun 5 07:58:41 2020 +0100

    cache-item: Simplify structure used for just memory optimization
    
    RedCacheItem was using an union to reuse cache item memory
    as a pipe item to release that specific cache item.
    Instead of spreading that structure everywhere move the specific
    optimization all in cache-item.tmpl.cpp.
    This make also code less cluttered.
    Add some comment on specific code.
    
    Signed-off-by: Frediano Ziglio <freddy77 at gmail.com>
    Acked-by: Julien Ropé <jrope at gmail.com>

diff --git a/server/cache-item.h b/server/cache-item.h
index f0002a86..610d1b8f 100644
--- a/server/cache-item.h
+++ b/server/cache-item.h
@@ -23,17 +23,16 @@
 
 #include "red-pipe-item.h"
 
-typedef struct RedCacheItem RedCacheItem;
+/* pipe item used to release a specific cached item on the client */
+struct RedCachePipeItem {
+    RedPipeItem base;
+    uint64_t id;
+};
 
 struct RedCacheItem {
-    union {
-        RedPipeItem pipe_data;
-        struct {
-            RingItem lru_link;
-            RedCacheItem *next;
-            size_t size;
-        } cache_data;
-    } u;
+    RingItem lru_link;
+    RedCacheItem *next;
+    size_t size;
     uint64_t id;
 };
 
diff --git a/server/cache-item.tmpl.cpp b/server/cache-item.tmpl.cpp
index 8aaadafd..89e75144 100644
--- a/server/cache-item.tmpl.cpp
+++ b/server/cache-item.tmpl.cpp
@@ -46,11 +46,11 @@ static RedCacheItem *FUNC_NAME(find)(CHANNELCLIENT *channel_client, uint64_t id)
 
     while (item) {
         if (item->id == id) {
-            ring_remove(&item->u.cache_data.lru_link);
-            ring_add(&channel_client->priv->VAR_NAME(lru), &item->u.cache_data.lru_link);
+            ring_remove(&item->lru_link);
+            ring_add(&channel_client->priv->VAR_NAME(lru), &item->lru_link);
             break;
         }
-        item = item->u.cache_data.next;
+        item = item->next;
     }
     return item;
 }
@@ -64,16 +64,21 @@ static void FUNC_NAME(remove)(CHANNELCLIENT *channel_client, RedCacheItem *item)
     for (;;) {
         spice_assert(*now);
         if (*now == item) {
-            *now = item->u.cache_data.next;
+            *now = item->next;
             break;
         }
-        now = &(*now)->u.cache_data.next;
+        now = &(*now)->next;
     }
-    ring_remove(&item->u.cache_data.lru_link);
-    channel_client->priv->VAR_NAME(available) += item->u.cache_data.size;
+    ring_remove(&item->lru_link);
+    channel_client->priv->VAR_NAME(available) += item->size;
 
-    red_pipe_item_init(&item->u.pipe_data, RED_PIPE_ITEM_TYPE_INVAL_ONE);
-    channel_client->pipe_add_tail(&item->u.pipe_data); // for now
+    // see "Optimization" comment on add function below
+    auto id = item->id;
+    RedCachePipeItem *pipe_item = reinterpret_cast<RedCachePipeItem*>(item);
+
+    red_pipe_item_init(&pipe_item->base, RED_PIPE_ITEM_TYPE_INVAL_ONE);
+    pipe_item->id = id;
+    channel_client->pipe_add_tail(&pipe_item->base); // for now
 }
 
 static int FUNC_NAME(add)(CHANNELCLIENT *channel_client, uint64_t id, size_t size)
@@ -81,13 +86,20 @@ static int FUNC_NAME(add)(CHANNELCLIENT *channel_client, uint64_t id, size_t siz
     RedCacheItem *item;
     int key;
 
-    item = g_new(RedCacheItem, 1);
+    /* Optimization: allocate memory in order to be able to store
+     * both cache item and pipe item to be able to reuse it when
+     * we need to remove cache telling client */
+    union RedCachePoolItem {
+        RedCacheItem cache_item;
+        RedCachePipeItem pipe_item;
+    };
+    item = (RedCacheItem *) g_new(RedCachePoolItem, 1);
 
     channel_client->priv->VAR_NAME(available) -= size;
-    SPICE_VERIFY(SPICE_OFFSETOF(RedCacheItem, u.cache_data.lru_link) == 0);
+    SPICE_VERIFY(SPICE_OFFSETOF(RedCacheItem, lru_link) == 0);
     while (channel_client->priv->VAR_NAME(available) < 0) {
         RedCacheItem *tail = SPICE_CONTAINEROF(ring_get_tail(&channel_client->priv->VAR_NAME(lru)),
-                                                             RedCacheItem, u.cache_data.lru_link);
+                                                             RedCacheItem, lru_link);
         if (!tail) {
             channel_client->priv->VAR_NAME(available) += size;
             g_free(item);
@@ -95,12 +107,12 @@ static int FUNC_NAME(add)(CHANNELCLIENT *channel_client, uint64_t id, size_t siz
         }
         FUNC_NAME(remove)(channel_client, tail);
     }
-    item->u.cache_data.next = channel_client->priv->CACHE_NAME[(key = CACHE_HASH_KEY(id))];
+    item->next = channel_client->priv->CACHE_NAME[(key = CACHE_HASH_KEY(id))];
     channel_client->priv->CACHE_NAME[key] = item;
-    ring_item_init(&item->u.cache_data.lru_link);
-    ring_add(&channel_client->priv->VAR_NAME(lru), &item->u.cache_data.lru_link);
+    ring_item_init(&item->lru_link);
+    ring_add(&channel_client->priv->VAR_NAME(lru), &item->lru_link);
     item->id = id;
-    item->u.cache_data.size = size;
+    item->size = size;
     return TRUE;
 }
 
@@ -111,7 +123,7 @@ static void FUNC_NAME(reset)(CHANNELCLIENT *channel_client, long size)
     for (i = 0; i < CACHE_HASH_SIZE; i++) {
         while (channel_client->priv->CACHE_NAME[i]) {
             RedCacheItem *item = channel_client->priv->CACHE_NAME[i];
-            channel_client->priv->CACHE_NAME[i] = item->u.cache_data.next;
+            channel_client->priv->CACHE_NAME[i] = item->next;
             g_free(item);
         }
     }
diff --git a/server/cursor-channel.cpp b/server/cursor-channel.cpp
index c8d4a227..cae2a72d 100644
--- a/server/cursor-channel.cpp
+++ b/server/cursor-channel.cpp
@@ -166,12 +166,12 @@ static void red_marshall_cursor(CursorChannelClient *ccc,
 
 static inline void red_marshall_inval(RedChannelClient *rcc,
                                       SpiceMarshaller *base_marshaller,
-                                      RedCacheItem *cach_item)
+                                      RedCachePipeItem *cache_item)
 {
     SpiceMsgDisplayInvalOne inval_one;
 
     rcc->init_send_data(SPICE_MSG_CURSOR_INVAL_ONE);
-    inval_one.id = cach_item->id;
+    inval_one.id = cache_item->id;
 
     spice_marshall_msg_cursor_inval_one(base_marshaller, &inval_one);
 }
@@ -186,7 +186,7 @@ void CursorChannelClient::send_item(RedPipeItem *pipe_item)
         red_marshall_cursor(ccc, m, SPICE_UPCAST(RedCursorPipeItem, pipe_item));
         break;
     case RED_PIPE_ITEM_TYPE_INVAL_ONE:
-        red_marshall_inval(this, m, SPICE_CONTAINEROF(pipe_item, RedCacheItem, u.pipe_data));
+        red_marshall_inval(this, m, SPICE_UPCAST(RedCachePipeItem, pipe_item));
         break;
     case RED_PIPE_ITEM_TYPE_CURSOR_INIT:
         reset_cursor_cache();
diff --git a/server/dcc-send.cpp b/server/dcc-send.cpp
index 333682e1..b6b4d5b4 100644
--- a/server/dcc-send.cpp
+++ b/server/dcc-send.cpp
@@ -1740,7 +1740,7 @@ static bool red_marshall_stream_data(DisplayChannelClient *dcc,
 
 static inline void marshall_inval_palette(RedChannelClient *rcc,
                                           SpiceMarshaller *base_marshaller,
-                                          RedCacheItem *cache_item)
+                                          RedCachePipeItem *cache_item)
 {
     SpiceMsgDisplayInvalOne inval_one;
 
@@ -2345,7 +2345,7 @@ void DisplayChannelClient::send_item(RedPipeItem *pipe_item)
         break;
     }
     case RED_PIPE_ITEM_TYPE_INVAL_ONE:
-        marshall_inval_palette(this, m, SPICE_CONTAINEROF(pipe_item, RedCacheItem, u.pipe_data));
+        marshall_inval_palette(this, m, SPICE_UPCAST(RedCachePipeItem, pipe_item));
         break;
     case RED_PIPE_ITEM_TYPE_STREAM_CREATE: {
         StreamCreateDestroyItem *item = SPICE_UPCAST(StreamCreateDestroyItem, pipe_item);


More information about the Spice-commits mailing list