[vlc-commits] [Git][videolan/vlc][master] 7 commits: win32: thread: replace vista-win7 futex critical section by SRWLOCK

Jean-Baptiste Kempf (@jbk) gitlab at videolan.org
Sat Oct 9 09:23:02 UTC 2021



Jean-Baptiste Kempf pushed to branch master at VideoLAN / VLC


Commits:
cb916334 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: replace vista-win7 futex critical section by SRWLOCK

Slim Reader/Write locks [1] are lighter than critical sections.
They don't require a release and even provide a static initializer.

We only use the exclusive mode which forbids recursive calls within a thread.

Only available since Vista so cannot be backported to 3.0.

[1] https://docs.microsoft.com/en-us/windows/win32/sync/slim-reader-writer--srw--locks

- - - - -
e4e464ce by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: set the WakeOnAddress fallback in vlc_wait_addr_init()

So we need to move vlc_wait_addr_init() down.

- - - - -
90556153 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: use the proper DLL to get the system WaitOnAddress

We won't be limited to 32 futeces on platforms that have the API.

The DLL to use is api-ms-win-core-synch-l1-2-0.dll [1].
It could also be in api-ms-win-core-synch-l1-2-1.dll [2].

Maybe it existed as some point in kernel32.dll but it doesn't on my Win10.
Mingw64 has this comment:
; MSDN says it's in Kernel32.dll but it's not.
; Link with libsynchronization.a instead.
; Commented out for compatibility with older
; versions of Windows.
;WaitOnAddress
;WakeByAddressSingle
;WakeByAddressAll

It seems we can use GetModuleHandle() as the DLL is implied by loading kernel32.
This will avoid calling FreeLibrary from DllMain.

In case it doesn't work we fallback to the local implementation of futex.

[1] https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitonaddress
[2] https://docs.microsoft.com/en-us/uwp/win32-and-com/win32-apis#apis-from-api-ms-win-core-synch-l1-2-0dll

- - - - -
4f9b4852 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: use a SRWLOCK for the setup_lock

No need to initialize it in DLL_PROCESS_ATTACH, nor destroy it.

- - - - -
e3c74a21 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: use a SRWLOCK for the super_mutex

No need to initialize it in DLL_PROCESS_ATTACH, nor destroy it.

- - - - -
7f003b81 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: remove unused super_variable

- - - - -
bc0fd236 by Steve Lhomme at 2021-10-09T09:07:31+00:00
win32: thread: rename super_mutex to super_lock

It's a SRWLOCK used with exclusive access.

No functional changes.

- - - - -


1 changed file:

- src/win32/thread.c


Changes:

=====================================
src/win32/thread.c
=====================================
@@ -46,8 +46,7 @@
 #include <vlc_atomic.h>
 
 /*** Static mutex and condition variable ***/
-static CRITICAL_SECTION super_mutex;
-static CONDITION_VARIABLE super_variable;
+static SRWLOCK super_lock = SRWLOCK_INIT;
 
 #ifndef VLC_WINSTORE_APP
 # define IS_INTERRUPTIBLE (1)
@@ -95,13 +94,13 @@ int vlc_threadvar_create (vlc_threadvar_t *p_tls, void (*destr) (void *))
     var->next = NULL;
     *p_tls = var;
 
-    EnterCriticalSection(&super_mutex);
+    AcquireSRWLockExclusive(&super_lock);
     var->prev = vlc_threadvar_last;
     if (var->prev)
         var->prev->next = var;
 
     vlc_threadvar_last = var;
-    LeaveCriticalSection(&super_mutex);
+    ReleaseSRWLockExclusive(&super_lock);
     return 0;
 }
 
