shm: remove deferred buffers from main list immediately

When unref:ing a "busy" buffer, destruction is (still) deferred to the
buffer release event.

However, we now move the buffer off the buffer list immediately, and
instead push it to a 'deferred' list. This prevents buffer re-use of
buffers scheduled for destruction.

It also means less buffers to iterate through when trying to find a
re-usable buffer in shm_get_buffer(), since we no longer have to wade
through a potentially long list of to-be-deleted buffers.
This commit is contained in:
Daniel Eklöf 2021-07-16 16:48:18 +02:00
parent 232fb20269
commit 4efb34927e
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F

143
shm.c
View file

@ -82,7 +82,8 @@ struct buffer_private {
bool scrollable; bool scrollable;
}; };
static tll(struct buffer_private) buffers; static tll(struct buffer_private *) buffers;
static tll(struct buffer_private *) deferred;
#undef MEASURE_SHM_ALLOCS #undef MEASURE_SHM_ALLOCS
#if defined(MEASURE_SHM_ALLOCS) #if defined(MEASURE_SHM_ALLOCS)
@ -146,18 +147,22 @@ buffer_destroy(struct buffer_private *buf)
free(buf->public.scroll_damage); free(buf->public.scroll_damage);
pixman_region32_fini(&buf->public.dirty); pixman_region32_fini(&buf->public.dirty);
free(buf);
} }
static bool static bool
buffer_unref_no_remove_from_cache(struct buffer_private *buf) buffer_unref_no_remove_from_cache(struct buffer_private *buf)
{ {
if (buf->ref_count > 0) xassert(buf->ref_count > 0);
buf->ref_count--; buf->ref_count--;
if (buf->ref_count > 0 || buf->busy) if (buf->ref_count > 0)
return false; return false;
buffer_destroy(buf); if (buf->busy)
tll_push_back(deferred, buf);
else
buffer_destroy(buf);
return true; return true;
} }
@ -168,15 +173,23 @@ shm_fini(void)
size_t non_busy_count UNUSED = 0; size_t non_busy_count UNUSED = 0;
tll_foreach(buffers, it) { tll_foreach(buffers, it) {
if (it->item.busy) if (it->item->busy)
busy_count++; busy_count++;
else else
non_busy_count++; non_busy_count++;
buffer_destroy(&it->item); buffer_destroy(it->item);
tll_remove(buffers, it); tll_remove(buffers, it);
} }
xassert(busy_count == 0);
busy_count = tll_length(deferred);
tll_foreach(deferred, it) {
buffer_destroy(it->item);
tll_remove(deferred, it);
}
LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count); LOG_DBG("buffers left: busy=%zu, non-busy=%zu", busy_count, non_busy_count);
if (non_busy_count > 0) { if (non_busy_count > 0) {
@ -201,8 +214,21 @@ buffer_release(void *data, struct wl_buffer *wl_buffer)
xassert(buffer->busy); xassert(buffer->busy);
buffer->busy = false; buffer->busy = false;
if (buffer->ref_count == 0) if (buffer->ref_count == 0) {
shm_unref(&buffer->public); LOG_WARN("deferred delete");
bool found = false;
tll_foreach(deferred, it) {
if (it->item == buffer) {
found = true;
buffer_destroy(buffer);
tll_remove(deferred, it);
break;
}
}
xassert(found);
}
} }
static const struct wl_buffer_listener buffer_listener = { static const struct wl_buffer_listener buffer_listener = {
@ -425,28 +451,33 @@ get_new_buffers(struct wl_shm *shm, size_t count,
for (size_t i = 0; i < count; i++) { for (size_t i = 0; i < count; i++) {
/* Push to list of available buffers, but marked as 'busy' */ /* Push to list of available buffers, but marked as 'busy' */
tll_push_front( struct buffer_private *buf = xmalloc(sizeof(*buf));
buffers, *buf = (struct buffer_private){
((struct buffer_private){ .public = {
.public = { .width = info[i].width,
.width = info[i].width, .height = info[i].height,
.height = info[i].height, .stride = stride[i],
.stride = stride[i], .pix_instances = pix_instances,
.pix_instances = pix_instances, .age = 1234, /* Force a full repaint */
.age = 1234, /* Force a full repaint */ },
}, .cookie = info[i].cookie,
.cookie = info[i].cookie, .ref_count = immediate_purge ? 0 : 1,
.ref_count = immediate_purge ? 0 : 1, .size = sizes[i],
.size = sizes[i], .busy = true,
.busy = true, .pool = pool,
.pool = pool, .scrollable = scrollable,
.scrollable = scrollable, .offset = 0,
.offset = 0, };
}));
struct buffer_private *buf = &tll_front(buffers); if (!instantiate_offset(shm, buf, offset)) {
if (!instantiate_offset(shm, buf, offset)) free(buf);
goto err; goto err;
}
if (immediate_purge)
tll_push_front(deferred, buf);
else
tll_push_front(buffers, buf);
pixman_region32_init(&buf->public.dirty); pixman_region32_init(&buf->public.dirty);
pool->ref_count++; pool->ref_count++;
@ -501,40 +532,42 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie,
{ {
struct buffer_private *cached = NULL; struct buffer_private *cached = NULL;
tll_foreach(buffers, it) { tll_foreach(buffers, it) {
if (it->item.public.width != width) struct buffer_private *buf = it->item;
if (buf->public.width != width)
continue; continue;
if (it->item.public.height != height) if (buf->public.height != height)
continue; continue;
if (it->item.cookie != cookie) if (buf->cookie != cookie)
continue; continue;
if (it->item.busy) if (buf->busy)
it->item.public.age++; buf->public.age++;
else else
#if FORCED_DOUBLE_BUFFERING #if FORCED_DOUBLE_BUFFERING
if (it->item.age == 0) if (buf->age == 0)
it->item.age++; buf->age++;
else else
#endif #endif
{ {
if (cached == NULL) { if (cached == NULL) {
LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)", LOG_DBG("cookie=%lx: re-using buffer from cache (buf=%p)",
cookie, (void *)&it->item); cookie, (void *)buf);
it->item.busy = true; buf->busy = true;
pixman_region32_clear(&it->item.public.dirty); pixman_region32_clear(&buf->public.dirty);
free(it->item.public.scroll_damage); free(buf->public.scroll_damage);
it->item.public.scroll_damage = NULL; buf->public.scroll_damage = NULL;
xassert(it->item.public.pix_instances == pix_instances); xassert(buf->public.pix_instances == pix_instances);
cached = &it->item; cached = it->item;
} else { } else {
/* We have multiple buffers eligible for /* We have multiple buffers eligible for
* re-use. Pick the youngest one, and mark the * re-use. Pick the youngest one, and mark the
* other one for purging */ * other one for purging */
if (it->item.public.age < cached->public.age) { if (buf->public.age < cached->public.age) {
shm_unref(&cached->public); shm_unref(&cached->public);
cached = &it->item; cached = buf;
} else { } else {
if (buffer_unref_no_remove_from_cache(&it->item)) if (buffer_unref_no_remove_from_cache(buf))
tll_remove(buffers, it); tll_remove(buffers, it);
} }
} }
@ -546,14 +579,16 @@ shm_get_buffer(struct wl_shm *shm, int width, int height, unsigned long cookie,
/* Mark old buffers associated with this cookie for purging */ /* Mark old buffers associated with this cookie for purging */
tll_foreach(buffers, it) { tll_foreach(buffers, it) {
if (it->item.cookie != cookie) struct buffer_private *buf = it->item;
if (buf->cookie != cookie)
continue; continue;
if (it->item.public.width == width && it->item.public.height == height) if (buf->public.width == width && buf->public.height == height)
continue; continue;
LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)&it->item); LOG_DBG("cookie=%lx: marking buffer %p for purging", cookie, (void *)buf);
if (buffer_unref_no_remove_from_cache(&it->item)) if (buffer_unref_no_remove_from_cache(buf))
tll_remove(buffers, it); tll_remove(buffers, it);
} }
@ -864,10 +899,12 @@ shm_purge(struct wl_shm *shm, unsigned long cookie)
/* Purge old buffers associated with this cookie */ /* Purge old buffers associated with this cookie */
tll_foreach(buffers, it) { tll_foreach(buffers, it) {
if (it->item.cookie != cookie) struct buffer_private *buf = it->item;
if (buf->cookie != cookie)
continue; continue;
if (buffer_unref_no_remove_from_cache(&it->item)) if (buffer_unref_no_remove_from_cache(buf))
tll_remove(buffers, it); tll_remove(buffers, it);
} }
} }
@ -887,7 +924,7 @@ shm_unref(struct buffer *_buf)
struct buffer_private *buf = (struct buffer_private *)_buf; struct buffer_private *buf = (struct buffer_private *)_buf;
tll_foreach(buffers, it) { tll_foreach(buffers, it) {
if (&it->item != buf) if (it->item != buf)
continue; continue;
if (buffer_unref_no_remove_from_cache(buf)) if (buffer_unref_no_remove_from_cache(buf))