[Spice-devel] [PATCH 16/30] Move others glz fields to dcc-encoders

Frediano Ziglio fziglio at redhat.com
Tue Jun 7 10:17:54 UTC 2016


Note that in this patch glz_drawable_count is removed.
This was used only to print some statistics.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/dcc-encoders.c    | 69 ++++++++++++++++++++++++------------------------
 server/dcc-encoders.h    | 18 +++++++------
 server/dcc.c             | 17 +++++-------
 server/dcc.h             |  5 ----
 server/display-channel.c | 16 +++++------
 server/display-channel.h |  2 --
 server/red-worker.c      |  6 ++---
 7 files changed, 60 insertions(+), 73 deletions(-)

diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index cc2d371..814cae7 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -26,7 +26,7 @@
 
 #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
 
-static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
+static void dcc_free_glz_drawable_instance(EncodersData *enc,
                                            GlzDrawableInstanceItem *item);
 
 static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
@@ -329,10 +329,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr, GlzUsrImageContext *im
 {
     GlzData *lz_data = (GlzData *)usr;
     GlzDrawableInstanceItem *glz_drawable_instance = (GlzDrawableInstanceItem *)image;
-    DisplayChannelClient *drawable_cc = glz_drawable_instance->glz_drawable->dcc;
-    DisplayChannelClient *this_cc = SPICE_CONTAINEROF(lz_data, DisplayChannelClient, encoders.glz_data);
-    if (this_cc == drawable_cc) {
-        dcc_free_glz_drawable_instance(drawable_cc, glz_drawable_instance);
+    EncodersData *drawable_enc = glz_drawable_instance->glz_drawable->encoders;
+    EncodersData *this_enc = SPICE_CONTAINEROF(lz_data, EncodersData, glz_data);
+    if (this_enc == drawable_enc) {
+        dcc_free_glz_drawable_instance(drawable_enc, glz_drawable_instance);
     } else {
         /* The glz dictionary is shared between all DisplayChannelClient
          * instances that belong to the same client, and glz_usr_free_image
@@ -341,10 +341,10 @@ static void glz_usr_free_image(GlzEncoderUsrContext *usr, GlzUsrImageContext *im
          * called from any DisplayChannelClient thread, hence the need for
          * this check.
          */
-        pthread_mutex_lock(&drawable_cc->glz_drawables_inst_to_free_lock);
+        pthread_mutex_lock(&drawable_enc->glz_drawables_inst_to_free_lock);
         ring_add_before(&glz_drawable_instance->free_link,
-                        &drawable_cc->glz_drawables_inst_to_free);
-        pthread_mutex_unlock(&drawable_cc->glz_drawables_inst_to_free_lock);
+                        &drawable_enc->glz_drawables_inst_to_free);
+        pthread_mutex_unlock(&drawable_enc->glz_drawables_inst_to_free_lock);
     }
 }
 
@@ -400,6 +400,10 @@ static void dcc_init_zlib(EncodersData *enc)
 
 void dcc_encoders_init(EncodersData *enc)
 {
+    ring_init(&enc->glz_drawables);
+    ring_init(&enc->glz_drawables_inst_to_free);
+    pthread_mutex_init(&enc->glz_drawables_inst_to_free_lock, NULL);
+
     dcc_init_glz_data(enc);
     dcc_init_quic(enc);
     dcc_init_lz(enc);
@@ -434,10 +438,9 @@ void dcc_encoders_free(EncodersData *enc)
    it is not used by Drawable).
    NOTE - 1) can be called only by the display channel that created the drawable
           2) it is assumed that the instance was already removed from the dictionary*/
