The SDL forums have moved to discourse.libsdl.org.
This is just a read-only archive of the previous forums, to keep old links working.

SDL Forum Index
SDL
Simple Directmedia Layer Forums
Avoid unnecessary heap allocation in SDL_CreateThread
Sascha Schwarz


Joined: 28 Apr 2015
Posts: 5
Location: Germany
Post Posted: Wed Sep 16, 2015 11:20 am
Hi all.
I was looking at how SDL_CreateThread() and SDL_RunThread() make use of the thread_args structure passed along.

SDL_CreateThread() waits for SDL_RunThread() to signal its consumption of thread_args and then frees it.
Why not make it a local variable and save one heap allocation per thread creation?

Code:
diff -r e0038858fff3 src/thread/SDL_thread.c
--- a/src/thread/SDL_thread.c   Wed Sep 16 12:23:20 2015 +0200
+++ b/src/thread/SDL_thread.c   Wed Sep 16 13:13:00 2015 +0200
@@ -316,7 +316,7 @@
 #endif
 {
     SDL_Thread *thread;
-    thread_args *args;
+    thread_args args;
     int ret;
 
     /* Allocate memory for the thread info structure */
@@ -329,7 +329,6 @@
     thread->status = -1;
     SDL_AtomicSet(&thread->state, SDL_THREAD_STATE_ALIVE);
 
-    /* Set up the arguments for the thread */
     if (name != NULL) {
         thread->name = SDL_strdup(name);
         if (thread->name == NULL) {
@@ -340,37 +339,27 @@
     }
 
     /* Set up the arguments for the thread */
-    args = (thread_args *) SDL_malloc(sizeof(*args));
-    if (args == NULL) {
-        SDL_OutOfMemory();
+    args.func = fn;
+    args.data = data;
+    args.info = thread;
+    args.wait = SDL_CreateSemaphore(0);
+    if (args.wait == NULL) {
         if (thread->name) {
             SDL_free(thread->name);
         }
         SDL_free(thread);
         return (NULL);
     }
-    args->func = fn;
-    args->data = data;
-    args->info = thread;
-    args->wait = SDL_CreateSemaphore(0);
-    if (args->wait == NULL) {
-        if (thread->name) {
-            SDL_free(thread->name);
-        }
-        SDL_free(thread);
-        SDL_free(args);
-        return (NULL);
-    }
 
     /* Create the thread and go! */
 #ifdef SDL_PASSED_BEGINTHREAD_ENDTHREAD
-    ret = SDL_SYS_CreateThread(thread, args, pfnBeginThread, pfnEndThread);
+    ret = SDL_SYS_CreateThread(thread, &args, pfnBeginThread, pfnEndThread);
 #else
-    ret = SDL_SYS_CreateThread(thread, args);
+    ret = SDL_SYS_CreateThread(thread, &args);
 #endif
     if (ret >= 0) {
         /* Wait for the thread function to use arguments */
-        SDL_SemWait(args->wait);
+        SDL_SemWait(args.wait);
     } else {
         /* Oops, failed.  Gotta free everything */
         if (thread->name) {
@@ -379,8 +368,7 @@
         SDL_free(thread);
         thread = NULL;
     }
-    SDL_DestroySemaphore(args->wait);
-    SDL_free(args);
+    SDL_DestroySemaphore(args.wait);
 
     /* Everything is running now */
     return (thread);
.3lite


Joined: 25 Oct 2013
Posts: 38
Post Posted: Wed Sep 16, 2015 7:06 pm
It's not unnecessary. The stack of a process should be protected and no other process should be able to access stack of that process!
Sascha Schwarz


Joined: 28 Apr 2015
Posts: 5
Location: Germany
Post Posted: Thu Sep 17, 2015 6:23 am
.3lite wrote:
It's not unnecessary. The stack of a process should be protected and no other process should be able to access stack of that process!
You are right.
SDL_CreateThread() on the other hand does not create processes. It creates threads sharing the same process.