[Spice-devel] [PATCH v3 03/17] Move others glz fields to dcc-encoders

Frediano Ziglio fziglio at redhat.com
Fri Jun 10 08:48:02 UTC 2016


Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/dcc-encoders.c    | 76 +++++++++++++++++++++++++-----------------------
 server/dcc-encoders.h    | 19 +++++++-----
 server/dcc.c             | 18 +++++-------
 server/dcc.h             |  5 ----
 server/display-channel.c | 12 ++++----
 server/display-channel.h |  2 --
 server/red-worker.c      |  4 +--
 7 files changed, 66 insertions(+), 70 deletions(-)

diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 94b07fe..656ce25 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -26,8 +26,8 @@
 
 #define ZLIB_DEFAULT_COMPRESSION_LEVEL 3
 
-static void dcc_free_glz_drawable_instance(DisplayChannelClient *dcc,
-                                           GlzDrawableInstanceItem *item);
+static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
+                                                      GlzDrawableInstanceItem *instance);
 
 static SPICE_GNUC_NORETURN SPICE_GNUC_PRINTF(2, 3) void
 quic_usr_error(QuicUsrContext *usr, const char *fmt, ...)
@@ -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);
+    ImageEncoders *drawable_enc = glz_drawable_instance->glz_drawable->encoders;
+    ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders, glz_data);
+    if (this_enc == drawable_enc) {
+        image_encoders_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);
     }
 }
 
@@ -403,6 +403,10 @@ void image_encoders_init(ImageEncoders *enc, ImageEncoderGlobals *globals)
     spice_assert(globals);
     enc->globals = globals;
 
+    ring_init(&enc->glz_drawables);
+    ring_init(&enc->glz_drawables_inst_to_free);
+    pthread_mutex_init(&enc->glz_drawables_inst_to_free_lock, NULL);
+
     image_encoders_init_glz_data(enc);
     image_encoders_init_quic(enc);
     image_encoders_init_lz(enc);
@@ -437,10 +441,9 @@ void image_encoders_free(ImageEncoders *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,
-                                           GlzDrawableInstanceItem *instance)
+static void image_encoders_free_glz_drawable_instance(ImageEncoders *enc,
+                                                      GlzDrawableInstanceItem *instance)
 {
-    DisplayChannel *display_channel = DCC_TO_DC(dcc);
     RedGlzDrawable *glz_drawable;
 
     spice_assert(instance);
@@ -448,7 +451,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);
@@ -469,7 +472,7 @@ 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--;
+        enc->globals->glz_drawable_count--;
         if (ring_item_is_linked(&glz_drawable->link)) {
             ring_remove(&glz_drawable->link);
         }
@@ -483,14 +486,14 @@ 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 image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable *drawable)
 {
     RingItem *head_instance = ring_get_head(&drawable->instances);
     int cont = (head_instance != NULL);
 
     while (cont) {
         if (drawable->instances_count == 1) {
-            /* Last instance: dcc_free_glz_drawable_instance will free the drawable */
+            /* Last instance: image_encoders_free_glz_drawable_instance will free the drawable */
             cont = FALSE;
         }
         GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
@@ -498,11 +501,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);
+        image_encoders_free_glz_drawable_instance(enc, instance);
 
         if (cont) {
             head_instance = ring_get_head(&drawable->instances);
@@ -515,49 +518,49 @@ 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 image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
 {
     RingItem *ring_link;
     int n = 0;
 
-    if (!dcc) {
+    if (!enc) {
         return 0;
     }
-    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);
+            image_encoders_free_glz_drawable(enc, glz_drawable);
             n++;
         }
     }
     return n;
 }
 
-void dcc_free_glz_drawables_to_free(DisplayChannelClient* dcc)
+void image_encoders_free_glz_drawables_to_free(ImageEncoders* 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);
+        image_encoders_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 image_encoders_free_glz_drawables(ImageEncoders *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;
@@ -565,11 +568,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);
+        image_encoders_free_glz_drawable(enc, drawable);
     }
     pthread_rwlock_unlock(&glz_dict->encode_lock);
 }
@@ -695,12 +698,11 @@ gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id)
 }
 
 /* destroy encoder, and dictionary if no one uses it*/
