mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-08 02:08:22 -10:00
video: the window title crosses threads in an atomic mailbox
The title handoff was the last thing the video thread took display_lock for on its frame path: the main thread assembles the title once per fps interval and every driver's frame() offers to apply it, with the 512-byte string uncopyable in one atomic step. The lock's job was only to prevent a torn copy - so the copy stops being shared at all. Where the atomics have pointer ops the title crosses as an immutable heap copy through a one-slot mailbox: publish is exchange-in, freeing whatever title was never taken, and take is exchange-out. A copy either is not published or is complete, because nothing writes it after publication; both sides are wait-free; a slow reader costs a superseded title, never a leak; and the read-side fast path stays one acquire load. The win is not the lock's cycles - it was uncontended - but the lock-order edge it removes: display_lock also guards the disp flags and the viewport rectangle, and the context drivers already document context_lock + display_lock ordering on the video thread, so the title no longer participates in that graph at all. On the one backend without pointer atomics, and in builds without threads, the old protocol stands unchanged behind the same macros, per retro_atomic.h's documented gate - the mpsc deferral's precedent. samples/gfx/window_title_mailbox reproduces the protocol and holds three claims across 20000 paced publishes: no torn copy (embedded checksums), supersession only forward with the final title always taken, and every copy freed exactly once under ASan. Clean under Clang ThreadSanitizer; a write-after-publish sabotage lights up all three checks. All five atomic backends compile the new video_driver.c, VOLATILE on the lock path; C89 holds; the 74-lane matrix and all four hot-function budgets pass.
This commit is contained in:
1 parent
dc8d39a347
commit
0cb8a462cf
4 files changed
+277
-11
No files matched your search
+70
-8
@@ -2630,6 +2630,16 @@ void video_driver_lock_new(void)
|
||||
video_st->display_lock = slock_new();
|
||||
if (!video_st->context_lock)
|
||||
video_st->context_lock = slock_new();
|
||||
#ifdef VIDEO_TITLE_MAILBOX
|
||||
/* Runs before the video thread exists; drop anything stale from a
|
||||
* previous driver's life and start empty. */
|
||||
{
|
||||
char *stale = (char*)retro_atomic_exchange_ptr(
|
||||
&video_st->window_title_pending, NULL);
|
||||
if (stale)
|
||||
free(stale);
|
||||
}
|
||||
#endif
|
||||
#endif
|
||||
}
|
||||
|
||||
@@ -4331,11 +4341,21 @@ bool video_driver_has_focus(void)
|
||||
return VIDEO_HAS_FOCUS(video_st);
|
||||
}
|
||||
|
||||
/* window_title is written on the main thread and read on the video
|
||||
* thread, so both sides use display_lock around it; window_title_update
|
||||
* says there is a new one. No-ops in a build without threads, where
|
||||
* there is no second reader. */
|
||||
#ifdef HAVE_THREADS
|
||||
/* The window title crosses from the main thread, which builds it, to
|
||||
* the video thread, which applies it. Where the atomics have pointer
|
||||
* ops it crosses as an immutable heap copy through a one-slot atomic
|
||||
* mailbox (see video_driver.h): exchange in, exchange out, no lock
|
||||
* and no possible torn copy - a copy either is not published yet or
|
||||
* is complete, because nothing writes it after publication. The
|
||||
* exchange-in frees whatever title was never taken, so a slow reader
|
||||
* costs a superseded title, never a leak. On the one backend without
|
||||
* pointer ops (and in a build without threads, where there is no
|
||||
* second thread) the old display_lock protocol stands below,
|
||||
* unchanged. */
|
||||
#if defined(HAVE_THREADS) && defined(RETRO_ATOMIC_HAS_PTR)
|
||||
#define VIDEO_TITLE_MAILBOX 1
|
||||
#endif
|
||||
#if defined(HAVE_THREADS) && !defined(VIDEO_TITLE_MAILBOX)
|
||||
#define VIDEO_TITLE_LOCK(st) do { if ((st)->display_lock) slock_lock((st)->display_lock); } while (0)
|
||||
#define VIDEO_TITLE_UNLOCK(st) do { if ((st)->display_lock) slock_unlock((st)->display_lock); } while (0)
|
||||
#else
|
||||
@@ -4343,6 +4363,23 @@ bool video_driver_has_focus(void)
|
||||
#define VIDEO_TITLE_UNLOCK(st) ((void)0)
|
||||
#endif
|
||||
|
||||
#ifdef VIDEO_TITLE_MAILBOX
|
||||
/* Publish window_title (main-thread scratch here) as the pending
|
||||
* title. Failure to allocate skips one title update - the next fps
|
||||
* interval brings another. */
|
||||
static void video_title_publish(video_driver_state_t *video_st)
|
||||
{
|
||||
char *copy = strdup(video_st->window_title);
|
||||
if (copy)
|
||||
{
|
||||
char *stale = (char*)retro_atomic_exchange_ptr(
|
||||
&video_st->window_title_pending, copy);
|
||||
if (stale)
|
||||
free(stale);
|
||||
}
|
||||
}
|
||||
#endif
|
||||
|
||||
/* Hands the pending window title to the caller and clears the pending
|
||||
* bit, so the next caller gets nothing until a new title is built.
|
||||
*
|
||||
@@ -4362,6 +4399,20 @@ size_t video_driver_get_window_title(char *s, size_t len)
|
||||
if (!s)
|
||||
return 0;
|
||||
|
||||
#ifdef VIDEO_TITLE_MAILBOX
|
||||
/* Nearly every frame has no new title */
|
||||
if (!retro_atomic_load_acquire_ptr(&video_st->window_title_pending))
|
||||
return 0;
|
||||
{
|
||||
char *taken = (char*)retro_atomic_exchange_ptr(
|
||||
&video_st->window_title_pending, NULL);
|
||||
if (taken)
|
||||
{
|
||||
n = strlcpy(s, taken, len);
|
||||
free(taken);
|
||||
}
|
||||
}
|
||||
#else
|
||||
/* Nearly every frame has no new title */
|
||||
if (!retro_atomic_load_acquire_int(&video_st->window_title_update))
|
||||
return 0;
|
||||
@@ -4373,6 +4424,7 @@ size_t video_driver_get_window_title(char *s, size_t len)
|
||||
retro_atomic_store_release_int(&video_st->window_title_update, 0);
|
||||
}
|
||||
VIDEO_TITLE_UNLOCK(video_st);
|
||||
#endif
|
||||
|
||||
if (!n)
|
||||
return 0;
|
||||
@@ -6077,9 +6129,11 @@ void video_driver_frame(const void *data, unsigned width,
|
||||
last_fps = TIME_TO_FPS(curr_time, new_time,
|
||||
fps_update_interval);
|
||||
|
||||
/* Built under display_lock, and the pending bit set inside
|
||||
* it, so the video thread's video_driver_get_window_title()
|
||||
* cannot copy a title halfway through being assembled. */
|
||||
/* Under the mailbox, window_title is this thread's scratch
|
||||
* and publishing is the exchange in video_title_publish();
|
||||
* under the lock protocol the assembly runs inside
|
||||
* display_lock so the video thread cannot copy a title
|
||||
* halfway through being assembled. */
|
||||
VIDEO_TITLE_LOCK(video_st);
|
||||
__len = strlcpy(video_st->window_title, video_st->title_buf,
|
||||
sizeof(video_st->window_title));
|
||||
@@ -6097,7 +6151,11 @@ void video_driver_frame(const void *data, unsigned width,
|
||||
sizeof(video_st->window_title) - __len);
|
||||
}
|
||||
|
||||
#ifdef VIDEO_TITLE_MAILBOX
|
||||
video_title_publish(video_st);
|
||||
#else
|
||||
retro_atomic_store_release_int(&video_st->window_title_update, 1);
|
||||
#endif
|
||||
VIDEO_TITLE_UNLOCK(video_st);
|
||||
|
||||
curr_time = new_time;
|
||||
@@ -6112,7 +6170,11 @@ void video_driver_frame(const void *data, unsigned width,
|
||||
video_st->window_title,
|
||||
video_st->title_buf,
|
||||
sizeof(video_st->window_title));
|
||||
#ifdef VIDEO_TITLE_MAILBOX
|
||||
video_title_publish(video_st);
|
||||
#else
|
||||
retro_atomic_store_release_int(&video_st->window_title_update, 1);
|
||||
#endif
|
||||
VIDEO_TITLE_UNLOCK(video_st);
|
||||
|
||||
status_text[0] = '\0';
|
||||
|
||||
+13
-3
@@ -1251,10 +1251,20 @@ typedef struct
|
||||
char cli_shader_path[PATH_MAX_LENGTH];
|
||||
char window_title[512];
|
||||
char window_title_prev[512];
|
||||
/* A new window_title waits for the thread that draws: raised with
|
||||
* the title, under display_lock, and taken with it by
|
||||
* video_driver_get_window_title(), which reads this first so a frame
|
||||
/* A new window title waits for the thread that draws. Where the
|
||||
* atomics have pointer ops (RETRO_ATOMIC_HAS_PTR) it waits as an
|
||||
* immutable heap copy in window_title_pending - a one-slot atomic
|
||||
* mailbox: the main thread exchanges a fresh copy in (freeing any
|
||||
* title never taken), video_driver_get_window_title() exchanges it
|
||||
* out. No tearing is possible and neither side takes a lock;
|
||||
* window_title is then main-thread scratch. On the one backend
|
||||
* without pointer ops the old protocol stands: window_title is
|
||||
* shared, rewritten and copied under display_lock, with
|
||||
* window_title_update as the pending bit read first so a frame
|
||||
* with no new title takes no lock. */
|
||||
#if defined(HAVE_THREADS) && defined(RETRO_ATOMIC_HAS_PTR)
|
||||
retro_atomic_ptr_t window_title_pending;
|
||||
#endif
|
||||
retro_atomic_int_t window_title_update;
|
||||
char gpu_api_version_string[128];
|
||||
char title_buf[64];
|
||||
|
||||
@@ -0,0 +1,35 @@
|
||||
TARGET := window_title_mailbox_test
|
||||
|
||||
LIBRETRO_COMM_DIR := ../../../libretro-common
|
||||
|
||||
# Self-contained: the hazard protocol under test is reproduced in the
|
||||
# harness (see the file header for why), so the only tree dependency
|
||||
# is rthreads.c for portable threads and locks. retro_atomic.h and
|
||||
# retro_miscellaneous.h are header-only.
|
||||
SOURCES := \
|
||||
window_title_mailbox_test.c \
|
||||
$(LIBRETRO_COMM_DIR)/rthreads/rthreads.c
|
||||
|
||||
OBJS := $(SOURCES:.c=.o)
|
||||
|
||||
CFLAGS += -Wall -std=gnu99 -g -O1 \
|
||||
-DHAVE_THREADS -I$(LIBRETRO_COMM_DIR)/include
|
||||
LDFLAGS += -lpthread -lrt
|
||||
|
||||
ifneq ($(SANITIZER),)
|
||||
CFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
|
||||
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
|
||||
endif
|
||||
|
||||
all: $(TARGET)
|
||||
|
||||
$(TARGET): $(OBJS)
|
||||
$(CC) -o $@ $(OBJS) $(LDFLAGS)
|
||||
|
||||
check: all
|
||||
./$(TARGET)
|
||||
|
||||
clean:
|
||||
rm -f $(OBJS) $(TARGET)
|
||||
|
||||
.PHONY: all check clean
|
||||
@@ -0,0 +1,159 @@
|
||||
/* window_title_mailbox_test.c -- the one-slot atomic title mailbox of
|
||||
* gfx/video_driver.c, reproduced and beaten on.
|
||||
*
|
||||
* The protocol: the main thread hands the window title to the video
|
||||
* thread as an immutable heap copy through one retro_atomic_ptr_t
|
||||
* slot. Publish is exchange-in (freeing whatever was never taken),
|
||||
* take is exchange-out. The claims a regression would break:
|
||||
*
|
||||
* 1. No torn copy: every title the consumer takes is internally
|
||||
* consistent (its embedded checksum holds), because a published
|
||||
* copy is never written again.
|
||||
* 2. No leak and no double free: every copy is freed exactly once,
|
||||
* by whichever side superseded or took it (ASan holds this).
|
||||
* 3. Supersession only forward: the sequence numbers the consumer
|
||||
* observes strictly increase, and the last published title is
|
||||
* taken once the producer stops.
|
||||
*
|
||||
* The harness reproduces the protocol rather than including
|
||||
* video_driver.c (which drags the whole frontend in); the shape is a
|
||||
* dozen lines and drift is caught by eye at the publish/take sites.
|
||||
*
|
||||
* Paced with short sleeps so a single-processor machine interleaves
|
||||
* the two threads; the protocol has no spin anywhere. */
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <retro_atomic.h>
|
||||
#include <rthreads/rthreads.h>
|
||||
|
||||
#ifndef RETRO_ATOMIC_HAS_PTR
|
||||
int main(void)
|
||||
{
|
||||
printf("window_title_mailbox: no pointer atomics on this backend; "
|
||||
"the lock protocol stands and this test does not apply\n");
|
||||
return 0;
|
||||
}
|
||||
#else
|
||||
|
||||
#if defined(_WIN32)
|
||||
#include <windows.h>
|
||||
static void sleep_us(unsigned us) { Sleep(us / 1000u + 1u); }
|
||||
#else
|
||||
#include <unistd.h>
|
||||
static void sleep_us(unsigned us) { usleep(us); }
|
||||
#endif
|
||||
|
||||
#define N_TITLES 20000
|
||||
|
||||
static retro_atomic_ptr_t g_pending;
|
||||
static retro_atomic_int_t g_stop;
|
||||
|
||||
static unsigned title_checksum(const char *body)
|
||||
{
|
||||
unsigned h = 2166136261u;
|
||||
for (; *body; body++)
|
||||
h = (h ^ (unsigned char)*body) * 16777619u;
|
||||
return h;
|
||||
}
|
||||
|
||||
static char *title_make(int seq)
|
||||
{
|
||||
char body[64], *s = (char*)malloc(96);
|
||||
if (!s)
|
||||
abort();
|
||||
snprintf(body, sizeof(body), "core 1.0 || seq %d fps 60.0", seq);
|
||||
snprintf(s, 96, "%s#%08x", body, title_checksum(body));
|
||||
return s;
|
||||
}
|
||||
|
||||
/* Returns the sequence number, or -1 on a torn/corrupt title. */
|
||||
static int title_check(const char *s)
|
||||
{
|
||||
char body[64];
|
||||
unsigned want;
|
||||
int seq;
|
||||
const char *hash = strrchr(s, '#');
|
||||
if (!hash || (size_t)(hash - s) >= sizeof(body))
|
||||
return -1;
|
||||
memcpy(body, s, (size_t)(hash - s));
|
||||
body[hash - s] = '\0';
|
||||
if (sscanf(hash + 1, "%x", &want) != 1)
|
||||
return -1;
|
||||
if (title_checksum(body) != want)
|
||||
return -1;
|
||||
if (sscanf(body, "core 1.0 || seq %d", &seq) != 1)
|
||||
return -1;
|
||||
return seq;
|
||||
}
|
||||
|
||||
static void producer(void *arg)
|
||||
{
|
||||
int i;
|
||||
(void)arg;
|
||||
for (i = 0; i < N_TITLES; i++)
|
||||
{
|
||||
char *copy = title_make(i);
|
||||
char *stale = (char*)retro_atomic_exchange_ptr(&g_pending, copy);
|
||||
if (stale)
|
||||
free(stale);
|
||||
if ((i & 63) == 0)
|
||||
sleep_us(100);
|
||||
}
|
||||
retro_atomic_store_release_int(&g_stop, 1);
|
||||
}
|
||||
|
||||
int main(void)
|
||||
{
|
||||
sthread_t *p;
|
||||
int last_seq = -1, taken = 0, torn = 0, order = 0;
|
||||
|
||||
retro_atomic_ptr_init(&g_pending, NULL);
|
||||
retro_atomic_store_relaxed_int(&g_stop, 0);
|
||||
|
||||
p = sthread_create(producer, NULL);
|
||||
|
||||
for (;;)
|
||||
{
|
||||
int stopped = retro_atomic_load_acquire_int(&g_stop);
|
||||
if (retro_atomic_load_acquire_ptr(&g_pending))
|
||||
{
|
||||
char *t = (char*)retro_atomic_exchange_ptr(&g_pending, NULL);
|
||||
if (t)
|
||||
{
|
||||
int seq = title_check(t);
|
||||
if (seq < 0)
|
||||
torn++;
|
||||
else if (seq <= last_seq)
|
||||
order++;
|
||||
else
|
||||
last_seq = seq;
|
||||
free(t);
|
||||
taken++;
|
||||
}
|
||||
}
|
||||
else if (stopped)
|
||||
break;
|
||||
sleep_us(50);
|
||||
}
|
||||
sthread_join(p);
|
||||
|
||||
printf("window_title_mailbox: %d published, %d taken, last seq %d\n",
|
||||
N_TITLES, taken, last_seq);
|
||||
if (torn)
|
||||
printf(" FAIL: %d torn/corrupt title(s) taken\n", torn);
|
||||
if (order)
|
||||
printf(" FAIL: %d title(s) out of order\n", order);
|
||||
if (last_seq != N_TITLES - 1)
|
||||
printf(" FAIL: final title lost (last seq %d, want %d)\n",
|
||||
last_seq, N_TITLES - 1);
|
||||
if (torn || order || last_seq != N_TITLES - 1)
|
||||
{
|
||||
printf("window_title_mailbox: FAILED\n");
|
||||
return 1;
|
||||
}
|
||||
printf("window_title_mailbox: ok\n");
|
||||
return 0;
|
||||
}
|
||||
#endif /* RETRO_ATOMIC_HAS_PTR */
|
||||
Reference in new issue
Block a user