[Spice-devel] [PATCH 4/5] server: move some pixmap cache code in own file

Frediano Ziglio fziglio at redhat.com
Fri Oct 16 04:57:43 PDT 2015


> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> Remove that hideous template header that should really be regular code
> since it's specialized and instanciated only for pixmap.
> ---
>  server/Makefile.am               |   3 +-
>  server/pixmap_cache.c            | 126 ++++++++++++++++++
>  server/pixmap_cache.h            |  59 +++++++++
>  server/red_client_shared_cache.h | 235 ----------------------------------
>  server/red_worker.c              | 269
>  +++++++++++++++++++++------------------
>  5 files changed, 332 insertions(+), 360 deletions(-)
>  create mode 100644 server/pixmap_cache.c
>  create mode 100644 server/pixmap_cache.h
>  delete mode 100644 server/red_client_shared_cache.h
> 
> diff --git a/server/Makefile.am b/server/Makefile.am
> index 78ccac9..dfa9ea9 100644
> --- a/server/Makefile.am
> +++ b/server/Makefile.am
> @@ -88,7 +88,6 @@ libspice_server_la_SOURCES =			\
>  	red_channel.c				\
>  	red_channel.h				\
>  	red_client_cache.h			\
> -	red_client_shared_cache.h		\
>  	red_common.h				\
>  	dispatcher.c				\
>  	dispatcher.h				\
> @@ -126,6 +125,8 @@ libspice_server_la_SOURCES =			\
>  	spice_server_utils.h		\
>  	spice_image_cache.h			\
>  	spice_image_cache.c			\
> +	pixmap_cache.h				\
> +	pixmap_cache.c				\
>  	$(NULL)
>  
>  if SUPPORT_GL
> diff --git a/server/pixmap_cache.c b/server/pixmap_cache.c
> new file mode 100644
> index 0000000..c6a2ac5
> --- /dev/null
> +++ b/server/pixmap_cache.c
> @@ -0,0 +1,126 @@
> +#include "pixmap_cache.h"
> +
> +int pixmap_cache_unlocked_set_lossy(PixmapCache *cache, uint64_t id, int
> lossy)
> +{
> +    NewCacheItem *item;
> +
> +    item = cache->hash_table[BITS_CACHE_HASH_KEY(id)];
> +
> +    while (item) {
> +        if (item->id == id) {
> +            item->lossy = lossy;
> +            break;
> +        }
> +        item = item->next;
> +    }
> +    return !!item;
> +}
> +
> +void pixmap_cache_clear(PixmapCache *cache)
> +{
> +    NewCacheItem *item;
> +
> +    if (cache->freezed) {
> +        cache->lru.next = cache->freezed_head;
> +        cache->lru.prev = cache->freezed_tail;
> +        cache->freezed = FALSE;
> +    }
> +
> +    while ((item = (NewCacheItem *)ring_get_head(&cache->lru))) {
> +        ring_remove(&item->lru_link);
> +        free(item);
> +    }
> +    memset(cache->hash_table, 0, sizeof(*cache->hash_table) *
> BITS_CACHE_HASH_SIZE);
> +
> +    cache->available = cache->size;
> +    cache->items = 0;
> +}
> +
> +int pixmap_cache_freeze(PixmapCache *cache)
> +{
> +    pthread_mutex_lock(&cache->lock);
> +
> +    if (cache->freezed) {
> +        pthread_mutex_unlock(&cache->lock);
> +        return FALSE;
> +    }
> +
> +    cache->freezed_head = cache->lru.next;
> +    cache->freezed_tail = cache->lru.prev;
> +    ring_init(&cache->lru);
> +    memset(cache->hash_table, 0, sizeof(*cache->hash_table) *
> BITS_CACHE_HASH_SIZE);
> +    cache->available = -1;
> +    cache->freezed = TRUE;
> +
> +    pthread_mutex_unlock(&cache->lock);
> +    return TRUE;
> +}
> +
> +static void pixmap_cache_destroy(PixmapCache *cache)
> +{
> +    spice_assert(cache);
> +
> +    pthread_mutex_lock(&cache->lock);
> +    pixmap_cache_clear(cache);
> +    pthread_mutex_unlock(&cache->lock);
> +}
> +
> +
> +static pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER;
> +static Ring pixmap_cache_list = {&pixmap_cache_list, &pixmap_cache_list};
> +
> +static PixmapCache *pixmap_cache_new(RedClient *client, uint8_t id, int64_t
> size)
> +{
> +    PixmapCache *cache = spice_new0(PixmapCache, 1);
> +
> +    ring_item_init(&cache->base);
> +    pthread_mutex_init(&cache->lock, NULL);
> +    cache->id = id;
> +    cache->refs = 1;
> +    ring_init(&cache->lru);
> +    cache->available = size;
> +    cache->size = size;
> +    cache->client = client;
> +
> +    return cache;
> +}
> +
> +PixmapCache *pixmap_cache_get(RedClient *client, uint8_t id, int64_t size)
> +{
> +    PixmapCache *ret = NULL;
> +    RingItem *now;
> +    pthread_mutex_lock(&cache_lock);
> +
> +    now = &pixmap_cache_list;
> +    while ((now = ring_next(&pixmap_cache_list, now))) {
> +        PixmapCache *cache = (PixmapCache *)now;
> +        if ((cache->client == client) && (cache->id == id)) {
> +            ret = cache;
> +            ret->refs++;
> +            break;
> +        }
> +    }
> +    if (!ret) {
> +        ret = pixmap_cache_new(client, id, size);
> +        ring_add(&pixmap_cache_list, &ret->base);
> +    }
> +    pthread_mutex_unlock(&cache_lock);
> +    return ret;
> +}
> +
> +
> +void pixmap_cache_unref(PixmapCache *cache)
> +{
> +    if (!cache)
> +        return;
> +
> +    pthread_mutex_lock(&cache_lock);
> +    if (--cache->refs) {
> +        pthread_mutex_unlock(&cache_lock);
> +        return;
> +    }
> +    ring_remove(&cache->base);
> +    pthread_mutex_unlock(&cache_lock);
> +    pixmap_cache_destroy(cache);
> +    free(cache);
> +}
> diff --git a/server/pixmap_cache.h b/server/pixmap_cache.h
> new file mode 100644
> index 0000000..011c49b
> --- /dev/null
> +++ b/server/pixmap_cache.h
> @@ -0,0 +1,59 @@
> +#ifndef _PIXMAP_CACHE_H
> +# define _PIXMAP_CACHE_H
> +
> +#include "red_channel.h"
> +#include "spice_server_utils.h"
> +
> +#define MAX_CACHE_CLIENTS 4
> +
> +#define BITS_CACHE_HASH_SHIFT 10
> +#define BITS_CACHE_HASH_SIZE (1 << BITS_CACHE_HASH_SHIFT)
> +#define BITS_CACHE_HASH_MASK (BITS_CACHE_HASH_SIZE - 1)
> +#define BITS_CACHE_HASH_KEY(id) ((id) & BITS_CACHE_HASH_MASK)
> +
> +typedef struct _DisplayChannelClient DisplayChannelClient;
> +
> +typedef struct _PixmapCache PixmapCache;
> +typedef struct _NewCacheItem NewCacheItem;
> +
> +struct _NewCacheItem {
> +    RingItem lru_link;
> +    NewCacheItem *next;
> +    uint64_t id;
> +    uint64_t sync[MAX_CACHE_CLIENTS];
> +    size_t size;
> +    int lossy;
> +};
> +
> +struct _PixmapCache {
> +    RingItem base;
> +    pthread_mutex_t lock;
> +    uint8_t id;
> +    uint32_t refs;
> +    NewCacheItem *hash_table[BITS_CACHE_HASH_SIZE];
> +    Ring lru;
> +    int64_t available;
> +    int64_t size;
> +    int32_t items;
> +
> +    int freezed;
> +    RingItem *freezed_head;
> +    RingItem *freezed_tail;
> +
> +    uint32_t generation;
> +    struct {
> +        uint8_t client;
> +        uint64_t message;
> +    } generation_initiator;
> +    uint64_t sync[MAX_CACHE_CLIENTS]; // here CLIENTS refer to different
> channel
> +                                      // clients of the same client
> +    RedClient *client;
> +};
> +
> +PixmapCache *pixmap_cache_get(RedClient *client, uint8_t id, int64_t size);
> +void         pixmap_cache_unref(PixmapCache *cache);
> +void         pixmap_cache_clear(PixmapCache *cache);
> +int          pixmap_cache_unlocked_set_lossy(PixmapCache *cache, uint64_t
> id, int lossy);
> +int          pixmap_cache_freeze(PixmapCache *cache);
> +
> +#endif /* _PIXMAP_CACHE_H */
> diff --git a/server/red_client_shared_cache.h
> b/server/red_client_shared_cache.h
> deleted file mode 100644
> index 7feb28e..0000000
> --- a/server/red_client_shared_cache.h
> +++ /dev/null
> @@ -1,235 +0,0 @@
> -/*
> -   Copyright (C) 2009 Red Hat, Inc.
> -
> -   This library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   This library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> -*/
> -
> -#if defined(CLIENT_PIXMAPS_CACHE)
> -
> -#define CACHE PixmapCache
> -
> -#define CACHE_NAME bits_cache
> -#define CACHE_HASH_KEY BITS_CACHE_HASH_KEY
> -#define CACHE_HASH_SIZE BITS_CACHE_HASH_SIZE
> -#define PIPE_ITEM_TYPE PIPE_ITEM_TYPE_INVAL_PIXMAP
> -#define FUNC_NAME(name) pixmap_cache_##name
> -#define PRIVATE_FUNC_NAME(name) __pixmap_cache_##name
> -#define CHANNEL DisplayChannel
> -#define CACH_GENERATION pixmap_cache_generation
> -#define INVAL_ALL_VERB SPICE_MSG_DISPLAY_INVAL_ALL_PIXMAPS
> -#else
> -
> -#error "no cache type."
> -
> -#endif
> -
> -#define CHANNEL_FROM_RCC(rcc) SPICE_CONTAINEROF((rcc)->channel, CHANNEL,
> common.base);
> -
> -static int FUNC_NAME(unlocked_hit)(CACHE *cache, uint64_t id, int *lossy,
> DisplayChannelClient *dcc)
> -{
> -    NewCacheItem *item;
> -    uint64_t serial;
> -
> -    serial = red_channel_client_get_message_serial(&dcc->common.base);
> -    item = cache->hash_table[CACHE_HASH_KEY(id)];
> -
> -    while (item) {
> -        if (item->id == id) {
> -            ring_remove(&item->lru_link);
> -            ring_add(&cache->lru, &item->lru_link);
> -            spice_assert(dcc->common.id < MAX_CACHE_CLIENTS);
> -            item->sync[dcc->common.id] = serial;
> -            cache->sync[dcc->common.id] = serial;
> -            *lossy = item->lossy;
> -            break;
> -        }
> -        item = item->next;
> -    }
> -
> -    return !!item;
> -}
> -
> -static int FUNC_NAME(hit)(CACHE *cache, uint64_t id, int *lossy,
> DisplayChannelClient *dcc)
> -{
> -    int hit;
> -    pthread_mutex_lock(&cache->lock);
> -    hit = FUNC_NAME(unlocked_hit)(cache,id,lossy, dcc);
> -    pthread_mutex_unlock(&cache->lock);
> -    return hit;
> -}
> -
> -static int FUNC_NAME(unlocked_set_lossy)(CACHE *cache, uint64_t id, int
> lossy)
> -{
> -    NewCacheItem *item;
> -
> -    item = cache->hash_table[CACHE_HASH_KEY(id)];
> -
> -    while (item) {
> -        if (item->id == id) {
> -            item->lossy = lossy;
> -            break;
> -        }
> -        item = item->next;
> -    }
> -    return !!item;
> -}
> -
> -static int FUNC_NAME(unlocked_add)(CACHE *cache, uint64_t id, uint32_t size,
> int lossy, DisplayChannelClient *dcc)
> -{
> -    NewCacheItem *item;
> -    uint64_t serial;
> -    int key;
> -
> -    spice_assert(size > 0);
> -
> -    item = spice_new(NewCacheItem, 1);
> -    serial = red_channel_client_get_message_serial(&dcc->common.base);
> -
> -    if (cache->generation != dcc->CACH_GENERATION) {
> -        if (!dcc->pending_pixmaps_sync) {
> -            red_channel_client_pipe_add_type(
> -                &dcc->common.base, PIPE_ITEM_TYPE_PIXMAP_SYNC);
> -            dcc->pending_pixmaps_sync = TRUE;
> -        }
> -        free(item);
> -        return FALSE;
> -    }
> -
> -    cache->available -= size;
> -    while (cache->available < 0) {
> -        NewCacheItem *tail;
> -        NewCacheItem **now;
> -
> -        if (!(tail = (NewCacheItem *)ring_get_tail(&cache->lru)) ||
> -
> tail->sync[dcc->common.id]
> == serial) {
> -            cache->available += size;
> -            free(item);
> -            return FALSE;
> -        }
> -
> -        now = &cache->hash_table[CACHE_HASH_KEY(tail->id)];
> -        for (;;) {
> -            spice_assert(*now);
> -            if (*now == tail) {
> -                *now = tail->next;
> -                break;
> -            }
> -            now = &(*now)->next;
> -        }
> -        ring_remove(&tail->lru_link);
> -        cache->items--;
> -        cache->available += tail->size;
> -        cache->sync[dcc->common.id] = serial;
> -        display_channel_push_release(dcc, SPICE_RES_TYPE_PIXMAP, tail->id,
> tail->sync);
> -        free(tail);
> -    }
> -    ++cache->items;
> -    item->next = cache->hash_table[(key = CACHE_HASH_KEY(id))];
> -    cache->hash_table[key] = item;
> -    ring_item_init(&item->lru_link);
> -    ring_add(&cache->lru, &item->lru_link);
> -    item->id = id;
> -    item->size = size;
> -    item->lossy = lossy;
> -    memset(item->sync, 0, sizeof(item->sync));
> -    item->sync[dcc->common.id] = serial;
> -    cache->sync[dcc->common.id] = serial;
> -    return TRUE;
> -}
> -
> -static void PRIVATE_FUNC_NAME(clear)(CACHE *cache)
> -{
> -    NewCacheItem *item;
> -
> -    if (cache->freezed) {
> -        cache->lru.next = cache->freezed_head;
> -        cache->lru.prev = cache->freezed_tail;
> -        cache->freezed = FALSE;
> -    }
> -
> -    while ((item = (NewCacheItem *)ring_get_head(&cache->lru))) {
> -        ring_remove(&item->lru_link);
> -        free(item);
> -    }
> -    memset(cache->hash_table, 0, sizeof(*cache->hash_table) *
> CACHE_HASH_SIZE);
> -
> -    cache->available = cache->size;
> -    cache->items = 0;
> -}
> -
> -static void FUNC_NAME(reset)(CACHE *cache, DisplayChannelClient *dcc,
> SpiceMsgWaitForChannels* sync_data)
> -{
> -    uint8_t wait_count;
> -    uint64_t serial;
> -    uint32_t i;
> -
> -    serial = red_channel_client_get_message_serial(&dcc->common.base);
> -    pthread_mutex_lock(&cache->lock);
> -    PRIVATE_FUNC_NAME(clear)(cache);
> -
> -    dcc->CACH_GENERATION = ++cache->generation;
> -    cache->generation_initiator.client = dcc->common.id;
> -    cache->generation_initiator.message = serial;
> -    cache->sync[dcc->common.id] = serial;
> -
> -    wait_count = 0;
> -    for (i = 0; i < MAX_CACHE_CLIENTS; i++) {
> -        if (cache->sync[i] && i != dcc->common.id) {
> -            sync_data->wait_list[wait_count].channel_type =
> SPICE_CHANNEL_DISPLAY;
> -            sync_data->wait_list[wait_count].channel_id = i;
> -            sync_data->wait_list[wait_count++].message_serial =
> cache->sync[i];
> -        }
> -    }
> -    sync_data->wait_count = wait_count;
> -    pthread_mutex_unlock(&cache->lock);
> -}
> -
> -static int FUNC_NAME(freeze)(CACHE *cache)
> -{
> -    pthread_mutex_lock(&cache->lock);
> -
> -    if (cache->freezed) {
> -        pthread_mutex_unlock(&cache->lock);
> -        return FALSE;
> -    }
> -
> -    cache->freezed_head = cache->lru.next;
> -    cache->freezed_tail = cache->lru.prev;
> -    ring_init(&cache->lru);
> -    memset(cache->hash_table, 0, sizeof(*cache->hash_table) *
> CACHE_HASH_SIZE);
> -    cache->available = -1;
> -    cache->freezed = TRUE;
> -
> -    pthread_mutex_unlock(&cache->lock);
> -    return TRUE;
> -}
> -
> -static void FUNC_NAME(destroy)(CACHE *cache)
> -{
> -    spice_assert(cache);
> -
> -    pthread_mutex_lock(&cache->lock);
> -    PRIVATE_FUNC_NAME(clear)(cache);
> -    pthread_mutex_unlock(&cache->lock);
> -}
> -
> -#undef CACHE_NAME
> -#undef CACHE_HASH_KEY
> -#undef CACHE_HASH_SIZE
> -#undef CACHE_INVAL_TYPE
> -#undef CACHE_MAX_CLIENT_SIZE
> -#undef FUNC_NAME
> -#undef VAR_NAME
> -#undef CHANNEL
> -#undef CHANNEL_FROM_RCC
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 742edfc..9a119a3 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -91,6 +91,7 @@
>  #include "red_time.h"
>  #include "spice_bitmap_utils.h"
>  #include "spice_image_cache.h"
> +#include "pixmap_cache.h"
>  
>  //#define COMPRESS_STAT
>  //#define DUMP_BITMAP
> @@ -301,20 +302,8 @@ typedef struct VerbItem {
>      uint16_t verb;
>  } VerbItem;
>  
> -#define MAX_CACHE_CLIENTS 4
>  #define MAX_LZ_ENCODERS MAX_CACHE_CLIENTS
>  
> -typedef struct NewCacheItem NewCacheItem;
> -
> -struct NewCacheItem {
> -    RingItem lru_link;
> -    NewCacheItem *next;
> -    uint64_t id;
> -    uint64_t sync[MAX_CACHE_CLIENTS];
> -    size_t size;
> -    int lossy;
> -};
> -
>  typedef struct CacheItem CacheItem;
>  
>  struct CacheItem {
> @@ -383,11 +372,6 @@ typedef struct LocalCursor {
>  #define WIDE_CLIENT_ACK_WINDOW 40
>  #define NARROW_CLIENT_ACK_WINDOW 20
>  
> -#define BITS_CACHE_HASH_SHIFT 10
> -#define BITS_CACHE_HASH_SIZE (1 << BITS_CACHE_HASH_SHIFT)
> -#define BITS_CACHE_HASH_MASK (BITS_CACHE_HASH_SIZE - 1)
> -#define BITS_CACHE_HASH_KEY(id) ((id) & BITS_CACHE_HASH_MASK)
> -
>  #define CLIENT_CURSOR_CACHE_SIZE 256
>  
>  #define CURSOR_CACHE_HASH_SHIFT 8
> @@ -420,7 +404,6 @@ typedef struct ImageItem {
>  typedef struct Drawable Drawable;
>  
>  typedef struct DisplayChannel DisplayChannel;
> -typedef struct DisplayChannelClient DisplayChannelClient;
>  
>  enum {
>      STREAM_FRAME_NONE,
> @@ -509,35 +492,6 @@ static const int BITMAP_FMP_BYTES_PER_PIXEL[] = {0, 0,
> 0, 0, 0, 1, 2, 3, 4, 4, 1
>      (bitmap_fmt_is_rgb(f)        &&                                     \
>       ((f) != SPICE_BITMAP_FMT_8BIT_A))
>  
> -pthread_mutex_t cache_lock = PTHREAD_MUTEX_INITIALIZER;
> -Ring pixmap_cache_list = {&pixmap_cache_list, &pixmap_cache_list};
> -
> -typedef struct PixmapCache PixmapCache;
> -struct PixmapCache {
> -    RingItem base;
> -    pthread_mutex_t lock;
> -    uint8_t id;
> -    uint32_t refs;
> -    NewCacheItem *hash_table[BITS_CACHE_HASH_SIZE];
> -    Ring lru;
> -    int64_t available;
> -    int64_t size;
> -    int32_t items;
> -
> -    int freezed;
> -    RingItem *freezed_head;
> -    RingItem *freezed_tail;
> -
> -    uint32_t generation;
> -    struct {
> -        uint8_t client;
> -        uint64_t message;
> -    } generation_initiator;
> -    uint64_t sync[MAX_CACHE_CLIENTS]; // here CLIENTS refer to different
> channel
> -                                      // clients of the same client
> -    RedClient *client;
> -};
> -
>  #define NUM_STREAMS 50
>  
>  typedef struct WaitForChannels {
> @@ -674,7 +628,7 @@ typedef struct CommonChannelClient {
>  /* Each drawable can refer to at most 3 images: src, brush and mask */
>  #define MAX_DRAWABLE_PIXMAP_CACHE_ITEMS 3
>  
> -struct DisplayChannelClient {
> +struct _DisplayChannelClient {
>      CommonChannelClient common;
>  
>      int expect_init;
> @@ -1045,7 +999,6 @@ static inline void red_detach_stream(RedWorker *worker,
> Stream *stream, int deta
>  static void red_stop_stream(RedWorker *worker, Stream *stream);
>  static inline void red_stream_maintenance(RedWorker *worker, Drawable
>  *candidate, Drawable *sect);
>  static inline void display_begin_send_message(RedChannelClient *rcc);
> -static void red_release_pixmap_cache(DisplayChannelClient *dcc);
>  static void red_release_glz(DisplayChannelClient *dcc);
>  static void red_freeze_glz(DisplayChannelClient *dcc);
>  static void display_channel_push_release(DisplayChannelClient *dcc, uint8_t
>  type, uint64_t id,
> @@ -1627,10 +1580,6 @@ static void common_release_recv_buf(RedChannelClient
> *rcc, uint16_t type, uint32
>      }
>  }
>  
> -#define CLIENT_PIXMAPS_CACHE
> -#include "red_client_shared_cache.h"
> -#undef CLIENT_PIXMAPS_CACHE
> -
>  #define CLIENT_CURSOR_CACHE
>  #include "red_client_cache.h"
>  #undef CLIENT_CURSOR_CACHE
> @@ -6376,6 +6325,70 @@ static inline int
> red_compress_image(DisplayChannelClient *dcc,
>      }
>  }
>  
> +int dcc_pixmap_cache_unlocked_add(DisplayChannelClient *dcc, uint64_t id,
> uint32_t size, int lossy)
> +{
> +    PixmapCache *cache = dcc->pixmap_cache;
> +    NewCacheItem *item;
> +    uint64_t serial;
> +    int key;
> +
> +    spice_assert(size > 0);
> +
> +    item = spice_new(NewCacheItem, 1);
> +    serial = red_channel_client_get_message_serial(&dcc->common.base);
> +
> +    if (cache->generation != dcc->pixmap_cache_generation) {
> +        if (!dcc->pending_pixmaps_sync) {
> +            red_channel_client_pipe_add_type(
> +                &dcc->common.base, PIPE_ITEM_TYPE_PIXMAP_SYNC);
> +            dcc->pending_pixmaps_sync = TRUE;
> +        }
> +        free(item);
> +        return FALSE;
> +    }
> +
> +    cache->available -= size;
> +    while (cache->available < 0) {
> +        NewCacheItem *tail;
> +        NewCacheItem **now;
> +
> +        if (!(tail = (NewCacheItem *)ring_get_tail(&cache->lru)) ||
> +
> tail->sync[dcc->common.id]
> == serial) {
> +            cache->available += size;
> +            free(item);
> +            return FALSE;
> +        }
> +
> +        now = &cache->hash_table[BITS_CACHE_HASH_KEY(tail->id)];
> +        for (;;) {
> +            spice_assert(*now);
> +            if (*now == tail) {
> +                *now = tail->next;
> +                break;
> +            }
> +            now = &(*now)->next;
> +        }
> +        ring_remove(&tail->lru_link);
> +        cache->items--;
> +        cache->available += tail->size;
> +        cache->sync[dcc->common.id] = serial;
> +        display_channel_push_release(dcc, SPICE_RES_TYPE_PIXMAP, tail->id,
> tail->sync);
> +        free(tail);
> +    }
> +    ++cache->items;
> +    item->next = cache->hash_table[(key = BITS_CACHE_HASH_KEY(id))];
> +    cache->hash_table[key] = item;
> +    ring_item_init(&item->lru_link);
> +    ring_add(&cache->lru, &item->lru_link);
> +    item->id = id;
> +    item->size = size;
> +    item->lossy = lossy;
> +    memset(item->sync, 0, sizeof(item->sync));
> +    item->sync[dcc->common.id] = serial;
> +    cache->sync[dcc->common.id] = serial;
> +    return TRUE;
> +}
> +
>  static inline void red_display_add_image_to_pixmap_cache(RedChannelClient
>  *rcc,
>                                                           SpiceImage *image,
>                                                           SpiceImage
>                                                           *io_image,
>                                                           int is_lossy)
> @@ -6386,9 +6399,9 @@ static inline void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
>      if ((image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
>          spice_assert(image->descriptor.width * image->descriptor.height >
>          0);
>          if (!(io_image->descriptor.flags &
>          SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME)) {
> -            if (pixmap_cache_unlocked_add(dcc->pixmap_cache,
> image->descriptor.id,
> -                                          image->descriptor.width *
> image->descriptor.height, is_lossy,
> -                                          dcc)) {
> +            if (dcc_pixmap_cache_unlocked_add(dcc, image->descriptor.id,
> +                                              image->descriptor.width *
> image->descriptor.height,
> +                                              is_lossy)) {
>                  io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
>                  dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++]
>                  =
>                                                                                 image->descriptor.id;
> @@ -6402,6 +6415,43 @@ static inline void
> red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
>      }
>  }
>  
> +static int dcc_pixmap_cache_unlocked_hit(DisplayChannelClient *dcc, uint64_t
> id, int *lossy)
> +{
> +    PixmapCache *cache = dcc->pixmap_cache;
> +    NewCacheItem *item;
> +    uint64_t serial;
> +
> +    serial = red_channel_client_get_message_serial(&dcc->common.base);
> +    item = cache->hash_table[BITS_CACHE_HASH_KEY(id)];
> +
> +    while (item) {
> +        if (item->id == id) {
> +            ring_remove(&item->lru_link);
> +            ring_add(&cache->lru, &item->lru_link);
> +            spice_assert(dcc->common.id < MAX_CACHE_CLIENTS);
> +            item->sync[dcc->common.id] = serial;
> +            cache->sync[dcc->common.id] = serial;
> +            *lossy = item->lossy;
> +            break;
> +        }
> +        item = item->next;
> +    }
> +
> +    return !!item;
> +}
> +
> +static int dcc_pixmap_cache_hit(DisplayChannelClient *dcc, uint64_t id, int
> *lossy)
> +{
> +    int hit;
> +    PixmapCache *cache = dcc->pixmap_cache;
> +
> +    pthread_mutex_lock(&cache->lock);
> +    hit = dcc_pixmap_cache_unlocked_hit(dcc, id, lossy);
> +    pthread_mutex_unlock(&cache->lock);
> +    return hit;
> +}
> +
> +
>  typedef enum {
>      FILL_BITS_TYPE_INVALID,
>      FILL_BITS_TYPE_CACHE,
> @@ -6437,8 +6487,7 @@ static FillBitsType fill_bits(DisplayChannelClient
> *dcc, SpiceMarshaller *m,
>  
>      if ((simage->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
>          int lossy_cache_item;
> -        if (pixmap_cache_unlocked_hit(dcc->pixmap_cache,
> image.descriptor.id,
> -                                      &lossy_cache_item, dcc)) {
> +        if (dcc_pixmap_cache_unlocked_hit(dcc, image.descriptor.id,
> &lossy_cache_item)) {
>              dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++]
>              =
>                                                                                 image.descriptor.id;
>              if (can_lossy || !lossy_cache_item) {
> @@ -6688,8 +6737,7 @@ static int is_bitmap_lossy(RedChannelClient *rcc,
> SpiceImage *image, SpiceRect *
>          int is_hit_lossy;
>  
>          out_data->id = image->descriptor.id;
> -        if (pixmap_cache_hit(dcc->pixmap_cache, image->descriptor.id,
> -                             &is_hit_lossy, dcc)) {
> +        if (dcc_pixmap_cache_hit(dcc, image->descriptor.id, &is_hit_lossy))
> {
>              out_data->type = BITMAP_DATA_TYPE_CACHE;
>              if (is_hit_lossy) {
>                  return TRUE;
> @@ -8110,8 +8158,7 @@ static inline void
> display_channel_send_free_list(RedChannelClient *rcc)
>           * But all this message pixmaps cache references used its old
>           serial.
>           * we use pixmap_cache_items to collect these pixmaps, and we update
>           their serial
>           * by calling pixmap_cache_hit. */
> -        pixmap_cache_hit(dcc->pixmap_cache,
> dcc->send_data.pixmap_cache_items[i],
> -                         &dummy, dcc);
> +        dcc_pixmap_cache_hit(dcc, dcc->send_data.pixmap_cache_items[i],
> &dummy);
>      }
>  
>      if (free_list->wait.header.wait_count) {
> @@ -8490,6 +8537,34 @@ static void
> display_channel_marshall_pixmap_sync(RedChannelClient *rcc,
>      spice_marshall_msg_wait_for_channels(base_marshaller, &wait);
>  }
>  
> +static void dcc_pixmap_cache_reset(DisplayChannelClient *dcc,
> SpiceMsgWaitForChannels* sync_data)
> +{
> +    PixmapCache *cache = dcc->pixmap_cache;
> +    uint8_t wait_count;
> +    uint64_t serial;
> +    uint32_t i;
> +
> +    serial = red_channel_client_get_message_serial(&dcc->common.base);
> +    pthread_mutex_lock(&cache->lock);
> +    pixmap_cache_clear(cache);
> +
> +    dcc->pixmap_cache_generation = ++cache->generation;
> +    cache->generation_initiator.client = dcc->common.id;
> +    cache->generation_initiator.message = serial;
> +    cache->sync[dcc->common.id] = serial;
> +
> +    wait_count = 0;
> +    for (i = 0; i < MAX_CACHE_CLIENTS; i++) {
> +        if (cache->sync[i] && i != dcc->common.id) {
> +            sync_data->wait_list[wait_count].channel_type =
> SPICE_CHANNEL_DISPLAY;
> +            sync_data->wait_list[wait_count].channel_id = i;
> +            sync_data->wait_list[wait_count++].message_serial =
> cache->sync[i];
> +        }
> +    }
> +    sync_data->wait_count = wait_count;
> +    pthread_mutex_unlock(&cache->lock);
> +}
> +
>  static void display_channel_marshall_reset_cache(RedChannelClient *rcc,
>                                                   SpiceMarshaller
>                                                   *base_marshaller)
>  {
> @@ -8497,7 +8572,7 @@ static void
> display_channel_marshall_reset_cache(RedChannelClient *rcc,
>      SpiceMsgWaitForChannels wait;
>  
>      red_channel_client_init_send_data(rcc,
>      SPICE_MSG_DISPLAY_INVAL_ALL_PIXMAPS, NULL);
> -    pixmap_cache_reset(dcc->pixmap_cache, dcc, &wait);
> +    dcc_pixmap_cache_reset(dcc, &wait);
>  
>      spice_marshall_msg_display_inval_all_pixmaps(base_marshaller,
>                                                   &wait);
> @@ -9113,7 +9188,8 @@ static void
> display_channel_client_on_disconnect(RedChannelClient *rcc)
>  #ifdef COMPRESS_STAT
>      print_compress_stats(display_channel);
>  #endif
> -    red_release_pixmap_cache(dcc);
> +    pixmap_cache_unref(dcc->pixmap_cache);
> +    dcc->pixmap_cache = NULL;
>      red_release_glz(dcc);
>      red_reset_palette_cache(dcc);
>      free(dcc->send_data.stream_outbuf);
> @@ -9681,67 +9757,12 @@ static void red_release_glz(DisplayChannelClient
> *dcc)
>      free(shared_dict);
>  }
>  
> -static PixmapCache *red_create_pixmap_cache(RedClient *client, uint8_t id,
> int64_t size)
> -{
> -    PixmapCache *cache = spice_new0(PixmapCache, 1);
> -    ring_item_init(&cache->base);
> -    pthread_mutex_init(&cache->lock, NULL);
> -    cache->id = id;
> -    cache->refs = 1;
> -    ring_init(&cache->lru);
> -    cache->available = size;
> -    cache->size = size;
> -    cache->client = client;
> -    return cache;
> -}
> -
> -static PixmapCache *red_get_pixmap_cache(RedClient *client, uint8_t id,
> int64_t size)
> -{
> -    PixmapCache *ret = NULL;
> -    RingItem *now;
> -    pthread_mutex_lock(&cache_lock);
> -
> -    now = &pixmap_cache_list;
> -    while ((now = ring_next(&pixmap_cache_list, now))) {
> -        PixmapCache *cache = (PixmapCache *)now;
> -        if ((cache->client == client) && (cache->id == id)) {
> -            ret = cache;
> -            ret->refs++;
> -            break;
> -        }
> -    }
> -    if (!ret) {
> -        ret = red_create_pixmap_cache(client, id, size);
> -        ring_add(&pixmap_cache_list, &ret->base);
> -    }
> -    pthread_mutex_unlock(&cache_lock);
> -    return ret;
> -}
> -
> -static void red_release_pixmap_cache(DisplayChannelClient *dcc)
> -{
> -    PixmapCache *cache;
> -    if (!(cache = dcc->pixmap_cache)) {
> -        return;
> -    }
> -    dcc->pixmap_cache = NULL;
> -    pthread_mutex_lock(&cache_lock);
> -    if (--cache->refs) {
> -        pthread_mutex_unlock(&cache_lock);
> -        return;
> -    }
> -    ring_remove(&cache->base);
> -    pthread_mutex_unlock(&cache_lock);
> -    pixmap_cache_destroy(cache);
> -    free(cache);
> -}
> -
>  static int display_channel_init_cache(DisplayChannelClient *dcc,
>  SpiceMsgcDisplayInit *init_info)
>  {
>      spice_assert(!dcc->pixmap_cache);
> -    return !!(dcc->pixmap_cache =
> red_get_pixmap_cache(dcc->common.base.client,
> -
> init_info->pixmap_cache_id,
> -
> init_info->pixmap_cache_size));
> +    return !!(dcc->pixmap_cache = pixmap_cache_get(dcc->common.base.client,
> +
> init_info->pixmap_cache_id,
> +
> init_info->pixmap_cache_size));
>  }
>  
>  static int display_channel_init_glz_dictionary(DisplayChannelClient *dcc,
> @@ -9875,8 +9896,8 @@ static int
> display_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t s
>       * channel client that froze the cache on the src size receives the
>       migrate
>       * data and unfreezes the cache by setting its size > 0 and by
>       triggering
>       * pixmap_cache_reset */
> -    dcc->pixmap_cache = red_get_pixmap_cache(dcc->common.base.client,
> -                                             migrate_data->pixmap_cache_id,
> -1);
> +    dcc->pixmap_cache = pixmap_cache_get(dcc->common.base.client,
> +                                         migrate_data->pixmap_cache_id, -1);
>      if (!dcc->pixmap_cache) {
>          return FALSE;
>      }

Looks good to me beside one things, some structure was renamed with underscore
prefix while the typedef did not have the prefix, specifically:

struct NewCacheItem -> struct _NewCacheItem
struct PixmapCache -> struct _PixmapCache
struct DisplayChannelClient -> struct _DisplayChannelClient

I don't like this change and also it's against C99 standards (an id starting with
underscore and capital case is a reserved id).

Frediano


More information about the Spice-devel mailing list