@@ -109,7 +108,7 @@ void vlc_threadvar_delete (vlc_threadvar_t *p_tls)
 {
     struct vlc_threadvar *var = *p_tls;
 
-    EnterCriticalSection(&super_mutex);
+    AcquireSRWLockExclusive(&super_lock);
     if (var->prev != NULL)
         var->prev->next = var->next;
 
@@ -118,7 +117,7 @@ void vlc_threadvar_delete (vlc_threadvar_t *p_tls)
     else
         vlc_threadvar_last = var->prev;
 
-    LeaveCriticalSection(&super_mutex);
+    ReleaseSRWLockExclusive(&super_lock);
 
     TlsFree (var->id);
     free (var);
@@ -149,19 +148,19 @@ static void vlc_threadvars_cleanup(void)
     vlc_threadvar_t key;
 retry:
     /* TODO: use RW lock or something similar */
-    EnterCriticalSection(&super_mutex);
+    AcquireSRWLockExclusive(&super_lock);
     for (key = vlc_threadvar_last; key != NULL; key = key->prev)
     {
         void *value = vlc_threadvar_get(key);
         if (value != NULL && key->destroy != NULL)
         {
-            LeaveCriticalSection(&super_mutex);
+            ReleaseSRWLockExclusive(&super_lock);
             vlc_threadvar_set(key, NULL);
             key->destroy(value);
             goto retry;
         }
     }
-    LeaveCriticalSection(&super_mutex);
+    ReleaseSRWLockExclusive(&super_lock);
 }
 
 /*** Futeces^WAddress waits ***/
@@ -175,7 +174,7 @@ static VOID (WINAPI *WakeByAddressSingle_)(PVOID);
 
 static struct wait_addr_bucket
 {
-    CRITICAL_SECTION lock;
+    SRWLOCK lock;
     CONDITION_VARIABLE wait;
 } wait_addr_buckets[32];
 
@@ -186,35 +185,14 @@ static struct wait_addr_bucket *wait_addr_get_bucket(void volatile *addr)
     return wait_addr_buckets + ((u >> 3) % ARRAY_SIZE(wait_addr_buckets));
 }
 
-static void vlc_wait_addr_init(void)
-{
-    for (size_t i = 0; i < ARRAY_SIZE(wait_addr_buckets); i++)
-    {
-        struct wait_addr_bucket *bucket = wait_addr_buckets + i;
-
-        InitializeCriticalSection(&bucket->lock);
-        InitializeConditionVariable(&bucket->wait);
-    }
-}
-
-static void vlc_wait_addr_deinit(void)
-{
-    for (size_t i = 0; i < ARRAY_SIZE(wait_addr_buckets); i++)
-    {
-        struct wait_addr_bucket *bucket = wait_addr_buckets + i;
-
-        DeleteCriticalSection(&bucket->lock);
-    }
-}
-
 static BOOL WINAPI WaitOnAddressFallback(void volatile *addr, void *value,
                                          SIZE_T size, DWORD ms)
 {
     struct wait_addr_bucket *bucket = wait_addr_get_bucket(addr);
     uint64_t futex, val = 0;
-    BOOL ret = 0;
+    BOOL ret = FALSE;
 
-    EnterCriticalSection(&bucket->lock);
+    AcquireSRWLockExclusive(&bucket->lock);
 
     switch (size)
     {
@@ -243,9 +221,9 @@ static BOOL WINAPI WaitOnAddressFallback(void volatile *addr, void *value,
     }
 
     if (futex == val)
-        ret = SleepConditionVariableCS(&bucket->wait, &bucket->lock, ms);
+        ret = SleepConditionVariableSRW(&bucket->wait, &bucket->lock, ms, 0);
 
-    LeaveCriticalSection(&bucket->lock);
+    ReleaseSRWLockExclusive(&bucket->lock);
     return ret;
 }
 
@@ -255,14 +233,14 @@ static void WINAPI WakeByAddressFallback(void *addr)
 
     /* Acquire the bucket critical section (only) to enforce proper sequencing.
      * The critical section does not protect any actual memory object. */
-    EnterCriticalSection(&bucket->lock);
+    AcquireSRWLockExclusive(&bucket->lock);
     /* No other threads can hold the lock for this bucket while it is held
      * here. Thus any other thread either:
      * - is already sleeping in SleepConditionVariableCS(), and to be woken up
      *   by the following WakeAllConditionVariable(), or
      * - has yet to retrieve the value at the wait address (with the
      *   'switch (size)' block). */
-    LeaveCriticalSection(&bucket->lock);
+    ReleaseSRWLockExclusive(&bucket->lock);
     /* At this point, other threads can retrieve the value at the wait address.
      * But the value will have already been changed by our call site, thus
      * (futex == val) will be false, and the threads will not go to sleep. */
@@ -271,6 +249,20 @@ static void WINAPI WakeByAddressFallback(void *addr)
      * one wait address per bucket, all threads must be woken up :-/ */
     WakeAllConditionVariable(&bucket->wait);
 }
