Restricting the TSAN tests to only the UnitTests for now (will add Pe…

…rformanceTest later again)

Turn on compiler optimizations
Implemented @SirLynix's suggestion

.github/workflows/build.yml

@@ -54,12 +54,6 @@ jobs:
     - name: Unit Tests
       working-directory: ${{github.workspace}}/Build/Linux_ReleaseTSAN
       run: ctest --output-on-failure --verbose
-    - name: Test ConvexVsMesh
-      working-directory: ${{github.workspace}}/Build/Linux_ReleaseTSAN
-      run: ./PerformanceTest -q=LinearCast -t=max -s=ConvexVsMesh
-    - name: Test Ragdoll
-      working-directory: ${{github.workspace}}/Build/Linux_ReleaseTSAN
-      run: ./PerformanceTest -q=LinearCast -t=max -s=Ragdoll
 
   linux-clang-so:
     runs-on: ubuntu-latest

Build/CMakeLists.txt

@@ -242,7 +242,7 @@ else()
 	set(CMAKE_CXX_FLAGS_DISTRIBUTION "${CMAKE_CXX_FLAGS_RELEASE}")
 	set(CMAKE_CXX_FLAGS_RELEASEASAN "-fsanitize=address")
 	set(CMAKE_CXX_FLAGS_RELEASEUBSAN "-fsanitize=undefined,implicit-conversion,float-divide-by-zero,local-bounds -fno-sanitize-recover=all")
-	set(CMAKE_CXX_FLAGS_RELEASETSAN "-fsanitize=thread")
+	set(CMAKE_CXX_FLAGS_RELEASETSAN "${CMAKE_CXX_FLAGS_RELEASE} -fsanitize=thread")
 	set(CMAKE_CXX_FLAGS_RELEASECOVERAGE "-O0 -DJPH_NO_FORCE_INLINE -fprofile-instr-generate -fcoverage-mapping")
 endif()
 

Jolt/Core/JobSystemWithBarrier.cpp

@@ -146,8 +146,11 @@ void JobSystemWithBarrier::BarrierImpl::Wait()
 			} while (has_executed);
 		}
 
-		// Wait for another thread to wake us when either there is more work to do or when all jobs have completed
-		int num_to_acquire = max(1, mSemaphore.GetValue()); // When there have been multiple releases, we acquire them all at the same time to avoid needlessly spinning on executing jobs
+		// Wait for another thread to wake us when either there is more work to do or when all jobs have completed.
+		// When there have been multiple releases, we acquire them all at the same time to avoid needlessly spinning on executing jobs.
+		// Note that using GetValue is inherently unsafe since we can read a stale value, but this is not an issue here as this is the only
+		// place where we acquire the semaphore. Other threads only release it, so we can only read a value that is lower or equal to the actual value.
+		int num_to_acquire = max(1, mSemaphore.GetValue());
 		mSemaphore.Acquire(num_to_acquire);
 		mNumToAcquire -= num_to_acquire;
 	}

Jolt/Core/Semaphore.cpp

@@ -51,7 +51,7 @@ void Semaphore::Release(uint inNumber)
 	}
 #else
 	std::lock_guard lock(mLock);
-	mCount += (int)inNumber;
+	mCount.fetch_add(inNumber, std::memory_order_relaxed);
 	if (inNumber > 1)
 		mWaitVariable.notify_all();
 	else
@@ -74,7 +74,7 @@ void Semaphore::Acquire(uint inNumber)
 	}
 #else
 	std::unique_lock lock(mLock);
-	mCount -= (int)inNumber;
+	mCount.fetch_sub(inNumber, std::memory_order_relaxed);
 	mWaitVariable.wait(lock, [this]() { return mCount >= 0; });
 #endif
 }

Jolt/Core/Semaphore.h

@@ -4,9 +4,8 @@
 
 #pragma once
 
-#include <Jolt/Core/Atomics.h>
-
 JPH_SUPPRESS_WARNINGS_STD_BEGIN
+#include <atomic>
 #include <mutex>
 #include <condition_variable>
 JPH_SUPPRESS_WARNINGS_STD_END
@@ -34,14 +33,7 @@ class JPH_EXPORT Semaphore
 	void				Acquire(uint inNumber = 1);
 
 	/// Get the current value of the semaphore
-	inline int			GetValue() const
-	{
-#if defined(JPH_TSAN_ENABLED) && !defined(JPH_PLATFORM_WINDOWS)
-		// TSAN complains that we're reading mCount without locking. We don't care if we read a stale value. Inserting a lock to keep TSAN happy.
-		std::unique_lock<mutex> lock(mLock);
-#endif
-		return mCount;
-	}
+	inline int			GetValue() const								{ return mCount.load(std::memory_order_relaxed); }
 
 private:
 #ifdef JPH_PLATFORM_WINDOWS
@@ -50,9 +42,9 @@ class JPH_EXPORT Semaphore
 	void *				mSemaphore;										///< The semaphore is an expensive construct so we only acquire/release it if we know that we need to wait/have waiting threads
 #else
 	// Other platforms: Emulate a semaphore using a mutex, condition variable and count
-	mutable mutex		mLock;
+	mutex				mLock;
 	condition_variable	mWaitVariable;
-	int					mCount = 0;
+	atomic<int>			mCount { 0 };
 #endif
 };