From 93b4b94f8d4ac1808e715766b180932948e671a6 Mon Sep 17 00:00:00 2001 From: Luis Horna Carranza Date: Fri, 11 Sep 2026 13:48:10 -0500 Subject: [PATCH] Threading: Use lock-free access for m_sliceType Replace the mutex-protected m_sliceType with an atomic to eliminate the 25% encoding speed penalty introduced by cb40d97 on AArch64. --- source/common/threading.h | 9 ++++++--- source/common/threadpool.cpp | 6 +++--- source/common/threadpool.h | 19 ++++++------------- source/common/wavefront.cpp | 4 ++-- source/encoder/frameencoder.cpp | 2 +- 5 files changed, 18 insertions(+), 22 deletions(-) diff --git a/source/common/threading.h b/source/common/threading.h index dd1f0bb9f..0a327c9cd 100644 --- a/source/common/threading.h +++ b/source/common/threading.h @@ -70,6 +70,7 @@ int64_t no_atomic_add64(int64_t* ptr, int64_t val); #define ATOMIC_ADD(ptr, val) (sizeof(*(ptr)) == 8 ? \ no_atomic_add64(const_cast(reinterpret_cast(ptr)), (int64_t)(val)) : \ no_atomic_add(const_cast(reinterpret_cast(ptr)), (int)(val))) +#define ATOMIC_LOAD(ptr) (*(ptr)) #define ATOMIC_STORE(ptr, val) (*(ptr) = (int)(val)) #define ATOMIC64_LOAD(ptr) (*(ptr)) #define ATOMIC64_STORE(ptr, val) (*(ptr) = (val)) @@ -90,8 +91,9 @@ int64_t no_atomic_add64(int64_t* ptr, int64_t val); #define ATOMIC_INC(ptr) __atomic_add_fetch(reinterpret_cast(ptr), 1, __ATOMIC_SEQ_CST) #define ATOMIC_DEC(ptr) __atomic_sub_fetch(reinterpret_cast(ptr), 1, __ATOMIC_SEQ_CST) #define ATOMIC_ADD(ptr, val) __atomic_fetch_add(reinterpret_cast(ptr), (__typeof__(*(ptr) + 0))(val), __ATOMIC_SEQ_CST) +#define ATOMIC_LOAD(ptr) __atomic_load_n(ptr, __ATOMIC_SEQ_CST) #define ATOMIC_STORE(ptr, val) __atomic_exchange_n(reinterpret_cast(ptr), (int32_t)(val), __ATOMIC_SEQ_CST) -#define ATOMIC64_LOAD(ptr) __atomic_fetch_add(reinterpret_cast(ptr), 0, __ATOMIC_SEQ_CST) +#define ATOMIC64_LOAD(ptr) __atomic_load_n(reinterpret_cast(ptr), __ATOMIC_SEQ_CST) #define ATOMIC64_STORE(ptr, val) __atomic_exchange_n(reinterpret_cast(ptr), val, __ATOMIC_SEQ_CST) #define ATOMIC64_ADD(ptr, val) __atomic_fetch_add(reinterpret_cast(ptr), val, __ATOMIC_SEQ_CST) #define GIVE_UP_TIME() usleep(0) @@ -111,6 +113,7 @@ int64_t no_atomic_add64(int64_t* ptr, int64_t val); InterlockedExchangeAdd(reinterpret_cast(ptr), (LONG)(val))) #define ATOMIC_OR(ptr, mask) _InterlockedOr(reinterpret_cast(ptr), static_cast(mask)) #define ATOMIC_AND(ptr, mask) _InterlockedAnd(reinterpret_cast(ptr), static_cast(mask)) +#define ATOMIC_LOAD(ptr) _InterlockedOr(reinterpret_cast(ptr), 0) #define ATOMIC_STORE(ptr, val) InterlockedExchange(reinterpret_cast(ptr), (LONG)(val)) #define ATOMIC64_LOAD(ptr) InterlockedAdd64(reinterpret_cast(ptr), 0) #define ATOMIC64_STORE(ptr, val) InterlockedExchange64(reinterpret_cast(ptr), val) @@ -896,7 +899,7 @@ template struct AtomicOps; template<> struct AtomicOps<4> { typedef int32_t Storage; - static int32_t load(volatile Storage* p) { return (int32_t)ATOMIC_OR(p, 0); } + static int32_t load(volatile Storage* p) { return (int32_t)ATOMIC_LOAD(p); } static void store(volatile Storage* p, int32_t v) { ATOMIC_STORE(p, v); } static int32_t inc(volatile Storage* p) { return (int32_t)ATOMIC_INC(p); } static int32_t dec(volatile Storage* p) { return (int32_t)ATOMIC_DEC(p); } @@ -989,7 +992,7 @@ class AtomicBool operator bool() const { - return (int32_t)ATOMIC_OR(&m_val, 0) != 0; + return (int32_t)ATOMIC_LOAD(&m_val) != 0; } AtomicBool& operator=(bool v) diff --git a/source/common/threadpool.cpp b/source/common/threadpool.cpp index 40a9d1c8e..cba965231 100644 --- a/source/common/threadpool.cpp +++ b/source/common/threadpool.cpp @@ -158,16 +158,16 @@ void WorkerThread::threadMain() /* if the current job provider still wants help, only switch to a * higher priority provider (lower slice type). Else take the first * available job provider with the highest priority */ - int curPriority = (bool)m_curJobProvider->m_helpWanted ? m_curJobProvider->m_sliceType.get() : + int curPriority = (bool)m_curJobProvider->m_helpWanted ? m_curJobProvider->m_sliceType : INVALID_SLICE_PRIORITY + 1; int nextProvider = -1; for (int i = 0; i < m_pool.m_numProviders; i++) { if ((bool)m_pool.m_jpTable[i]->m_helpWanted && - m_pool.m_jpTable[i]->m_sliceType.get() < curPriority) + m_pool.m_jpTable[i]->m_sliceType < curPriority) { nextProvider = i; - curPriority = m_pool.m_jpTable[i]->m_sliceType.get(); + curPriority = m_pool.m_jpTable[i]->m_sliceType; } } if (nextProvider != -1 && m_curJobProvider != m_pool.m_jpTable[nextProvider]) diff --git a/source/common/threadpool.h b/source/common/threadpool.h index ca402a380..adeda81f6 100644 --- a/source/common/threadpool.h +++ b/source/common/threadpool.h @@ -36,16 +36,10 @@ class BondedTaskGroup; #if X86_64 || X265_ARCH_ARM64 typedef uint64_t sleepbitmap_t; -#ifdef __GNUC__ -#define SLEEPBITMAP_LOAD(ptr) __sync_fetch_and_or(ptr, 0) -#elif defined(_MSC_VER) -/* LONG64 vs. uint64_t signedness mismatch is intentional: InterlockedOr64 only needs the bit pattern */ -#define SLEEPBITMAP_LOAD(ptr) InterlockedOr64(reinterpret_cast(ptr), 0) -#endif +#define SLEEPBITMAP_LOAD(ptr) ATOMIC64_LOAD(ptr) #else typedef uint32_t sleepbitmap_t; -/* use 32-bit primitives defined in threading.h */ -#define SLEEPBITMAP_LOAD(ptr) ATOMIC_OR(ptr, 0) +#define SLEEPBITMAP_LOAD(ptr) ATOMIC_LOAD(ptr) #endif static const sleepbitmap_t ALL_POOL_THREADS = (sleepbitmap_t)-1; @@ -61,19 +55,18 @@ class JobProvider ThreadPool* m_pool; sleepbitmap_t m_ownerBitmap; int m_jpId; - ThreadSafeInteger m_sliceType; - AtomicInt32 m_helpWanted; + AtomicInt32 m_sliceType; + AtomicBool m_helpWanted; bool m_isFrameEncoder; /* rather ugly hack, but nothing better presents itself */ JobProvider() : m_pool(NULL) , m_ownerBitmap(0) , m_jpId(-1) + , m_sliceType(INVALID_SLICE_PRIORITY) , m_helpWanted(false) , m_isFrameEncoder(false) - { - m_sliceType.set(INVALID_SLICE_PRIORITY); - } + {} virtual ~JobProvider() {} diff --git a/source/common/wavefront.cpp b/source/common/wavefront.cpp index 2d73ebff2..42ec2ff34 100644 --- a/source/common/wavefront.cpp +++ b/source/common/wavefront.cpp @@ -103,7 +103,7 @@ void WaveFront::findJob(int threadId) /* Loop over each word until all available rows are finished */ for (int w = 0; w < m_numWords; w++) { - uint32_t oldval = ATOMIC_OR(&m_internalDependencyBitmap[w], 0) & ATOMIC_OR(&m_externalDependencyBitmap[w], 0); + uint32_t oldval = ATOMIC_LOAD(&m_internalDependencyBitmap[w]) & ATOMIC_LOAD(&m_externalDependencyBitmap[w]); while (oldval) { BSF(id, oldval); @@ -117,7 +117,7 @@ void WaveFront::findJob(int threadId) return; /* check for a higher priority task */ } - oldval = ATOMIC_OR(&m_internalDependencyBitmap[w], 0) & ATOMIC_OR(&m_externalDependencyBitmap[w], 0); + oldval = ATOMIC_LOAD(&m_internalDependencyBitmap[w]) & ATOMIC_LOAD(&m_externalDependencyBitmap[w]); } } diff --git a/source/encoder/frameencoder.cpp b/source/encoder/frameencoder.cpp index 5b64950b4..a3bf77183 100644 --- a/source/encoder/frameencoder.cpp +++ b/source/encoder/frameencoder.cpp @@ -283,7 +283,7 @@ bool FrameEncoder::startCompressFrame(Frame* curFrame[MAX_LAYERS]) curFrame[layer]->m_encData->m_jobProvider = this; curFrame[layer]->m_encData->m_slice->m_mref = m_mref; } - m_sliceType.set(curFrame[0]->m_lowres.sliceType); + m_sliceType = curFrame[0]->m_lowres.sliceType; if (!m_cuGeoms) {