input: the key lane takes no lock, and keeps the releases that do not fit

Key events reported off the frontend's thread - the window procedure
and X11's event loop, which with threaded video run on the video
thread - wait in a queue for the poll. Its writers took turns through
a flag they spun on: a lock in all but name. And when it was full the
newest events were dropped, releases with presses: a dropped release
left the key held in the core until it was pressed again.

The queue is a bounded one of many writers and one reader, with no
lock: each slot has a sequence number saying whose turn it is. A
writer claims the next slot by moving the tail with a compare-and-
swap, fills it and hands it to the reader through the sequence; the
reader hands it back the same way. Writers never wait on each other -
one that loses a slot takes the next - and the reader never waits on a
writer. Each writer's events stay in the order it sent them. A slot's
sequence is kept less its index, so the zeroed queue the program
starts with is its first state.

When it is full a press is dropped and counted, as before. A release
is not lost: it is noted in a bitmap, and the poll, once the queue is
drained, turns it back into the release of a key it last saw go down.
A full queue cannot leave a key held.

The key events lane grows two cases: a release that does not fit
reaches the core and the key does not stay held; and two threads
sending at once each get every event through, in order, on the core's
thread. Its full-queue case is unchanged: sixty-four through in order,
thirty-six counted.

Run in a tree configured as the frontend-overhead CI job is, where
all lanes pass. The queue on its own was run with four writers of
20,000 events each, optimised and under ThreadSanitizer: every event
through in order, no report. Built with overlays and networking off,
and with threads off.

input/input_driver.c

@@ -12469,86 +12469,147 @@ static const char *accessibility_lut_name(char key)
  * The window's thread hands its keys over once a frame as it is, so
  * the poll that takes them is the first that could have seen them.
  *
- * One reader, the poll, which takes no lock. Writers take turns
- * through a flag; in practice there is one, the video thread. Sixty-
- * four events between two polls is more than a keyboard sends; past
- * that the newest are dropped, counted and said. */
+ * One reader, the poll, and any number of writers - in practice the
+ * video thread - with no lock between them (see input_key_lane below).
+ * Sixty-four events between two polls is more than a keyboard sends;
+ * past that a press is dropped and counted, and a release is kept. */
 static void input_keyboard_event_now(bool down, unsigned code,
       uint32_t character, uint16_t mod, unsigned device);
 
 #ifdef HAVE_THREADS
 #define INPUT_KEY_LANE_SIZE 64 /* a power of two */
+#define INPUT_KEY_WORDS     ((RETROK_LAST + 31) / 32)
 
+/* A bounded queue of many writers and one reader, without a lock: each
+ * slot has a sequence number that says whose turn it is. A writer
+ * claims the next slot by moving the tail with a compare-and-swap,
+ * fills it, and hands it to the reader through its sequence; the
+ * reader hands it back the same way. Writers never wait on each other
+ * - one that loses the race for a slot takes the next - and the reader
+ * never waits on a writer: a slot not yet filled is simply not there
+ * yet. Each writer's events stay in the order it sent them.
+ *
+ * A slot's sequence is kept less its index, so that the zeroed queue
+ * the program starts with is the queue's first state.
+ *
+ * When it is full, a key down is dropped and counted; a key up is not
+ * lost: it is noted in a bitmap the poll turns back into the release,
+ * once the queue is drained, for a key it last saw go down. A full
+ * queue cannot leave a key held. */
 static struct
 {
    struct
    {
+      retro_atomic_int_t seq;    /* less the slot's index */
       uint32_t character;
       unsigned code;
       unsigned device;
       uint16_t mod;
       bool     down;
    } slot[INPUT_KEY_LANE_SIZE];
-   retro_atomic_int_t head;      /* next to take: the poll's */
-   retro_atomic_int_t tail;      /* next to fill: the writers' */
-   retro_atomic_int_t writing;   /* a writer is in */
+   retro_atomic_int_t tail;      /* next to claim: the writers' */
+   int                head;      /* next to take: the poll's alone */
    retro_atomic_int_t dropped;
+   retro_atomic_int_t released[INPUT_KEY_WORDS]; /* ups that did not fit */
    bool warned;
 } input_key_lane;
 