+
+static void vlc_wait_addr_init(void)
+{
+    for (size_t i = 0; i < ARRAY_SIZE(wait_addr_buckets); i++)
+    {
+        struct wait_addr_bucket *bucket = wait_addr_buckets + i;
+
+        InitializeSRWLock(&bucket->lock);
+        InitializeConditionVariable(&bucket->wait);
+    }
+    WaitOnAddress_ = WaitOnAddressFallback;
+    WakeByAddressAll_ = WakeByAddressFallback;
+    WakeByAddressSingle_ = WakeByAddressFallback;
+}
 #endif
 
 void vlc_atomic_wait(void *addr, unsigned val)
@@ -740,14 +732,14 @@ unsigned vlc_GetCPUCount (void)
 
 
 /*** Initialization ***/
-static CRITICAL_SECTION setup_lock; /* FIXME: use INIT_ONCE */
+static SRWLOCK setup_lock = SRWLOCK_INIT; /* FIXME: use INIT_ONCE */
 
 void vlc_threads_setup(libvlc_int_t *vlc)
 {
-    EnterCriticalSection(&setup_lock);
+    AcquireSRWLockExclusive(&setup_lock);
     if (mdate_selected != mdate_default)
     {
-        LeaveCriticalSection(&setup_lock);
+        ReleaseSRWLockExclusive(&setup_lock);
         return;
     }
 
@@ -769,7 +761,7 @@ void vlc_threads_setup(libvlc_int_t *vlc)
             msg_Dbg(vlc, "could not raise process priority");
     }
 #endif
-    LeaveCriticalSection(&setup_lock);
+    ReleaseSRWLockExclusive(&setup_lock);
 }
 
 #define LOOKUP(s) (((s##_) = (void *)GetProcAddress(h, #s)) != NULL)
@@ -784,36 +776,21 @@ BOOL WINAPI DllMain (HANDLE hinstDll, DWORD fdwReason, LPVOID lpvReserved)
         case DLL_PROCESS_ATTACH:
         {
 #if (_WIN32_WINNT < _WIN32_WINNT_WIN8)
-            HMODULE h = GetModuleHandle(TEXT("kernel32.dll"));
-            if (unlikely(h == NULL))
-                return FALSE;
-
-            if (!LOOKUP(WaitOnAddress)
+            HMODULE h = GetModuleHandle(TEXT("api-ms-win-core-synch-l1-2-0.dll"));
+            if (h == NULL || !LOOKUP(WaitOnAddress)
              || !LOOKUP(WakeByAddressAll) || !LOOKUP(WakeByAddressSingle))
             {
                 vlc_wait_addr_init();
-                WaitOnAddress_ = WaitOnAddressFallback;
-                WakeByAddressAll_ = WakeByAddressFallback;
-                WakeByAddressSingle_ = WakeByAddressFallback;
             }
 #endif
             thread_key = TlsAlloc();
             if (unlikely(thread_key == TLS_OUT_OF_INDEXES))
                 return FALSE;
-            InitializeCriticalSection(&setup_lock);
-            InitializeCriticalSection(&super_mutex);
-            InitializeConditionVariable(&super_variable);
             break;
         }
 
         case DLL_PROCESS_DETACH:
-            DeleteCriticalSection(&super_mutex);
-            DeleteCriticalSection(&setup_lock);
             TlsFree(thread_key);
-#if (_WIN32_WINNT < _WIN32_WINNT_WIN8)
-            if (WaitOnAddress_ == WaitOnAddressFallback)
-                vlc_wait_addr_deinit();
-#endif
             break;
 
         case DLL_THREAD_DETACH:



View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/988aa5b02183f1c8c7431d1eadba6ceee263c0be...bc0fd2364dff3bb4c8edd92d3bc48f4a2126cdbf

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/compare/988aa5b02183f1c8c7431d1eadba6ceee263c0be...bc0fd2364dff3bb4c8edd92d3bc48f4a2126cdbf
You're receiving this email because of your account on code.videolan.org.




More information about the vlc-commits mailing list