-void dcc_release_glz(DisplayChannelClient *dcc)
+void image_encoders_release_glz(ImageEncoders *enc)
 {
-    ImageEncoders *enc = &dcc->encoders;
     GlzSharedDictionary *shared_dict;
 
-    dcc_free_glz_drawables(dcc);
+    image_encoders_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 51f0cfe..b32b34b 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -43,14 +43,13 @@ void image_encoder_globals_stat_print(const ImageEncoderGlobals *globals);
 
 void image_encoders_init(ImageEncoders *enc, ImageEncoderGlobals *globals);
 void image_encoders_free(ImageEncoders *enc);
-void             dcc_free_glz_drawable                       (DisplayChannelClient *dcc,
-                                                              RedGlzDrawable *drawable);
-int              dcc_free_some_independent_glz_drawables     (DisplayChannelClient *dcc);
-void             dcc_free_glz_drawables                      (DisplayChannelClient *dcc);
-void             dcc_free_glz_drawables_to_free              (DisplayChannelClient* dcc);
+void image_encoders_free_glz_drawable(ImageEncoders *enc, RedGlzDrawable *drawable);
+int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
+void image_encoders_free_glz_drawables(ImageEncoders *enc);
+void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
 gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
 void image_encoders_freeze_glz(ImageEncoders *enc);
-void             dcc_release_glz                             (DisplayChannelClient *dcc);
+void image_encoders_release_glz(ImageEncoders *enc);
 
 #define RED_COMPRESS_BUF_SIZE (1024 * 64)
 struct RedCompressBuf {
@@ -166,10 +165,12 @@ struct RedGlzDrawable {
     GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
     Ring instances;
     uint8_t instances_count;
-    DisplayChannelClient *dcc;
+    ImageEncoders *encoders;
 };
 
 struct ImageEncoderGlobals {
+    uint32_t glz_drawable_count;
+
     stat_info_t off_stat;
     stat_info_t lz_stat;
     stat_info_t glz_stat;
@@ -208,6 +209,10 @@ struct ImageEncoders {
     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 1cc85b5..82862af 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);
-
     image_encoders_init(&dcc->encoders, &display->encoder_globals);
 
     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);
+    image_encoders_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(ImageEncoders *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,9 @@ 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++;
+    enc->globals->glz_drawable_count++;
     return ret;
 }
 
@@ -724,7 +720,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 a4b917b..362237d 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 4aa912a..c48db00 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1143,7 +1143,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);
+        image_encoders_free_glz_drawables_to_free(&dcc->encoders);
     }
 }
 
@@ -1155,7 +1155,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);
+        image_encoders_free_glz_drawables(&dcc->encoders);
     }
 }
 
@@ -1174,7 +1174,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);
+            image_encoders_free_glz_drawable(glz->encoders, glz);
         }
     }
     drawable_draw(display, drawable);
@@ -1200,7 +1200,7 @@ void display_channel_free_some(DisplayChannel *display)
     GList *link, *next;
 
     spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
-                display->glz_drawable_count);
+                display->encoder_globals.glz_drawable_count);
     FOREACH_CLIENT(display, link, next, dcc) {
         GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
 
@@ -1208,7 +1208,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 = image_encoders_free_some_independent_glz_drawables(&dcc->encoders);
         }
     }
 
@@ -1853,7 +1853,7 @@ static void on_disconnect(RedChannelClient *rcc)
     // this was the last channel client
     spice_debug("#draw=%d, #glz_draw=%d",
                 display->drawable_count,
-                display->glz_drawable_count);
+                display->encoder_globals.glz_drawable_count);
 }
 
 static int handle_migrate_flush_mark(RedChannelClient *rcc)
diff --git a/server/display-channel.h b/server/display-channel.h
index cdf1dce..8bf4d5d 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 c53bc15..aba3e6f 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -823,7 +823,7 @@ static void handle_dev_oom(void *opaque, void *payload)
     // streams? but without streams also leak
     spice_debug("OOM1 #draw=%u, #glz_draw=%u current %u pipes %u",
                 display->drawable_count,
-                display->glz_drawable_count,
+                display->encoder_globals.glz_drawable_count,
                 display->current_size,
                 red_channel_sum_pipes_size(display_red_channel));
     while (red_process_display(worker, &ring_is_empty)) {
@@ -835,7 +835,7 @@ static void handle_dev_oom(void *opaque, void *payload)
     }
     spice_debug("OOM2 #draw=%u, #glz_draw=%u current %u pipes %u",
                 display->drawable_count,
-                display->glz_drawable_count,
+                display->encoder_globals.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