+/* Keys the frontend last saw go down, for the ups above: the main
+ * thread's alone. */
+static uint32_t input_keys_down[INPUT_KEY_WORDS];
+
 static void input_key_lane_push(bool down, unsigned code,
       uint32_t character, uint16_t mod, unsigned device)
 {
-   int head, tail;
+   int pos = retro_atomic_load_relaxed_int(&input_key_lane.tail);
 
-   while (retro_atomic_exchange_int(&input_key_lane.writing, 1))
-      ; /* another writer: there is not one in practice */
-
-   head = retro_atomic_load_acquire_int(&input_key_lane.head);
-   tail = retro_atomic_load_relaxed_int(&input_key_lane.tail);
-   if ((unsigned)(tail - head) >= INPUT_KEY_LANE_SIZE)
-      retro_atomic_fetch_add_int(&input_key_lane.dropped, 1);
-   else
+   for (;;)
    {
-      unsigned i                       = (unsigned)tail
-         & (INPUT_KEY_LANE_SIZE - 1);
-      input_key_lane.slot[i].down      = down;
-      input_key_lane.slot[i].code      = code;
-      input_key_lane.slot[i].character = character;
-      input_key_lane.slot[i].mod       = mod;
-      input_key_lane.slot[i].device    = device;
-      retro_atomic_store_release_int(&input_key_lane.tail, tail + 1);
+      unsigned i = (unsigned)pos & (INPUT_KEY_LANE_SIZE - 1);
+      int seq    = retro_atomic_load_acquire_int(
+            &input_key_lane.slot[i].seq) + (int)i;
+      int dif    = seq - pos;
+
+      if (dif == 0)
+      {
+         if (retro_atomic_cas_int(&input_key_lane.tail, pos, pos + 1))
+         {
+            input_key_lane.slot[i].down      = down;
+            input_key_lane.slot[i].code      = code;
+            input_key_lane.slot[i].character = character;
+            input_key_lane.slot[i].mod       = mod;
+            input_key_lane.slot[i].device    = device;
+            retro_atomic_store_release_int(&input_key_lane.slot[i].seq,
+                  pos + 1 - (int)i);
+            return;
+         }
+         pos = retro_atomic_load_relaxed_int(&input_key_lane.tail);
+      }
+      else if (dif < 0)
+         break;                  /* full */
+      else
+         pos = retro_atomic_load_relaxed_int(&input_key_lane.tail);
    }
 
-   retro_atomic_store_release_int(&input_key_lane.writing, 0);
+   retro_atomic_fetch_add_int(&input_key_lane.dropped, 1);
+   if (!down && code < RETROK_LAST)
+      retro_atomic_fetch_or_int(&input_key_lane.released[code >> 5],
+            (int)(1u << (code & 31)));
 }
 