-static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
+static void dcc_free_glz_drawable_instance(EncodersData *enc,
                                            GlzDrawableInstanceItem *instance)
 {
-    DisplayChannel *display_channel = DCC_TO_DC(dcc);
     RedGlzDrawable *glz_drawable;
 
     spice_assert(instance);
@@ -445,7 +448,7 @@ static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
 
     glz_drawable = instance->glz_drawable;
 
-    spice_assert(glz_drawable->dcc == dcc);
+    spice_assert(glz_drawable->encoders == enc);
     spice_assert(glz_drawable->instances_count > 0);
 
     ring_remove(&instance->glz_link);
@@ -466,7 +469,6 @@ static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
             ring_remove(&glz_drawable->drawable_link);
         }
         red_drawable_unref(glz_drawable->red_drawable);
-        display_channel->glz_drawable_count--;
         if (ring_item_is_linked(&glz_drawable->link)) {
             ring_remove(&glz_drawable->link);
         }
@@ -480,7 +482,7 @@ static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
  * if possible.
  * NOTE - the caller should prevent encoding using the dictionary during this operation
  */
-void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
+void dcc_free_glz_drawable(EncodersData *enc, RedGlzDrawable *drawable)
 {
     RingItem *head_instance = ring_get_head(&drawable->instances);
     int cont = (head_instance != NULL);
@@ -495,11 +497,11 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
                                                         glz_link);
         if (!ring_item_is_linked(&instance->free_link)) {
             // the instance didn't get out from window yet
-            glz_enc_dictionary_remove_image(dcc->encoders.glz_dict->dict,
+            glz_enc_dictionary_remove_image(enc->glz_dict->dict,
                                             instance->context,
-                                            &dcc->encoders.glz_data.usr);
+                                            &enc->glz_data.usr);
         }
