[Spice-devel] [PATCH x11spice] Add cache for SHM segments

Brendan Shanks bshanks at codeweavers.com
Thu Jul 11 20:03:05 UTC 2019


Add a cache to allow the reuse of SHM segments.
Shared memory segments are added to the cache instead of being
deallocated, and the cache is searched instead of/before allocating a
new segment.

Both the SHM segments and their attachment with the X server are cached.

The cache currently has a fixed number of 10 entries, this provided a
good cache hit rate while keeping memory usage under control.
In my testing, over 90% of requests for SHM segments were being
satisfied from the cache.

Signed-off-by: Brendan Shanks <bshanks at codeweavers.com>
---
 src/display.c | 172 +++++++++++++++++++++++++++++++++++++++++++-------
 src/display.h |  12 ++++
 2 files changed, 162 insertions(+), 22 deletions(-)

diff --git a/src/display.c b/src/display.c
index 01e0e85..346d310 100644
--- a/src/display.c
+++ b/src/display.c
@@ -213,11 +213,125 @@ static int register_for_events(display_t *d)
     return 0;
 }
 
+static void shm_cache_entry_destroy(display_t *d, shm_cache_entry_t *entry)
+{
+    if (entry->shmid == -1)
+        return;
+
+    xcb_shm_detach(d->c, entry->shmseg);
+    shmdt(entry->shmaddr);
+    shmctl(entry->shmid, IPC_RMID, NULL);
+    entry->shmid = -1;
+}
+
+static int shm_cache_get(display_t *d, int size, shm_image_t *shmi)
+{
+    int i;
+
+    g_mutex_lock(&d->shm_cache_mutex);
+
+    /* First check for a cache entry of the exact size being requested */
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        shm_cache_entry_t *entry = &d->shm_cache[i];
+
+        if (entry->shmid == -1 || size != entry->size)
+            continue;
+
+        shmi->shmid = entry->shmid;
+        shmi->shmseg = entry->shmseg;
+        shmi->shmaddr = entry->shmaddr;
+        shmi->shmsize = entry->size;
+        entry->shmid = -1;
+
+        g_mutex_unlock(&d->shm_cache_mutex);
+        return 1;
+    }
+
+    /* An exact-size entry wasn't found, look for the first entry that's big enough */
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        shm_cache_entry_t *entry = &d->shm_cache[i];
+
+        if (entry->shmid == -1 || size > entry->size)
+            continue;
+
+        shmi->shmid = entry->shmid;
+        shmi->shmseg = entry->shmseg;
+        shmi->shmaddr = entry->shmaddr;
+        shmi->shmsize = entry->size;
+        entry->shmid = -1;
+
+        g_mutex_unlock(&d->shm_cache_mutex);
+        return 1;
+    }
+
+    /* No usable entry found in the cache */
+    g_mutex_unlock(&d->shm_cache_mutex);
+    return 0;
+}
+
+static int shm_cache_add(display_t *d, shm_image_t *shmi)
+{
+    int i;
+
+    g_mutex_lock(&d->shm_cache_mutex);
+
+    /* Search cache and use the first empty entry */
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        shm_cache_entry_t *entry = &d->shm_cache[i];
+
+        if (entry->shmid != -1)
+            continue;
+
+        entry->shmid = shmi->shmid;
+        entry->shmseg = shmi->shmseg;
+        entry->shmaddr = shmi->shmaddr;
+        entry->size = shmi->shmsize;
+
+        g_mutex_unlock(&d->shm_cache_mutex);
+        return 1;
+    }
+
+    /* Cache is full. Search again, but this time evict the first entry smaller than the one being added */
+    /* TODO: a possible optimization: remove the smallest entry in the cache, rather than just the first smaller one */
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        shm_cache_entry_t *entry = &d->shm_cache[i];
+
+        if (entry->shmid != -1 && entry->size < shmi->shmsize) {
+            shm_cache_entry_destroy(d, entry);
+
+            entry->shmid = shmi->shmid;
+            entry->shmseg = shmi->shmseg;
+            entry->shmaddr = shmi->shmaddr;
+            entry->size = shmi->shmsize;
+
+            g_mutex_unlock(&d->shm_cache_mutex);
+            return 1;
+        }
+    }
+
+    /* Cache is full, and contained no entries smaller than the one being added */
+    g_mutex_unlock(&d->shm_cache_mutex);
+    return 0;
+}
+
+static void shm_cache_destroy(display_t *d)
+{
+    int i;
+
+    g_mutex_lock(&d->shm_cache_mutex);
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        shm_cache_entry_t *entry = &d->shm_cache[i];
+
+        shm_cache_entry_destroy(d, entry);
+    }
+    g_mutex_unlock(&d->shm_cache_mutex);
+}
 
 int display_open(display_t *d, session_t *session)
 {
     int scr;
     int rc;
+    int i;
     xcb_damage_query_version_cookie_t dcookie;
     xcb_damage_query_version_reply_t *damage_version;
     xcb_xkb_use_extension_cookie_t use_cookie;
@@ -314,6 +428,11 @@ int display_open(display_t *d, session_t *session)
     if (rc)
         return rc;
 
+    g_mutex_init(&d->shm_cache_mutex);
+    for (i = 0; i < sizeof(d->shm_cache) / sizeof(d->shm_cache[0]); i++) {
+        d->shm_cache[i].shmid = -1;
+    }
+
     rc = display_create_screen_images(d);
 
     g_message("Display %s opened", session->options.display ? session->options.display : "");
@@ -349,25 +468,29 @@ shm_image_t *create_shm_image(display_t *d, int w, int h)
     shmi->bytes_per_line = (bits_per_pixel(d) / 8) * shmi->w;
     imgsize = shmi->bytes_per_line * shmi->h;
 
-    shmi->shmid = shmget(IPC_PRIVATE, imgsize, IPC_CREAT | 0700);
-    if (shmi->shmid != -1)
-        shmi->shmaddr = shmat(shmi->shmid, 0, 0);
-    if (shmi->shmid == -1 || shmi->shmaddr == (void *) -1) {
-        g_warning("Cannot get shared memory of size %d; errno %d", imgsize, errno);
-        free(shmi);
-        return NULL;
-    }
-    /* We tell shmctl to detach now; that prevents us from holding this
-       shared memory segment forever in case of abnormal process exit. */
-    shmctl(shmi->shmid, IPC_RMID, NULL);
-
-    shmi->shmseg = xcb_generate_id(d->c);
-    cookie = xcb_shm_attach_checked(d->c, shmi->shmseg, shmi->shmid, 0);
-    error = xcb_request_check(d->c, cookie);
-    if (error) {
-        g_warning("Could not attach; type %d; code %d; major %d; minor %d\n",
-                error->response_type, error->error_code, error->major_code, error->minor_code);
-        return NULL;
+    if (!shm_cache_get(d, imgsize, shmi)) {
+        /* No usable shared memory segment found in cache, allocate a new one */
+        shmi->shmid = shmget(IPC_PRIVATE, imgsize, IPC_CREAT | 0700);
+        if (shmi->shmid != -1)
+            shmi->shmaddr = shmat(shmi->shmid, 0, 0);
+        if (shmi->shmid == -1 || shmi->shmaddr == (void *) -1) {
+            g_warning("Cannot get shared memory of size %d; errno %d", imgsize, errno);
+            free(shmi);
+            return NULL;
+        }
+        /* We tell shmctl to detach now; that prevents us from holding this
+           shared memory segment forever in case of abnormal process exit. */
+        shmctl(shmi->shmid, IPC_RMID, NULL);
+        shmi->shmsize = imgsize;
+
+        shmi->shmseg = xcb_generate_id(d->c);
+        cookie = xcb_shm_attach_checked(d->c, shmi->shmseg, shmi->shmid, 0);
+        error = xcb_request_check(d->c, cookie);
+        if (error) {
+            g_warning("Could not attach; type %d; code %d; major %d; minor %d\n",
+                    error->response_type, error->error_code, error->major_code, error->minor_code);
+            return NULL;
+        }
     }
 
     return shmi;
@@ -451,9 +574,12 @@ void display_copy_image_into_fullscreen(display_t *d, shm_image_t *shmi, int x,
 
 void destroy_shm_image(display_t *d, shm_image_t *shmi)
 {
-    xcb_shm_detach(d->c, shmi->shmseg);
-    shmdt(shmi->shmaddr);
-    shmctl(shmi->shmid, IPC_RMID, NULL);
+    if (!shm_cache_add(d, shmi)) {
+        /* Could not add to cache, destroy this segment */
+        xcb_shm_detach(d->c, shmi->shmseg);
+        shmdt(shmi->shmaddr);
+        shmctl(shmi->shmid, IPC_RMID, NULL);
+    }
     if (shmi->drawable_ptr)
         free(shmi->drawable_ptr);
     free(shmi);
@@ -503,6 +629,8 @@ void display_stop_event_thread(display_t *d)
 
 void display_close(display_t *d)
 {
+    shm_cache_destroy(d);
+    g_mutex_clear(&d->shm_cache_mutex);
     xcb_damage_destroy(d->c, d->damage);
     display_destroy_screen_images(d);
     xcb_disconnect(d->c);
diff --git a/src/display.h b/src/display.h
index dc4254b..175512e 100644
--- a/src/display.h
+++ b/src/display.h
@@ -21,6 +21,7 @@
 #ifndef DISPLAY_H_
 #define DISPLAY_H_
 
+#include <glib.h>
 #include <xcb/xcb.h>
 #include <xcb/damage.h>
 #include <xcb/shm.h>
@@ -39,8 +40,16 @@ typedef struct {
     xcb_shm_seg_t shmseg;
     void *shmaddr;
     void *drawable_ptr;
+    int shmsize;
 } shm_image_t;
 
+typedef struct {
+    int shmid;
+    int size;
+    xcb_shm_seg_t shmseg;
+    void *shmaddr;
+} shm_cache_entry_t;
+
 typedef struct {
     xcb_connection_t *c;
     xcb_window_t root;
@@ -58,6 +67,9 @@ typedef struct {
     shm_image_t *fullscreen;
     shm_image_t *scanline;
 
+    shm_cache_entry_t shm_cache[10];
+    GMutex shm_cache_mutex;
+
     pthread_t event_thread;
     struct session_struct *session;
 } display_t;
-- 
2.17.1



More information about the Spice-devel mailing list