-/* The poll's: every event that has come since the last one. */
+/* The poll's: every event that has come since the last one, then the
+ * releases that did not fit. */
 static void input_key_lane_take(void)
 {
-   int head = retro_atomic_load_relaxed_int(&input_key_lane.head);
-   int tail = retro_atomic_load_acquire_int(&input_key_lane.tail);
+   unsigned w;
 
-   while (head != tail)
+   for (;;)
    {
-      unsigned i         = (unsigned)head & (INPUT_KEY_LANE_SIZE - 1);
-      bool down          = input_key_lane.slot[i].down;
-      unsigned code      = input_key_lane.slot[i].code;
-      uint32_t character = input_key_lane.slot[i].character;
-      uint16_t mod       = input_key_lane.slot[i].mod;
-      unsigned device    = input_key_lane.slot[i].device;
+      int pos    = input_key_lane.head;
+      unsigned i = (unsigned)pos & (INPUT_KEY_LANE_SIZE - 1);
+      int seq    = retro_atomic_load_acquire_int(
+            &input_key_lane.slot[i].seq) + (int)i;
+      bool down;
+      unsigned code, device;
+      uint32_t character;
+      uint16_t mod;
 
+      if (seq != pos + 1)
+         break;                  /* nothing more, or not filled yet */
+
+      down      = input_key_lane.slot[i].down;
+      code      = input_key_lane.slot[i].code;
+      character = input_key_lane.slot[i].character;
+      mod       = input_key_lane.slot[i].mod;
+      device    = input_key_lane.slot[i].device;
       /* the slot is free before the event is acted on */
-      retro_atomic_store_release_int(&input_key_lane.head, ++head);
+      retro_atomic_store_release_int(&input_key_lane.slot[i].seq,
+            pos + INPUT_KEY_LANE_SIZE - (int)i);
+      input_key_lane.head = pos + 1;
       input_keyboard_event_now(down, code, character, mod, device);
    }
 
+   for (w = 0; w < INPUT_KEY_WORDS; w++)
+   {
+      uint32_t ups;
+      unsigned b;
+      if (!retro_atomic_load_relaxed_int(&input_key_lane.released[w]))
+         continue;
+      ups = (uint32_t)retro_atomic_exchange_int(
+            &input_key_lane.released[w], 0);
+      for (b = 0; b < 32; b++)
+         if ((ups & (1u << b)) && (input_keys_down[w] & (1u << b)))
+            input_keyboard_event_now(false, w * 32 + b, 0, 0,
+                  RETRO_DEVICE_KEYBOARD);
+   }
+
    if (     !input_key_lane.warned
          && retro_atomic_load_relaxed_int(&input_key_lane.dropped))
    {
       input_key_lane.warned = true;
       RARCH_WARN("[Input] More than %d keyboard events came between two"
-            " polls: the newest were dropped.\n", INPUT_KEY_LANE_SIZE);
+            " polls: key presses past that were dropped, releases"
+            " kept.\n", INPUT_KEY_LANE_SIZE);
    }
 }
 
@@ -12571,8 +12632,28 @@ void input_keyboard_event(bool down, unsigned code,
    input_keyboard_event_now(down, code, character, mod, device);
 }
 
+static void input_keyboard_event_now_act(bool down, unsigned code,
+      uint32_t character, uint16_t mod, unsigned device);
+
+/* Every key event is acted on here, on the frontend's thread; the keys
+ * it last saw go down are kept for the key lane's releases. */
 static void input_keyboard_event_now(bool down, unsigned code,
       uint32_t character, uint16_t mod, unsigned device)
+{
+#ifdef HAVE_THREADS
+   if (code < RETROK_LAST)
+   {
+      if (down)
+         input_keys_down[code >> 5] |=  (1u << (code & 31));
+      else
+         input_keys_down[code >> 5] &= ~(1u << (code & 31));
+   }
+#endif
+   input_keyboard_event_now_act(down, code, character, mod, device);
+}
+
+static void input_keyboard_event_now_act(bool down, unsigned code,
+      uint32_t character, uint16_t mod, unsigned device)
 {
    runloop_state_t *runloop_st = runloop_state_get_ptr();
    retro_keyboard_event_t

samples/runloop/frontend_overhead/frontend_overhead_test.c

@@ -3943,6 +3943,55 @@ static void RETRO_CALLCONV keylane_core_cb(bool down, unsigned keycode,
    keylane_got++;
 }
 
+/* Two senders at once, each with its own key: per key, every event
+ * arrives and in the order sent. And a release that does not fit. */
+static unsigned keylane2_got[2], keylane2_bad[2], keylane_f15_down, keylane_f15_up;
+
+static void RETRO_CALLCONV keylane2_core_cb(bool down, unsigned keycode,
+      uint32_t character, uint16_t mods)
+{
+   unsigned k = (keycode == RETROK_F13) ? 0 : (keycode == RETROK_F12) ? 1 : 2;
+   (void)mods;
+   if (keycode == RETROK_F15)
+   {
+      if (down)
+         keylane_f15_down++;
+      else
+         keylane_f15_up++;
+      return;
+   }
+   if (k > 1)
+      return;
+   if (     character != keylane2_got[k]
+         || down != ((keylane2_got[k] & 1) == 0))
+      keylane2_bad[k]++;
+   keylane2_got[k]++;
+}
+
+static void keylane2_sender(void *data)
+{
+   unsigned key = *(unsigned*)data;
+   unsigned i;
+   for (i = 0; i < KEYLANE_EVENTS; i++)
+   {
+      input_keyboard_event((i & 1) == 0, key, i, 0, RETRO_DEVICE_KEYBOARD);
+      if ((i & 7) == 7)
+         retro_sleep(1);
+   }
+}
+
+/* F15 down, sixty-three more to fill the queue, then F15 up */
+static void keylane_fill_sender(void *data)
+{
+   unsigned i;
+   (void)data;
+   input_keyboard_event(true, RETROK_F15, 0, 0, RETRO_DEVICE_KEYBOARD);
+   for (i = 0; i < 63; i++)
+      input_keyboard_event((i & 1) == 0, RETROK_F14, i, 0,
+            RETRO_DEVICE_KEYBOARD);
+   input_keyboard_event(false, RETROK_F15, 0, 0, RETRO_DEVICE_KEYBOARD);
+}
+
 static void keylane_sender(void *data)
 {
    unsigned i, n = *(unsigned*)data;
@@ -4026,13 +4075,60 @@ static void lane_key_events(void)
             "key events: those that did not fit were not counted");
    }
 