-        dcc_free_glz_drawable_instance(dcc, instance);
+        dcc_free_glz_drawable_instance(enc, instance);
 
         if (cont) {
             head_instance = ring_get_head(&drawable->instances);
@@ -512,48 +514,48 @@ void dcc_free_glz_drawable(DisplayChannelClient *dcc, RedGlzDrawable *drawable)
  * Drawable (their qxl drawables are released too).
  * NOTE - the caller should prevent encoding using the dictionary during the operation
  */
-int dcc_free_some_independent_glz_drawables(DisplayChannelClient *dcc, int n)
+int dcc_free_some_independent_glz_drawables(EncodersData *enc, int n)
 {
     RingItem *ring_link;
 
-    if (!dcc) {
+    if (!enc) {
         return n;
     }
-    ring_link = ring_get_head(&dcc->glz_drawables);
+    ring_link = ring_get_head(&enc->glz_drawables);
     while ((n < RED_RELEASE_BUNCH_SIZE) && (ring_link != NULL)) {
         RedGlzDrawable *glz_drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
-        ring_link = ring_next(&dcc->glz_drawables, ring_link);
+        ring_link = ring_next(&enc->glz_drawables, ring_link);
         if (!glz_drawable->drawable) {
-            dcc_free_glz_drawable(dcc, glz_drawable);
+            dcc_free_glz_drawable(enc, glz_drawable);
             n++;
         }
     }
     return n;
 }
 
-void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
+void dcc_free_glz_drawables_to_free(EncodersData* enc)
 {
     RingItem *ring_link;
 
-    if (!dcc->encoders.glz_dict) {
+    if (!enc->glz_dict) {
         return;
     }
-    pthread_mutex_lock(&dcc->glz_drawables_inst_to_free_lock);
-    while ((ring_link = ring_get_head(&dcc->glz_drawables_inst_to_free))) {
+    pthread_mutex_lock(&enc->glz_drawables_inst_to_free_lock);
+    while ((ring_link = ring_get_head(&enc->glz_drawables_inst_to_free))) {
         GlzDrawableInstanceItem *drawable_instance = SPICE_CONTAINEROF(ring_link,
                                                                  GlzDrawableInstanceItem,
                                                                  free_link);
-        dcc_free_glz_drawable_instance(dcc, drawable_instance);
+        dcc_free_glz_drawable_instance(enc, drawable_instance);
     }
-    pthread_mutex_unlock(&dcc->glz_drawables_inst_to_free_lock);
+    pthread_mutex_unlock(&enc->glz_drawables_inst_to_free_lock);
 }
 
 /* Clear all lz drawables - enforce their removal from the global dictionary.
    NOTE - prevents encoding using the dictionary during the operation*/
-void dcc_free_glz_drawables(DisplayChannelClient *dcc)
+void dcc_free_glz_drawables(EncodersData *enc)
 {
     RingItem *ring_link;
-    GlzSharedDictionary *glz_dict = dcc ? dcc->encoders.glz_dict : NULL;
+    GlzSharedDictionary *glz_dict = enc ? enc->glz_dict : NULL;
 
     if (!glz_dict) {
         return;
@@ -561,11 +563,11 @@ void dcc_free_glz_drawables(DisplayChannelClient *dcc)
 
     // assure no display channel is during global lz encoding
     pthread_rwlock_wrlock(&glz_dict->encode_lock);
-    while ((ring_link = ring_get_head(&dcc->glz_drawables))) {
+    while ((ring_link = ring_get_head(&enc->glz_drawables))) {
         RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
         // no need to lock the to_free list, since we assured no other thread is encoding and
         // thus not other thread access the to_free list of the channel
-        dcc_free_glz_drawable(dcc, drawable);
+        dcc_free_glz_drawable(enc, drawable);
     }
     pthread_rwlock_unlock(&glz_dict->encode_lock);
 }
@@ -691,12 +693,11 @@ gboolean dcc_glz_encoder_create(EncodersData *enc, uint8_t id)
 }
 
 /* destroy encoder, and dictionary if no one uses it*/
-void dcc_release_glz(DisplayChannelClient *dcc)
+void dcc_release_glz(EncodersData *enc)
 {
-    EncodersData *enc = &dcc->encoders;
     GlzSharedDictionary *shared_dict;
 
-    dcc_free_glz_drawables(dcc);
+    dcc_free_glz_drawables(enc);
 
     glz_encoder_destroy(enc->glz);
     enc->glz = NULL;
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 90122fe..2d54c9d 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -38,15 +38,13 @@ typedef struct EncodersData EncodersData;
 
 void dcc_encoders_init(EncodersData *enc);
 void dcc_encoders_free(EncodersData *enc);
-void             dcc_free_glz_drawable                       (DisplayChannelClient *dcc,
-                                                              RedGlzDrawable *drawable);
-int              dcc_free_some_independent_glz_drawables     (DisplayChannelClient *dcc,
-                                                              int release_count);
-void             dcc_free_glz_drawables                      (DisplayChannelClient *dcc);
-void             dcc_free_glz_drawables_to_free              (DisplayChannelClient* dcc);
+void dcc_free_glz_drawable(EncodersData *enc, RedGlzDrawable *drawable);
+int dcc_free_some_independent_glz_drawables(EncodersData *enc, int release_count);
+void dcc_free_glz_drawables(EncodersData *enc);
+void dcc_free_glz_drawables_to_free(EncodersData* enc);
 gboolean dcc_glz_encoder_create(EncodersData *enc, uint8_t id);
 void dcc_freeze_glz(EncodersData *enc);
-void             dcc_release_glz                             (DisplayChannelClient *dcc);
+void dcc_release_glz(EncodersData *enc);
 
 #define RED_COMPRESS_BUF_SIZE (1024 * 64)
 struct RedCompressBuf {
@@ -162,7 +160,7 @@ struct RedGlzDrawable {
     GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
     Ring instances;
     uint8_t instances_count;
-    DisplayChannelClient *dcc;
+    EncodersData *encoders;
 };
 
 struct EncodersData {
@@ -191,6 +189,10 @@ struct EncodersData {
     GlzSharedDictionary *glz_dict;
     GlzEncoderContext *glz;
     GlzData glz_data;
+
+    Ring glz_drawables;               // all the living lz drawable, ordered by encoding time
+    Ring glz_drawables_inst_to_free;               // list of instances to be freed
+    pthread_mutex_t glz_drawables_inst_to_free_lock;
 };
 
 typedef struct compress_send_data_t {
diff --git a/server/dcc.c b/server/dcc.c
index 05258ee..1b6a089 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -392,10 +392,6 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
 
     dcc_init_stream_agents(dcc);
 
-    ring_init(&dcc->glz_drawables);
-    ring_init(&dcc->glz_drawables_inst_to_free);
-    pthread_mutex_init(&dcc->glz_drawables_inst_to_free_lock, NULL);
-
     dcc_encoders_init(&dcc->encoders);
 
     return dcc;
@@ -493,7 +489,7 @@ void dcc_stop(DisplayChannelClient *dcc)
 
     pixmap_cache_unref(dcc->pixmap_cache);
     dcc->pixmap_cache = NULL;
-    dcc_release_glz(dcc);
+    dcc_release_glz(&dcc->encoders);
     dcc_palette_cache_reset(dcc);
     free(dcc->send_data.stream_outbuf);
     free(dcc->send_data.free_list.res);
@@ -638,7 +634,7 @@ void dcc_destroy_surface(DisplayChannelClient *dcc, uint32_t surface_id)
 
 /* if already exists, returns it. Otherwise allocates and adds it (1) to the ring tail
    in the channel (2) to the Drawable*/
-static RedGlzDrawable *get_glz_drawable(DisplayChannelClient *dcc, Drawable *drawable)
+static RedGlzDrawable *get_glz_drawable(EncodersData *enc, Drawable *drawable)
 {
     RedGlzDrawable *ret;
     RingItem *item, *next;
@@ -647,14 +643,14 @@ static RedGlzDrawable *get_glz_drawable(DisplayChannelClient *dcc, Drawable *dra
     // now that we have multiple glz_dicts, so the only way to go from dcc to drawable glz is to go
     // over the glz_ring (unless adding some better data structure then a ring)
     DRAWABLE_FOREACH_GLZ_SAFE(drawable, item, next, ret) {
-        if (ret->dcc == dcc) {
+        if (ret->encoders == enc) {
             return ret;
         }
     }
 
     ret = spice_new(RedGlzDrawable, 1);
 
-    ret->dcc = dcc;
+    ret->encoders = enc;
     ret->red_drawable = red_drawable_ref(drawable->red_drawable);
     ret->drawable = drawable;
     ret->instances_count = 0;
@@ -662,9 +658,8 @@ static RedGlzDrawable *get_glz_drawable(DisplayChannelClient *dcc, Drawable *dra
 
     ring_item_init(&ret->link);
     ring_item_init(&ret->drawable_link);
-    ring_add_before(&ret->link, &dcc->glz_drawables);
+    ring_add_before(&ret->link, &enc->glz_drawables);
     ring_add(&drawable->glz_ring, &ret->drawable_link);
-    DCC_TO_DC(dcc)->glz_drawable_count++;
     return ret;
 }
 
@@ -724,7 +719,7 @@ static int dcc_compress_image_glz(DisplayChannelClient *dcc,
 
     encoder_data_init(&glz_data->data);
 
-    glz_drawable = get_glz_drawable(dcc, drawable);
+    glz_drawable = get_glz_drawable(&dcc->encoders, drawable);
     glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
 
     glz_data->data.u.lines_data.chunks = src->data;
diff --git a/server/dcc.h b/server/dcc.h
index cf8fe8b..5637a35 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -83,11 +83,6 @@ struct DisplayChannelClient {
         int num_pixmap_cache_items;
     } send_data;
 
-    /* global lz encoding entities */
-    Ring glz_drawables;               // all the living lz drawable, ordered by encoding time
-    Ring glz_drawables_inst_to_free;               // list of instances to be freed
-    pthread_mutex_t glz_drawables_inst_to_free_lock;
-
     uint8_t surface_client_created[NUM_SURFACES];
     QRegion surface_client_lossy_region[NUM_SURFACES];
 
diff --git a/server/display-channel.c b/server/display-channel.c
index a54c62b..6b6b542 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1202,7 +1202,7 @@ void display_channel_free_glz_drawables_to_free(DisplayChannel *display)
     spice_return_if_fail(display);
 
     FOREACH_CLIENT(display, link, next, dcc) {
-        dcc_free_glz_drawables_to_free(dcc);
+        dcc_free_glz_drawables_to_free(&dcc->encoders);
     }
 }
 
@@ -1214,7 +1214,7 @@ void display_channel_free_glz_drawables(DisplayChannel *display)
     spice_return_if_fail(display);
 
     FOREACH_CLIENT(display, link, next, dcc) {
-        dcc_free_glz_drawables(dcc);
+        dcc_free_glz_drawables(&dcc->encoders);
     }
 }
 
@@ -1233,7 +1233,7 @@ static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
         RingItem *glz_item, *next_item;
         RedGlzDrawable *glz;
         DRAWABLE_FOREACH_GLZ_SAFE(drawable, glz_item, next_item, glz) {
-            dcc_free_glz_drawable(glz->dcc, glz);
+            dcc_free_glz_drawable(glz->encoders, glz);
         }
     }
     drawable_draw(display, drawable);
@@ -1258,8 +1258,7 @@ void display_channel_free_some(DisplayChannel *display)
     DisplayChannelClient *dcc;
     GList *link, *next;
 
-    spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
-                display->glz_drawable_count);
+    spice_debug("#draw=%d", display->drawable_count);
     FOREACH_CLIENT(display, link, next, dcc) {
         GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
 
@@ -1267,7 +1266,7 @@ void display_channel_free_some(DisplayChannel *display)
             // encoding using the dictionary is prevented since the following operations might
             // change the dictionary
             pthread_rwlock_wrlock(&glz_dict->encode_lock);
-            n = dcc_free_some_independent_glz_drawables(dcc, n);
+            n = dcc_free_some_independent_glz_drawables(&dcc->encoders, n);
         }
     }
 
@@ -1910,9 +1909,8 @@ static void on_disconnect(RedChannelClient *rcc)
     display_channel_compress_stats_print(display);
 
     // this was the last channel client
-    spice_debug("#draw=%d, #glz_draw=%d",
-                display->drawable_count,
-                display->glz_drawable_count);
+    spice_debug("#draw=%d",
+                display->drawable_count);
 }
 
 static int handle_migrate_flush_mark(RedChannelClient *rcc)
diff --git a/server/display-channel.h b/server/display-channel.h
index 16ea709..45032cb 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -183,8 +183,6 @@ struct DisplayChannel {
     _Drawable drawables[NUM_DRAWABLES];
     _Drawable *free_drawables;
 
-    uint32_t glz_drawable_count;
-
     int stream_video;
     uint32_t stream_count;
     Stream streams_buf[NUM_STREAMS];
diff --git a/server/red-worker.c b/server/red-worker.c
index e754bd2..2e81f00 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -814,9 +814,8 @@ static void handle_dev_oom(void *opaque, void *payload)
 
     spice_return_if_fail(worker->running);
     // streams? but without streams also leak
-    spice_debug("OOM1 #draw=%u, #glz_draw=%u current %u pipes %u",
+    spice_debug("OOM1 #draw=%u, current %u pipes %u",
                 display->drawable_count,
-                display->glz_drawable_count,
                 display->current_size,
                 red_channel_sum_pipes_size(display_red_channel));
     while (red_process_display(worker, &ring_is_empty)) {
@@ -826,9 +825,8 @@ static void handle_dev_oom(void *opaque, void *payload)
         display_channel_free_some(worker->display_channel);
         red_qxl_flush_resources(worker->qxl);
     }
-    spice_debug("OOM2 #draw=%u, #glz_draw=%u current %u pipes %u",
+    spice_debug("OOM2 #draw=%u, current %u pipes %u",
                 display->drawable_count,
-                display->glz_drawable_count,
                 display->current_size,
                 red_channel_sum_pipes_size(display_red_channel));
     red_qxl_clear_pending(worker->qxl->st, RED_DISPATCHER_PENDING_OOM);
-- 
2.7.4



More information about the Spice-devel mailing list