[PATCH] Some multithreaded improvement to the event queue

[[ Sorry about the duplicate message, but I had forgotten the [PATCH] mark in
the subject and I didn’t want it to get filtered because of it ]]

Hello all.

I did a small improvement run on SDL_events.c to stratch an itch of mine that
results in the following features:

  • SDL_WaitEvent() is now properly blocks the calling thread when there are no
    pending events (and wakes it up immediately when events arrive rather than
    miss them by up to 10ms because of the SDL_Delay)

  • If the event queue gets full, the thread that is adding the events will be
    blocked until room becomes available rather than just drop them on the floor.

Both features are only enabled when SDL_INIT_EVENTTHREAD is given to
SDL_Init() and the event thread was created successfuly. They don’t make
sense if you don’t have a separate event collection thread anyways.

I’ve only tested this under Linux, but I use no machine dependent code so
it /should/ work on any platform where threads work correctly.

The included diff was made against 1.2.9.

– Marc A. Pelletier

— orig/src/events/SDL_events.c 2005-09-13 00:26:16.000000000 -0400
+++ new/src/events/SDL_events.c 2005-09-13 08:34:23.000000000 -0400
@@ -68,6 +68,16 @@
int safe;
} SDL_EventLock;

+static struct {

  • SDL_mutex* lock;
  • SDL_cond* cond;
    +} SDL_Empty_EventCond;+
    +static struct {
  • SDL_mutex* lock;
  • SDL_cond* cond;
    +} SDL_Full_EventCond;

/* Thread functions */
static SDL_Thread SDL_EventThread = NULL; / Thread handle /
static Uint32 event_thread; /
The event thread id */
@@ -156,7 +166,13 @@

if ( (flags&SDL_INIT_EVENTTHREAD) == SDL_INIT_EVENTTHREAD ) {
SDL_EventLock.lock = SDL_CreateMutex();

  • if ( SDL_EventLock.lock == NULL ) {
  • SDL_Empty_EventCond.lock = SDL_CreateMutex();
  • SDL_Empty_EventCond.cond = SDL_CreateCond();
  • SDL_Full_EventCond.lock = SDL_CreateMutex();
  • SDL_Full_EventCond.cond = SDL_CreateCond();
  • if (SDL_EventLock.lock== NULL
  • || SDL_Empty_EventCond.lock==NULL || SDL_Empty_EventCond.cond==NULL
  • || SDL_Full_EventCond.lock==NULL || SDL_Full_EventCond.cond==NULL) {
    return(-1);
    }
    SDL_EventLock.safe = 0;
    @@ -249,31 +265,39 @@
    /* Add an event to the event queue – called with the queue locked */
    static int SDL_AddEvent(SDL_Event *event)
    {
  • int tail, added;
  • int tail;
  • tail = (SDL_EventQ.tail+1)%MAXEVENTS;
  • if ( tail == SDL_EventQ.head ) {
  • /* Overflow, drop event */
  • added = 0;
  • } else {
  • SDL_EventQ.event[SDL_EventQ.tail] = *event;
  • if (event->type == SDL_SYSWMEVENT) {
  • /* Note that it’s possible to lose an event */
  • int next = SDL_EventQ.wmmsg_next;
  • SDL_EventQ.wmmsg[next] = *event->syswm.msg;
  •      SDL_EventQ.event[SDL_EventQ.tail].syswm.msg =
    
  •  &SDL_EventQ.wmmsg[next];
    
  • SDL_EventQ.wmmsg_next = (next+1)%MAXEVENTS;
  • }
  • SDL_EventQ.tail = tail;
  • added = 1;
  • if ( SDL_EventThread )
  • SDL_mutexP(SDL_Full_EventCond.lock);
  • while ( (tail = (SDL_EventQ.tail+1)%MAXEVENTS) == SDL_EventQ.head ) {
  • if ( !SDL_EventThread )
  • return 0;
  • SDL_CondWait(SDL_Full_EventCond.cond, SDL_Full_EventCond.lock);
  • }
  • SDL_EventQ.event[SDL_EventQ.tail] = *event;
  • if (event->type == SDL_SYSWMEVENT) {
  • /* Note that it’s possible to lose an event */
  • int next = SDL_EventQ.wmmsg_next;
  • SDL_EventQ.wmmsg[next] = *event->syswm.msg;
  •     SDL_EventQ.event[SDL_EventQ.tail].syswm.msg =
    
  • &SDL_EventQ.wmmsg[next];
    
  • SDL_EventQ.wmmsg_next = (next+1)%MAXEVENTS;
    }
  • return(added);
  • SDL_EventQ.tail = tail;
  • if ( SDL_EventThread ) {
  • SDL_CondSignal(SDL_Empty_EventCond.cond);
  • SDL_mutexV(SDL_Full_EventCond.lock);
  • }
  • return 1;
    }

/* Cut an event, and return the next valid spot, or the tail /
/
– called with the queue locked /
-static int SDL_CutEvent(int spot)
+static int SDL_CutEvent_internal(int spot)
{
if ( spot == SDL_EventQ.head ) {
SDL_EventQ.head = (SDL_EventQ.head+1)%MAXEVENTS;
@@ -300,6 +324,19 @@
/
NOTREACHED */
}

+static int SDL_CutEvent(int spot)
+{

  • int retval;
  • if ( !SDL_EventThread )
  • return SDL_CutEvent_internal(spot);
  • SDL_mutexP(SDL_Full_EventCond.lock);
  • retval = SDL_CutEvent_internal(spot);
  • SDL_CondSignal(SDL_Full_EventCond.cond);
  • SDL_mutexV(SDL_Full_EventCond.lock);
    +}

/* Lock the event queue, take a peep at it, and unlock it */
int SDL_PeepEvents(SDL_Event *events, int numevents, SDL_eventaction action,
Uint32 mask)
@@ -387,12 +424,24 @@

int SDL_WaitEvent (SDL_Event *event)
{

  • if ( SDL_EventThread )
  • SDL_mutexP(SDL_Empty_EventCond.lock);
    while ( 1 ) {
    SDL_PumpEvents();
    switch(SDL_PeepEvents(event, 1, SDL_GETEVENT, SDL_ALLEVENTS)) {
  •  case -1: return 0;
    
  •  case 1: return 1;
    
  •  case 0: SDL_Delay(10);
    
  •  case -1:
    
  • if ( SDL_EventThread )
  • SDL_mutexV(SDL_Empty_EventCond.lock);
  • return 0;
  •  case 1:
    
  • if ( SDL_EventThread )
  • SDL_mutexV(SDL_Empty_EventCond.lock);
  • return 1;
  •  case 0:
    
  • if ( SDL_EventThread )
  • SDL_CondWait(SDL_Empty_EventCond.cond, SDL_Empty_EventCond.lock);
  • else
  • SDL_Delay(10);
    }
    }
    }

… and, of course, I managed to use the wrong diff, with a minor bug left
in it. :slight_smile:

At the end of SDL_CutEvent() there should be an extra line:

  • SDL_CondSignal(SDL_Full_EventCond.cond);
  • SDL_mutexV(SDL_Full_EventCond.lock);
  • return(retval);
    +}