+   /* 4. A release that does not fit is not lost: the key does not
+    *    stay held. */
+   runloop_st->key_event = keylane2_core_cb;
+   keylane_f15_down = keylane_f15_up = 0;
+   thr = sthread_create(keylane_fill_sender, NULL);
+   if (thr)
+   {
+      sthread_join(thr);
+      run_frames(2);
+      CHECK(keylane_f15_down == 1 && keylane_f15_up == 1,
+            "key events: a release that did not fit in the queue was lost, and the key stayed held");
+   }
+
+   /* 5. Two senders at once, with no lock between them: per key,
+    *    every event arrives, in order, on the core's thread. */
+   {
+      unsigned key0 = RETROK_F13, key1 = RETROK_F12;
+      sthread_t *t0, *t1;
+      keylane2_got[0] = keylane2_got[1] = keylane2_bad[0] = keylane2_bad[1] = 0;
+      dropped0 = input_driver_key_events_dropped();
+      t0 = sthread_create(keylane2_sender, &key0);
+      t1 = sthread_create(keylane2_sender, &key1);
+      if (t0 && t1)
+      {
+         for (i = 0; i < 800 && (keylane2_got[0] < KEYLANE_EVENTS || keylane2_got[1] < KEYLANE_EVENTS); i++)
+         {
+            run_frames(1);
+            retro_sleep(1);
+         }
+         sthread_join(t0);
+         sthread_join(t1);
+         run_frames(2);
+         if (keylane2_got[0] != KEYLANE_EVENTS || keylane2_got[1] != KEYLANE_EVENTS || keylane2_bad[0] || keylane2_bad[1])
+            fprintf(stderr, "       F13 %u/%u (%u out of order), F12 %u/%u (%u out of order), %u dropped\n",
+                  keylane2_got[0], KEYLANE_EVENTS, keylane2_bad[0],
+                  keylane2_got[1], KEYLANE_EVENTS, keylane2_bad[1],
+                  input_driver_key_events_dropped() - dropped0);
+         CHECK(keylane2_got[0] == KEYLANE_EVENTS && keylane2_got[1] == KEYLANE_EVENTS,
+               "key events: from two threads at once, not all reached the core");
+         CHECK(!keylane2_bad[0] && !keylane2_bad[1],
+               "key events: from two threads at once, one thread's reached the core out of order");
+      }
+      else
+         CHECK(false, "key events: no two threads to report from");
+   }
+
    runloop_st->key_event = saved;
    fast_forward(false);
    run_frames(2);
    if (failures == had)
       printf("[pass] key events: from the frontend's thread acted on at once;"
             " from another, the core gets all of them in order on its own thread"
-            " at the next poll; a full queue drops the newest and counts them\n");
+            " at the next poll; a full queue drops presses, counts them, and"
+            " keeps releases; two threads at once, no lock, each in order\n");
 }
 
 /* ---- a joypad driver restart asked for from another thread --------- */