[Spice-commits] 4 commits - server/dcc-encoders.c server/dcc-encoders.h server/dcc.c server/display-channel.c server/display-channel.h server/red-worker.c
Frediano Ziglio
fziglio at kemper.freedesktop.org
Thu Jun 16 20:05:12 UTC 2016
server/dcc-encoders.c | 41 +++++++++++++++++++++++++++++++++++++++++
server/dcc-encoders.h | 13 +++----------
server/dcc.c | 23 +++++++----------------
server/display-channel.c | 25 +++++++++----------------
server/display-channel.h | 2 +-
server/red-worker.c | 4 ++--
6 files changed, 63 insertions(+), 45 deletions(-)
New commits:
commit 0628ce62287b07b7e08daaed183b8e5874fed5a3
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Tue Jun 7 08:16:20 2016 +0100
Make GlzSharedDictionary structure private in dcc-encoders.c
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 156ce2f..ea68ac1 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -32,6 +32,16 @@
typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
+struct GlzSharedDictionary {
+ RingItem base;
+ GlzEncDictContext *dict;
+ uint32_t refs;
+ uint8_t id;
+ pthread_rwlock_t encode_lock;
+ int migrate_freeze;
+ RedClient *client; // channel clients of the same client share the dict
+};
+
/* for each qxl drawable, there may be several instances of lz drawables */
/* TODO - reuse this stuff for the top level. I just added a second level of multiplicity
* at the Drawable by keeping a ring, so:
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 10119eb..114dacd 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -35,6 +35,7 @@ typedef struct RedCompressBuf RedCompressBuf;
typedef struct RedGlzDrawable RedGlzDrawable;
typedef struct ImageEncoders ImageEncoders;
typedef struct ImageEncoderSharedData ImageEncoderSharedData;
+typedef struct GlzSharedDictionary GlzSharedDictionary;
void image_encoder_shared_init(ImageEncoderSharedData *shared_data);
void image_encoder_shared_stat_reset(ImageEncoderSharedData *shared_data);
@@ -71,16 +72,6 @@ static inline void compress_buf_free(RedCompressBuf *buf)
g_free(buf);
}
-typedef struct GlzSharedDictionary {
- RingItem base;
- GlzEncDictContext *dict;
- uint32_t refs;
- uint8_t id;
- pthread_rwlock_t encode_lock;
- int migrate_freeze;
- RedClient *client; // channel clients of the same client share the dict
-} GlzSharedDictionary;
-
gboolean image_encoders_get_glz_dictionary(ImageEncoders *enc,
struct RedClient *client,
uint8_t id, int window_size);
commit 141bcd1eec2896cd85b97c88484eeb168539529e
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Tue Jun 7 10:21:51 2016 +0100
Do not access ImageEncoders internal to lock/unlock glz encoding
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index 7a6a87d..156ce2f 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -552,6 +552,22 @@ static void red_glz_drawable_free(RedGlzDrawable *glz_drawable)
}
}
+gboolean image_encoders_glz_encode_lock(ImageEncoders *enc)
+{
+ if (enc->glz_dict) {
+ pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
+ return TRUE;
+ }
+ return FALSE;
+}
+
+void image_encoders_glz_encode_unlock(ImageEncoders *enc)
+{
+ if (enc->glz_dict) {
+ pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+ }
+}
+
/*
* Remove from the global lz dictionary some glz_drawables that have no reference to
* Drawable (their qxl drawables are released too).
diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h
index 284ed74..10119eb 100644
--- a/server/dcc-encoders.h
+++ b/server/dcc-encoders.h
@@ -48,6 +48,8 @@ void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc);
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id);
void image_encoders_glz_get_restore_data(ImageEncoders *enc,
uint8_t *out_id, GlzEncDictRestoreData *out_data);
+gboolean image_encoders_glz_encode_lock(ImageEncoders *enc);
+void image_encoders_glz_encode_unlock(ImageEncoders *enc);
void drawable_free_glz_drawables(struct Drawable *drawable);
void drawable_detach_glz_drawables(struct Drawable *drawable);
diff --git a/server/display-channel.c b/server/display-channel.c
index 4a0094b..2366a98 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1206,12 +1206,9 @@ void display_channel_free_some(DisplayChannel *display)
spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
display->encoder_shared_data.glz_drawable_count);
FOREACH_CLIENT(display, link, next, dcc) {
- GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
-
- if (glz_dict) {
- // encoding using the dictionary is prevented since the following operations might
- // change the dictionary
- pthread_rwlock_wrlock(&glz_dict->encode_lock);
+ // encoding using the dictionary is prevented since the following operations might
+ // change the dictionary
+ if (image_encoders_glz_encode_lock(&dcc->encoders)) {
n = image_encoders_free_some_independent_glz_drawables(&dcc->encoders);
}
}
@@ -1221,11 +1218,7 @@ void display_channel_free_some(DisplayChannel *display)
}
FOREACH_CLIENT(display, link, next, dcc) {
- GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
-
- if (glz_dict) {
- pthread_rwlock_unlock(&glz_dict->encode_lock);
- }
+ image_encoders_glz_encode_unlock(&dcc->encoders);
}
}
commit cc26a9a91ecee1a52db2b18a65ba19286180a7c0
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Mon Jun 6 09:48:52 2016 +0100
Better encapsulation for image_encoders_compress_glz call
Do not access too much encoders data.
Slightly different as now if glz is frozen lz compression is used.
Glz is frozen only during migration.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c
index e53fd24..7a6a87d 100644
--- a/server/dcc-encoders.c
+++ b/server/dcc-encoders.c
@@ -1195,6 +1195,17 @@ int image_encoders_compress_glz(ImageEncoders *enc,
spice_info("LZ global compress fmt=%d", src->format);
#endif
+ if ((src->x * src->y) >= glz_enc_dictionary_get_size(enc->glz_dict->dict)) {
+ return FALSE;
+ }
+
+ pthread_rwlock_rdlock(&enc->glz_dict->encode_lock);
+ /* using the global dictionary only if it is not frozen */
+ if (enc->glz_dict->migrate_freeze) {
+ pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+ return FALSE;
+ }
+
encoder_data_init(&glz_data->data);
glz_drawable = get_glz_drawable(enc, drawable);
@@ -1245,8 +1256,12 @@ int image_encoders_compress_glz(ImageEncoders *enc,
o_comp_data->comp_buf_size = zlib_size;
stat_compress_add(&enc->shared_data->zlib_glz_stat, start_time, glz_size, zlib_size);
+ pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
return TRUE;
+
glz:
+ pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
+
dest->descriptor.type = SPICE_IMAGE_TYPE_GLZ_RGB;
dest->u.lz_rgb.data_size = glz_size;
diff --git a/server/dcc.c b/server/dcc.c
index 099f5f6..e5c80ad 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -735,19 +735,10 @@ int dcc_compress_image(DisplayChannelClient *dcc,
success = image_encoders_compress_quic(&dcc->encoders, dest, src, o_comp_data);
break;
case SPICE_IMAGE_COMPRESSION_GLZ:
- if ((src->x * src->y) < glz_enc_dictionary_get_size(dcc->encoders.glz_dict->dict)) {
- int frozen;
- /* using the global dictionary only if it is not frozen */
- pthread_rwlock_rdlock(&dcc->encoders.glz_dict->encode_lock);
- frozen = dcc->encoders.glz_dict->migrate_freeze;
- if (!frozen) {
- success = image_encoders_compress_glz(&dcc->encoders, dest, src, drawable, o_comp_data,
- display_channel->enable_zlib_glz_wrap);
- }
- pthread_rwlock_unlock(&dcc->encoders.glz_dict->encode_lock);
- if (!frozen) {
- break;
- }
+ success = image_encoders_compress_glz(&dcc->encoders, dest, src, drawable, o_comp_data,
+ display_channel->enable_zlib_glz_wrap);
+ if (success) {
+ break;
}
goto lz_compress;
#ifdef USE_LZ4
commit 2d1c7243012f86fa6f04d50bd343799feafa1db0
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Wed Jun 15 23:12:50 2016 +0100
Rename encoder_globals to encoder_shared_data
Was missing due to a mistake.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
diff --git a/server/dcc.c b/server/dcc.c
index 48c009d..099f5f6 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -388,7 +388,7 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
dcc_init_stream_agents(dcc);
- image_encoders_init(&dcc->encoders, &display->encoder_globals);
+ image_encoders_init(&dcc->encoders, &display->encoder_shared_data);
return dcc;
}
@@ -720,7 +720,7 @@ int dcc_compress_image(DisplayChannelClient *dcc,
stat_start_time_t start_time;
int success = FALSE;
- stat_start_time_init(&start_time, &display_channel->encoder_globals.off_stat);
+ stat_start_time_init(&start_time, &display_channel->encoder_shared_data.off_stat);
image_compression = get_compression_for_bitmap(src, dcc->image_compression, drawable);
switch (image_compression) {
@@ -771,7 +771,7 @@ lz_compress:
if (!success) {
uint64_t image_size = src->stride * src->y;
- stat_compress_add(&display_channel->encoder_globals.off_stat, start_time, image_size, image_size);
+ stat_compress_add(&display_channel->encoder_shared_data.off_stat, start_time, image_size, image_size);
}
return success;
diff --git a/server/display-channel.c b/server/display-channel.c
index fae2e25..4a0094b 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -38,7 +38,7 @@ void display_channel_compress_stats_reset(DisplayChannel *display)
{
spice_return_if_fail(display);
- image_encoder_shared_stat_reset(&display->encoder_globals);
+ image_encoder_shared_stat_reset(&display->encoder_shared_data);
}
void display_channel_compress_stats_print(const DisplayChannel *display_channel)
@@ -47,7 +47,7 @@ void display_channel_compress_stats_print(const DisplayChannel *display_channel)
spice_return_if_fail(display_channel);
spice_info("==> Compression stats for display %u", display_channel->common.base.id);
- image_encoder_shared_stat_print(&display_channel->encoder_globals);
+ image_encoder_shared_stat_print(&display_channel->encoder_shared_data);
#endif
}
@@ -1204,7 +1204,7 @@ void display_channel_free_some(DisplayChannel *display)
GList *link, *next;
spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
- display->encoder_globals.glz_drawable_count);
+ display->encoder_shared_data.glz_drawable_count);
FOREACH_CLIENT(display, link, next, dcc) {
GlzSharedDictionary *glz_dict = dcc->encoders.glz_dict;
@@ -1854,7 +1854,7 @@ static void on_disconnect(RedChannelClient *rcc)
// this was the last channel client
spice_debug("#draw=%d, #glz_draw=%d",
display->drawable_count,
- display->encoder_globals.glz_drawable_count);
+ display->encoder_shared_data.glz_drawable_count);
}
static int handle_migrate_flush_mark(RedChannelClient *rcc)
@@ -1927,7 +1927,7 @@ DisplayChannel* display_channel_new(SpiceServer *reds, RedWorker *worker,
display->non_cache_counter = stat_add_counter(reds, channel->stat,
"non_cache", TRUE);
#endif
- image_encoder_shared_init(&display->encoder_globals);
+ image_encoder_shared_init(&display->encoder_shared_data);
display->n_surfaces = n_surfaces;
display->renderer = RED_RENDERER_INVALID;
diff --git a/server/display-channel.h b/server/display-channel.h
index 9a264a3..ec84f2d 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -208,7 +208,7 @@ struct DisplayChannel {
uint64_t *add_to_cache_counter;
uint64_t *non_cache_counter;
#endif
- ImageEncoderSharedData encoder_globals;
+ ImageEncoderSharedData encoder_shared_data;
};
static inline int get_stream_id(DisplayChannel *display, Stream *stream)
diff --git a/server/red-worker.c b/server/red-worker.c
index f8b1913..9238632 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->encoder_globals.glz_drawable_count,
+ display->encoder_shared_data.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->encoder_globals.glz_drawable_count,
+ display->encoder_shared_data.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);
More information about the Spice-commits
mailing list