Note that in this patch glz_drawable_count is removed. This was used only to print some statistics. Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> --- server/dcc-encoders.c | 75 ++++++++++++++++++++++++------------------------ server/dcc-encoders.h | 17 ++++++----- 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, 63 insertions(+), 75 deletions(-) diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c index 82d7a1a..3a4910c 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); } } @@ -400,6 +400,10 @@ static void image_encoders_init_zlib(ImageEncoders *enc) void image_encoders_init(ImageEncoders *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); + image_encoders_init_glz_data(enc); image_encoders_init_quic(enc); image_encoders_init_lz(enc); @@ -434,10 +438,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); @@ -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,14 +482,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, @@ -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); + image_encoders_free_glz_drawable_instance(enc, instance); if (cont) { head_instance = ring_get_head(&drawable->instances); @@ -512,49 +514,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; @@ -562,11 +564,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); } @@ -692,12 +694,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 70c1adc..cde48b8 100644 --- a/server/dcc-encoders.h +++ b/server/dcc-encoders.h @@ -38,14 +38,13 @@ typedef struct ImageEncoders ImageEncoders; void image_encoders_init(ImageEncoders *enc); 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 { @@ -161,7 +160,7 @@ struct RedGlzDrawable { GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES]; Ring instances; uint8_t instances_count; - DisplayChannelClient *dcc; + ImageEncoders *encoders; }; struct ImageEncoders { @@ -190,6 +189,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 a828e52..fb06cdc 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); 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,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 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 57f77d4..163c746 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); + image_encoders_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); + image_encoders_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); + image_encoders_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 = image_encoders_free_some_independent_glz_drawables(&dcc->encoders); } } @@ -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 c53bc15..bb65f8a 100644 --- a/server/red-worker.c +++ b/server/red-worker.c @@ -821,9 +821,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)) { @@ -833,9 +832,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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel