The code was quite complicated.
- a GlzEncDictImageContext was bound to a GlzDrawableInstanceItem
(1-1 relation);
- multiple GlzDrawableInstanceItem were attached to RedGlzDrawable;
- multiple RedGlzDrawable (one for RedClient) were attached to Drawable
using GlzImageRetention;
- OOM code require all Glz dictionary used by DisplayChannel to be locked
at the same time;
- drawable count computation during OOM was not corrent in case of
multiple clients;
- not clear why to call glz_retention_free_drawables or
glz_retention_detach_drawables.
Now:
- a GlzEncDictImageContext is bound to a GlzDictItem (still 1-1 relation);
- multiple GlzDictItem are attached to a Drawable using GlzImageRetention;
- there is only a glz_retention_free to be called when the structure
must be destroyed.
Implementation detail:
- red_drawable_unref returns a gboolean to indicate if the object
was really freed;
- glz_drawable_count was removed as there are no more RedGlzDrawable;
- image_encoders_glz_encode_lock and image_encoders_glz_encode_unlock
were removed and now the locking is handled entirely by ImageEncoders;
- instead of marking GlzDictItem/GlzDrawableInstanceItem changing
has_drawable flag contexts are moved to a glz_orphans ring;
- code is smaller (100 lines less in image-encoders.c) and I hope easier
to read.
Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
server/display-channel.c | 53 +++++----
server/image-encoders.c | 300 +++++++++++++++--------------------------------
server/image-encoders.h | 27 +++--
server/red-worker.c | 11 +-
server/red-worker.h | 2 +-
5 files changed, 146 insertions(+), 247 deletions(-)
diff --git a/server/display-channel.c b/server/display-channel.c
index 073d45e..6d25239 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -1167,7 +1167,7 @@ void display_channel_free_glz_drawables(DisplayChannel *display)
}
}
-static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
+static bool free_one_drawable(DisplayChannel *display)
{
RingItem *ring_item = ring_get_tail(&display->current_list);
Drawable *drawable;
@@ -1178,9 +1178,6 @@ static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
}
drawable = SPICE_CONTAINEROF(ring_item, Drawable, list_link);
- if (force_glz_free) {
- glz_retention_free_drawables(&drawable->glz_retention);
- }
drawable_draw(display, drawable);
container = drawable->tree_item.base.container;
@@ -1192,33 +1189,44 @@ static bool free_one_drawable(DisplayChannel *display, int force_glz_free)
void display_channel_current_flush(DisplayChannel *display, int surface_id)
{
while (!ring_is_empty(&display->surfaces[surface_id].current_list)) {
- free_one_drawable(display, FALSE);
+ free_one_drawable(display);
}
current_remove_all(display, surface_id);
}
void display_channel_free_some(DisplayChannel *display)
{
- int n = 0;
+ int left_to_free = RED_RELEASE_BUNCH_SIZE;
DisplayChannelClient *dcc;
GList *link, *next;
- spice_debug("#draw=%d, #glz_draw=%d", display->drawable_count,
- display->encoder_shared_data.glz_drawable_count);
+ /* As we are trying to release some memory as requested by guest driver
+ * loop througth Glz freeing drawables which have no corresponding Drawable
+ * as this will cause guest memory to be freed. */
+ spice_debug("#draw=%d", display->drawable_count);
FOREACH_CLIENT(display, link, next, dcc) {
- // 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);
- }
+ left_to_free -= image_encoders_free_some_independent_glz_drawables(&dcc->encoders, left_to_free);
}
- while (!ring_is_empty(&display->current_list) && n++ < RED_RELEASE_BUNCH_SIZE) {
- free_one_drawable(display, TRUE);
- }
+ if (left_to_free > 0) {
+ int saved_to_free = left_to_free;
- FOREACH_CLIENT(display, link, next, dcc) {
- image_encoders_glz_encode_unlock(&dcc->encoders);
+ while (left_to_free > 0 && !ring_is_empty(&display->current_list)) {
+ free_one_drawable(display);
+ --left_to_free;
+ }
+
+ /* We freed Drawables in the loop above but the underlaying RedDrawables
+ * could still be referenced by Glz so scan again the Glz for independent
+ * drawables. Note that there is a 1-to-1 relatioship between Drawable
+ * and RedDrawable so in theory there will be a maximum of saved_to_free
+ * RedDrawables to free. Use this limit in any case, this should not be
+ * a problem and will make sure we won't free more items that we are
+ * supposed to free. */
+ left_to_free = saved_to_free;
+ FOREACH_CLIENT(display, link, next, dcc) {
+ left_to_free -= image_encoders_free_some_independent_glz_drawables(&dcc->encoders, left_to_free);
+ }
}
}
@@ -1263,7 +1271,7 @@ static Drawable *display_channel_drawable_try_new(DisplayChannel *display,
Drawable *drawable;
while (!(drawable = drawable_try_new(display))) {
- if (!free_one_drawable(display, FALSE))
+ if (!free_one_drawable(display))
return NULL;
}
@@ -1341,7 +1349,7 @@ void drawable_unref(Drawable *drawable)
drawable_unref_surface_deps(display, drawable);
display_channel_surface_unref(display, drawable->surface_id);
- glz_retention_detach_drawables(&drawable->glz_retention);
+ glz_retention_destroy(&drawable->glz_retention);
if (drawable->red_drawable) {
red_drawable_unref(drawable->red_drawable);
@@ -1845,9 +1853,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->encoder_shared_data.glz_drawable_count);
+ spice_debug("#draw=%d",
+ display->drawable_count);
}
static int handle_migrate_flush_mark(RedChannelClient *rcc)
diff --git a/server/image-encoders.c b/server/image-encoders.c
index 5759230..0ade8c8 100644
--- a/server/image-encoders.c
+++ b/server/image-encoders.c
@@ -30,11 +30,6 @@
#define ENCODER_MESSAGE_SIZE 512
-#define MAX_GLZ_DRAWABLE_INSTANCES 2
-
-typedef struct RedGlzDrawable RedGlzDrawable;
-typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem;
-
struct GlzSharedDictionary {
RingItem base;
GlzEncDictContext *dict;
@@ -45,37 +40,28 @@ struct GlzSharedDictionary {
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:
- * Drawable -> (ring of) RedGlzDrawable -> (up to 2) GlzDrawableInstanceItem
- * and it should probably (but need to be sure...) be
- * Drawable -> ring of GlzDrawableInstanceItem.
+/**
+ * This structure is used to retain RedDrawable objects so they don't
+ * disappear while we are using their memory.
+ * This is because Glz will keep using image memory for future compressions.
*/
-struct GlzDrawableInstanceItem {
- RingItem glz_link;
+typedef struct GlzDictItem {
+ /** linked to ImageEncoders::glz_dict_items. Always inserted */
+ RingItem glz_context_link;
+ /** linked to GlzImageRetention::ring or ImageEncoders::orphans. Always inserted */
+ RingItem retention_link;
+ /** linked to ImageEncoders::glz_dict_items_to_free */
RingItem free_link;
- GlzEncDictImageContext *context;
- RedGlzDrawable *glz_drawable;
-};
-
-struct RedGlzDrawable {
- RingItem link; // ordered by the time it was encoded
- RingItem drawable_link;
+ /** context pointer from Glz encoder, used to free the drawable */
+ GlzEncDictImageContext *dict_image_context;
+ /** where the drawable came from */
+ ImageEncoders *enc;
+ /** hold a reference to this object, this to make sure Glz
+ * can continue to use its memory */
RedDrawable *red_drawable;
- GlzDrawableInstanceItem instances_pool[MAX_GLZ_DRAWABLE_INSTANCES];
- Ring instances;
- uint8_t instances_count;
- gboolean has_drawable;
- ImageEncoders *encoders;
-};
-
-#define LINK_TO_GLZ(ptr) SPICE_CONTAINEROF((ptr), RedGlzDrawable, \
- drawable_link)
-#define DRAWABLE_FOREACH_GLZ_SAFE(drawable, link, next, glz) \
- SAFE_FOREACH(link, next, drawable, &(drawable)->glz_retention.ring, glz, LINK_TO_GLZ(link))
+} GlzDictItem;
-static void glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance);
+static gboolean glz_context_free(GlzDictItem *glz_item);
static void encoder_data_init(EncoderData *data);
static void encoder_data_reset(EncoderData *data);
static void image_encoders_release_glz(ImageEncoders *enc);
@@ -382,23 +368,24 @@ static void image_encoders_init_lz(ImageEncoders *enc)
static void glz_usr_free_image(GlzEncoderUsrContext *usr, GlzUsrImageContext *image)
{
GlzData *lz_data = (GlzData *)usr;
- GlzDrawableInstanceItem *glz_drawable_instance = (GlzDrawableInstanceItem *)image;
- ImageEncoders *drawable_enc = glz_drawable_instance->glz_drawable->encoders;
+ GlzDictItem *glz_item = (GlzDictItem *)image;
+ ImageEncoders *drawable_enc = glz_item->enc;
ImageEncoders *this_enc = SPICE_CONTAINEROF(lz_data, ImageEncoders, glz_data);
+ glz_item->dict_image_context = NULL;
if (this_enc == drawable_enc) {
- glz_drawable_instance_item_free(glz_drawable_instance);
+ glz_context_free(glz_item);
} else {
/* The glz dictionary is shared between all DisplayChannelClient
- * instances that belong to the same client, and glz_usr_free_image
+ * items that belong to the same client, and glz_usr_free_image
* can be called by the dictionary code
* (glz_dictionary_window_remove_head). Thus this function can be
* called from any DisplayChannelClient thread, hence the need for
* this check.
*/
- pthread_mutex_lock(&drawable_enc->glz_drawables_inst_to_free_lock);
- ring_add_before(&glz_drawable_instance->free_link,
- &drawable_enc->glz_drawables_inst_to_free);
- pthread_mutex_unlock(&drawable_enc->glz_drawables_inst_to_free_lock);
+ pthread_mutex_lock(&drawable_enc->glz_dict_items_to_free_lock);
+ ring_add_before(&glz_item->free_link,
+ &drawable_enc->glz_dict_items_to_free);
+ pthread_mutex_unlock(&drawable_enc->glz_dict_items_to_free_lock);
}
}
@@ -457,9 +444,10 @@ void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData *shared_data
spice_assert(shared_data);
enc->shared_data = shared_data;
- ring_init(&enc->glz_drawables);
- ring_init(&enc->glz_drawables_inst_to_free);
- pthread_mutex_init(&enc->glz_drawables_inst_to_free_lock, NULL);
+ ring_init(&enc->glz_orphans);
+ ring_init(&enc->glz_dict_items);
+ ring_init(&enc->glz_dict_items_to_free);
+ pthread_mutex_init(&enc->glz_dict_items_to_free_lock, NULL);
image_encoders_init_glz_data(enc);
image_encoders_init_quic(enc);
@@ -489,121 +477,63 @@ void image_encoders_free(ImageEncoders *enc)
#endif
zlib_encoder_destroy(enc->zlib);
enc->zlib = NULL;
- pthread_mutex_destroy(&enc->glz_drawables_inst_to_free_lock);
+ pthread_mutex_destroy(&enc->glz_dict_items_to_free_lock);
}
/* Remove from the to_free list and the instances_list.
- When no instance is left - the RedGlzDrawable is released too. (and the qxl drawable too, if
- 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 glz_drawable_instance_item_free(GlzDrawableInstanceItem *instance)
+ Returns TRUE if a RedDrawable was freed.
+ NOTE - can be called only by the display channel that created the drawable */
+static gboolean glz_context_free(GlzDictItem *glz_item)
{
- RedGlzDrawable *glz_drawable;
-
- spice_assert(instance);
- spice_assert(instance->glz_drawable);
+ gboolean ret;
- glz_drawable = instance->glz_drawable;
+ spice_assert(glz_item);
- spice_assert(glz_drawable->instances_count > 0);
-
- ring_remove(&instance->glz_link);
- glz_drawable->instances_count--;
+ ring_remove(&glz_item->glz_context_link);
+ ring_remove(&glz_item->retention_link);
// when the remove callback is performed from the channel that the
- // drawable belongs to, the instance is not added to the 'to_free' list
- if (ring_item_is_linked(&instance->free_link)) {
- ring_remove(&instance->free_link);
- }
-
- if (ring_is_empty(&glz_drawable->instances)) {
- spice_assert(glz_drawable->instances_count == 0);
-
- if (glz_drawable->has_drawable) {
- ring_remove(&glz_drawable->drawable_link);
- }
- red_drawable_unref(glz_drawable->red_drawable);
- glz_drawable->encoders->shared_data->glz_drawable_count--;
- if (ring_item_is_linked(&glz_drawable->link)) {
- ring_remove(&glz_drawable->link);
- }
- free(glz_drawable);
- }
-}
-
-/*
- * Releases all the instances of the drawable from the dictionary and the display channel client.
- * The release of the last instance will also release the drawable itself and the qxl drawable
- * if possible.
- * NOTE - the caller should prevent encoding using the dictionary during this operation
- */
-static void red_glz_drawable_free(RedGlzDrawable *glz_drawable)
-{
- ImageEncoders *enc = glz_drawable->encoders;
- RingItem *head_instance = ring_get_head(&glz_drawable->instances);
- int cont = (head_instance != NULL);
-
- while (cont) {
- if (glz_drawable->instances_count == 1) {
- /* Last instance: glz_drawable_instance_item_free will free the glz_drawable */
- cont = FALSE;
- }
- GlzDrawableInstanceItem *instance = SPICE_CONTAINEROF(head_instance,
- GlzDrawableInstanceItem,
- glz_link);
- if (!ring_item_is_linked(&instance->free_link)) {
- // the instance didn't get out from window yet
- glz_enc_dictionary_remove_image(enc->glz_dict->dict,
- instance->context,
- &enc->glz_data.usr);
- }
- glz_drawable_instance_item_free(instance);
-
- if (cont) {
- head_instance = ring_get_head(&glz_drawable->instances);
- }
+ // drawable belongs to, the item is not added to the 'to_free' list
+ if (ring_item_is_linked(&glz_item->free_link)) {
+ ring_remove(&glz_item->free_link);
+ } else if (glz_item->dict_image_context) {
+ ImageEncoders *enc = glz_item->enc;
+
+ // the item didn't get out from window yet
+ glz_enc_dictionary_remove_image(enc->glz_dict->dict,
+ glz_item->dict_image_context,
+ &enc->glz_data.usr);
}
-}
-gboolean image_encoders_glz_encode_lock(ImageEncoders *enc)
-{
- if (enc->glz_dict) {
- pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
- return TRUE;
- }
- return FALSE;
-}
+ ret = red_drawable_unref(glz_item->red_drawable);
+ free(glz_item);
-void image_encoders_glz_encode_unlock(ImageEncoders *enc)
-{
- if (enc->glz_dict) {
- pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
- }
+ return ret;
}
/*
- * Remove from the global lz dictionary some glz_drawables that have no reference to
+ * Remove from the global lz dictionary some glz_dict_items that have no reference to
* Drawable (their qxl drawables are released too).
- * NOTE - the caller should prevent encoding using the dictionary during the operation
*/
-int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc)
+int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc, int max_to_free)
{
RingItem *ring_link;
int n = 0;
- if (!enc) {
- return 0;
+ if (!enc || !enc->glz_dict || max_to_free <= 0) {
+ return n;
}
- 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(&enc->glz_drawables, ring_link);
- if (!glz_drawable->has_drawable) {
- red_glz_drawable_free(glz_drawable);
- n++;
+
+ // prevent encoding using the dictionary during the operation
+ pthread_rwlock_wrlock(&enc->glz_dict->encode_lock);
+ while (max_to_free > n
+ && (ring_link = ring_get_head(&enc->glz_orphans)) != NULL) {
+ GlzDictItem *glz_item = SPICE_CONTAINEROF(ring_link, GlzDictItem, retention_link);
+ if (glz_context_free(glz_item)) {
+ ++n;
}
}
+ pthread_rwlock_unlock(&enc->glz_dict->encode_lock);
return n;
}
@@ -614,14 +544,12 @@ void image_encoders_free_glz_drawables_to_free(ImageEncoders* enc)
if (!enc->glz_dict) {
return;
}
- 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);
- glz_drawable_instance_item_free(drawable_instance);
+ pthread_mutex_lock(&enc->glz_dict_items_to_free_lock);
+ while ((ring_link = ring_get_head(&enc->glz_dict_items_to_free))) {
+ GlzDictItem * glz_item = SPICE_CONTAINEROF(ring_link, GlzDictItem, free_link);
+ glz_context_free(glz_item);
}
- pthread_mutex_unlock(&enc->glz_drawables_inst_to_free_lock);
+ pthread_mutex_unlock(&enc->glz_dict_items_to_free_lock);
}
/* Clear all lz drawables - enforce their removal from the global dictionary.
@@ -637,31 +565,25 @@ void image_encoders_free_glz_drawables(ImageEncoders *enc)
// assure no display channel is during global lz encoding
pthread_rwlock_wrlock(&glz_dict->encode_lock);
- while ((ring_link = ring_get_head(&enc->glz_drawables))) {
- RedGlzDrawable *drawable = SPICE_CONTAINEROF(ring_link, RedGlzDrawable, link);
+ while ((ring_link = ring_get_head(&enc->glz_dict_items))) {
+ GlzDictItem *glz_item = SPICE_CONTAINEROF(ring_link, GlzDictItem, glz_context_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
- red_glz_drawable_free(drawable);
+ glz_context_free(glz_item);
}
pthread_rwlock_unlock(&glz_dict->encode_lock);
}
-void glz_retention_free_drawables(GlzImageRetention *ret)
+void glz_retention_destroy(GlzImageRetention *retention)
{
- RingItem *glz_item, *next_item;
- RedGlzDrawable *glz;
- SAFE_FOREACH(glz_item, next_item, TRUE, &ret->ring, glz, LINK_TO_GLZ(glz_item)) {
- red_glz_drawable_free(glz);
- }
-}
+ RingItem *head;
-void glz_retention_detach_drawables(GlzImageRetention *ret)
-{
- RingItem *item, *next;
+ /* append all ring to orphans one */
+ while ((head = ring_get_head(&retention->ring)) != NULL) {
+ GlzDictItem *glz_item = SPICE_CONTAINEROF(head, GlzDictItem, retention_link);
- RING_FOREACH_SAFE(item, next, &ret->ring) {
- SPICE_CONTAINEROF(item, RedGlzDrawable, drawable_link)->has_drawable = FALSE;
- ring_remove(item);
+ ring_remove(head);
+ ring_add_before(head, &glz_item->enc->glz_orphans);
}
}
@@ -1156,52 +1078,22 @@ int image_encoders_compress_lz4(ImageEncoders *enc, SpiceImage *dest,
/* 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(ImageEncoders *enc, RedDrawable *red_drawable,
- GlzImageRetention *glz_retention)
+static GlzDictItem *get_glz_context(ImageEncoders *enc, RedDrawable *red_drawable,
+ GlzImageRetention *glz_retention)
{
- RedGlzDrawable *ret;
- RingItem *item, *next;
-
- // TODO - I don't really understand what's going on here, so doing the technical equivalent
- // 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)
- SAFE_FOREACH(item, next, TRUE, &glz_retention->ring, ret, LINK_TO_GLZ(item)) {
- if (ret->encoders == enc) {
- return ret;
- }
- }
-
- ret = spice_new(RedGlzDrawable, 1);
-
- ret->encoders = enc;
- ret->red_drawable = red_drawable_ref(red_drawable);
- ret->has_drawable = TRUE;
- ret->instances_count = 0;
- ring_init(&ret->instances);
-
- ring_item_init(&ret->link);
- ring_item_init(&ret->drawable_link);
- ring_add_before(&ret->link, &enc->glz_drawables);
- ring_add(&glz_retention->ring, &ret->drawable_link);
- enc->shared_data->glz_drawable_count++;
- return ret;
-}
+ GlzDictItem *ret;
-/* allocates new instance and adds it to instances in the given drawable.
- NOTE - the caller should set the glz_instance returned by the encoder by itself.*/
-static GlzDrawableInstanceItem *add_glz_drawable_instance(RedGlzDrawable *glz_drawable)
-{
- spice_assert(glz_drawable->instances_count < MAX_GLZ_DRAWABLE_INSTANCES);
- // NOTE: We assume the additions are performed consecutively, without removals in the middle
- GlzDrawableInstanceItem *ret = glz_drawable->instances_pool + glz_drawable->instances_count;
- glz_drawable->instances_count++;
+ ret = spice_new(GlzDictItem, 1);
+ ring_item_init(&ret->glz_context_link);
+ ring_item_init(&ret->retention_link);
ring_item_init(&ret->free_link);
- ring_item_init(&ret->glz_link);
- ring_add(&glz_drawable->instances, &ret->glz_link);
- ret->context = NULL;
- ret->glz_drawable = glz_drawable;
+ ret->enc = enc;
+ ret->red_drawable = red_drawable_ref(red_drawable);
+
+ ring_add_before(&ret->glz_context_link, &enc->glz_dict_items);
+ ring_add(&glz_retention->ring, &ret->retention_link);
return ret;
}
@@ -1220,8 +1112,7 @@ int image_encoders_compress_glz(ImageEncoders *enc,
GlzData *glz_data = &enc->glz_data;
ZlibData *zlib_data;
LzImageType type = bitmap_fmt_to_lz_image_type[src->format];
- RedGlzDrawable *glz_drawable;
- GlzDrawableInstanceItem *glz_drawable_instance;
+ GlzDictItem *glz_item;
int glz_size;
int zlib_size;
@@ -1242,8 +1133,7 @@ int image_encoders_compress_glz(ImageEncoders *enc,
encoder_data_init(&glz_data->data);
- glz_drawable = get_glz_drawable(enc, red_drawable, glz_retention);
- glz_drawable_instance = add_glz_drawable_instance(glz_drawable);
+ glz_item = get_glz_context(enc, red_drawable, glz_retention);
glz_data->data.u.lines_data.chunks = src->data;
glz_data->data.u.lines_data.stride = src->stride;
@@ -1254,8 +1144,8 @@ int image_encoders_compress_glz(ImageEncoders *enc,
(src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL, 0,
src->stride, glz_data->data.bufs_head->buf.bytes,
sizeof(glz_data->data.bufs_head->buf),
- glz_drawable_instance,
- &glz_drawable_instance->context);
+ glz_item,
+ &glz_item->dict_image_context);
stat_compress_add(&enc->shared_data->glz_stat, start_time, src->stride * src->y, glz_size);
diff --git a/server/image-encoders.h b/server/image-encoders.h
index 5bad0d4..c715f3e 100644
--- a/server/image-encoders.h
+++ b/server/image-encoders.h
@@ -45,16 +45,18 @@ void image_encoder_shared_stat_print(const ImageEncoderSharedData *shared_data);
void image_encoders_init(ImageEncoders *enc, ImageEncoderSharedData *shared_data);
void image_encoders_free(ImageEncoders *enc);
-int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc);
+int image_encoders_free_some_independent_glz_drawables(ImageEncoders *enc, int max_to_free);
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_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 glz_retention_free_drawables(GlzImageRetention *ret);
-void glz_retention_detach_drawables(GlzImageRetention *ret);
+
+/**
+ * Free retention structure so Glz code that drawables
+ * are no more owned by something else.
+ */
+void glz_retention_destroy(GlzImageRetention *retention);
#define RED_COMPRESS_BUF_SIZE (1024 * 64)
struct RedCompressBuf {
@@ -136,14 +138,12 @@ struct GlzImageRetention {
Ring ring;
};
-static inline void glz_retention_init(GlzImageRetention *ret)
+static inline void glz_retention_init(GlzImageRetention *retention)
{
- ring_init(&ret->ring);
+ ring_init(&retention->ring);
}
struct ImageEncoderSharedData {
- uint32_t glz_drawable_count;
-
stat_info_t off_stat;
stat_info_t lz_stat;
stat_info_t glz_stat;
@@ -183,9 +183,12 @@ struct ImageEncoders {
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;
+ /** list to all Glz contexts that are owned only by Glz and could
+ * be freed in case of OOM */
+ Ring glz_orphans;
+ Ring glz_dict_items; // all the living lz drawable, ordered by encoding time
+ Ring glz_dict_items_to_free; // list of items to be freed
+ pthread_mutex_t glz_dict_items_to_free_lock;
};
typedef struct compress_send_data_t {
diff --git a/server/red-worker.c b/server/red-worker.c
index 9238632..bce6d69 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -123,14 +123,15 @@ static void common_release_recv_buf(RedChannelClient *rcc, uint16_t type, uint32
}
}
-void red_drawable_unref(RedDrawable *red_drawable)
+gboolean red_drawable_unref(RedDrawable *red_drawable)
{
if (--red_drawable->refs) {
- return;
+ return FALSE;
}
red_qxl_release_resource(red_drawable->qxl, red_drawable->release_info_ext);
red_put_drawable(red_drawable);
free(red_drawable);
+ return TRUE;
}
static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
@@ -821,9 +822,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->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)) {
@@ -833,9 +833,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->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);
diff --git a/server/red-worker.h b/server/red-worker.h
index 63be8b5..1639df8 100644
--- a/server/red-worker.h
+++ b/server/red-worker.h
@@ -92,7 +92,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
const ClientCbs *client_display_cbs);
bool red_worker_run(RedWorker *worker);
-void red_drawable_unref(RedDrawable *red_drawable);
+gboolean red_drawable_unref(RedDrawable *red_drawable);
CommonGraphicsChannel *red_worker_new_channel(RedWorker *worker, int size,
const char *name,
--
2.7.4
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel