Skip to content

Commit ae9de49

Browse files
PhoenixWhitefirePhoenixWhitefire
committed
Fixes some race conditions, Minor refactoring, Adds ThreadSanitizer CI builds
1 parent 3b51dd8 commit ae9de49

11 files changed

Lines changed: 87 additions & 59 deletions

File tree

‎.github/workflows/cmake-multi-platform.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ jobs:
2929

3030
matrix:
3131
os: [windows-latest, ubuntu-latest]
32-
build_type: [Debug, Release]
32+
build_type: [Release, Debug, ReleaseTSan, DebugTSan]
3333
c_compiler: [cl, gcc]
3434
include:
3535
- os: windows-latest
@@ -43,6 +43,10 @@ jobs:
4343
c_compiler: gcc
4444
- os: windows-latest
4545
c_compiler: clang
46+
- os: windows-latest
47+
build_type: ReleaseTSan
48+
- os: windows-latest
49+
build_type: DebugTSan
4650
- os: ubuntu-latest
4751
c_compiler: clang
4852
- os: ubuntu-latest

‎CMakeLists.txt‎

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ set_property(GLOBAL PROPERTY USE_FOLDERS ON)
1515
# Release: Optimizations, LTCG, no Incremental linking, ASan disabled, Tracy in ON_DEMAND mode
1616
# but for distribution binaries to be `Release` for best performance (because Tracy introduces it's own overhead)
1717
# 19/01/2025
18-
set(CMAKE_CONFIGURATION_TYPES "Debug;Release")
18+
set(CMAKE_CONFIGURATION_TYPES "Debug;Release;DebugTSan;ReleaseTSan")
1919

2020
project(PhoenixEngine LANGUAGES C CXX)
2121

@@ -57,7 +57,7 @@ target_include_directories(PhoenixEngine PUBLIC
5757
"Vendor/tinyfd"
5858
)
5959

60-
set(PHX_BUILD_TYPE $<IF:$<CONFIG:Debug>,Debug,$<IF:$<CONFIG:Release>,Release,RelTracy>>)
60+
set(PHX_BUILD_TYPE $<IF:$<CONFIG:Debug>,Debug,$<IF:$<CONFIG:Release>,Release,$<IF:$<CONFIG:DebugTSan>,DebugTSan,ReleaseTSan>>>)
6161

6262
target_compile_definitions(PhoenixEngine PRIVATE PHX_BUILD_TYPE="${PHX_BUILD_TYPE}")
6363

@@ -67,6 +67,7 @@ endif()
6767

6868
target_link_libraries(PhoenixEngine PRIVATE
6969
Vendor
70+
OpenGL
7071
glm
7172
miniaudio
7273
$<IF:$<BOOL:${PHX_HEADLESS}>,,glfw>
@@ -84,6 +85,18 @@ add_compile_definitions(TRACY_ENABLE TRACY_FIBERS)
8485
add_compile_definitions($<$<CONFIG:Release>:NDEBUG>)
8586
add_compile_definitions($<$<CONFIG:Release>:TRACY_ON_DEMAND>)
8687

88+
# remove unnecessary projects
89+
option(LUAU_BUILD_CLI "Build CLI" OFF)
90+
option(LUAU_BUILD_TESTS "Build tests" OFF)
91+
option(MINIAUDIO_NO_EXTRA_NODES "Do not build extra node graph nodes" ON)
92+
93+
add_subdirectory(Vendor)
94+
add_subdirectory(Vendor/luau)
95+
add_subdirectory(Vendor/glm)
96+
add_subdirectory(Vendor/tracy)
97+
add_subdirectory(Vendor/miniaudio)
98+
add_subdirectory(Vendor/glfw)
99+
87100
if (MSVC)
88101
include(cmake/cmake-msvc.txt)
89102
target_compile_definitions(PhoenixEngine PRIVATE PHX_TARGET_COMPILER="MSVC")
@@ -101,18 +114,6 @@ else()
101114
message(WARNING "Unsupported compiler - expected either MSVC or GNU (G++)")
102115
endif()
103116

104-
# remove unnecessary projects
105-
option(LUAU_BUILD_CLI "Build CLI" OFF)
106-
option(LUAU_BUILD_TESTS "Build tests" OFF)
107-
option(MINIAUDIO_NO_EXTRA_NODES "Do not build extra node graph nodes" ON)
108-
109-
add_subdirectory(Vendor)
110-
add_subdirectory(Vendor/luau)
111-
add_subdirectory(Vendor/glm)
112-
add_subdirectory(Vendor/tracy)
113-
add_subdirectory(Vendor/miniaudio)
114-
add_subdirectory(Vendor/glfw)
115-
116117
# organize VS folders
117118
set_property(TARGET Vendor PROPERTY FOLDER "Vendor")
118119
set_property(TARGET TracyClient PROPERTY FOLDER "Vendor")
@@ -137,6 +138,7 @@ set_property(TARGET update_mappings PROPERTY FOLDER "Vendor/GLFW3")
137138
# build engine after all libs
138139
add_dependencies(PhoenixEngine
139140
Vendor
141+
OpenGL
140142
TracyClient
141143
glm
142144
Luau.Compiler

‎Vendor/CMakeLists.txt‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,6 @@ set(VEND_SRCS
1111
"stb/stb_image.h"
1212
"stb.cpp"
1313

14-
"glad/gl.h"
15-
"glad/gl.c"
16-
"glad/KHR/khrplatform.h"
17-
1814
"imgui/imgui.cpp"
1915
"imgui/imgui.h"
2016
"imgui/imgui_internal.h"
@@ -46,3 +42,6 @@ target_include_directories(Vendor PUBLIC
4642
"glfw/include"
4743
"tinyfd"
4844
)
45+
46+
add_subdirectory(glad)
47+
add_dependencies(Vendor OpenGL)

‎cmake/cmake-gnug.txt‎

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
set(PHX_TRACY_LIB_PATH Vendor/tracy/${PHX_BUILD_TYPE}/TracyClient)
44

5-
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:RelTracy>:-O2 -g3>)
5+
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:ReleaseTSan>:-O2>)
66
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:Release>:-O2>)
77
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:Debug>:-O0>)
88
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:Debug>:-g3>)
@@ -24,22 +24,45 @@ target_compile_options(PhoenixEngine PRIVATE
2424
-Wno-format-truncation
2525
)
2626

27+
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
28+
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
2729
target_compile_options(PhoenixEngine PRIVATE $<$<CONFIG:Debug>:-fsanitize=address,undefined>)
2830

29-
target_link_options(PhoenixEngine PRIVATE $<$<CONFIG:RelTracy>:-flto -fsanitize=address,undefined>)
31+
target_link_options(PhoenixEngine PRIVATE $<$<CONFIG:ReleaseTSan>:-flto -fsanitize=thread>)
32+
target_link_options(PhoenixEngine PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
3033
target_link_options(PhoenixEngine PRIVATE $<$<CONFIG:Release>:-flto>)
3134
target_link_options(PhoenixEngine PRIVATE $<$<CONFIG:Debug>:-fsanitize=address,undefined>)
3235

3336
target_link_libraries(PhoenixEngine PRIVATE stdc++)
3437

35-
add_compile_options($<$<CONFIG:RelTracy>:-O2>)
36-
add_compile_options($<$<CONFIG:RelTracy>:-g3>)
38+
# exclude GLFW and Glad
39+
target_compile_options(Vendor PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
40+
target_compile_options(glm PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
41+
target_compile_options(TracyClient PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
42+
target_compile_options(Luau.Compiler PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
43+
target_compile_options(Luau.Ast PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
44+
target_compile_options(Luau.VM PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
45+
target_compile_options(Luau.Require PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
46+
target_compile_options(Luau.Config PRIVATE $<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
47+
48+
target_compile_options(Vendor PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
49+
target_compile_options(glm PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
50+
target_compile_options(TracyClient PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
51+
target_compile_options(Luau.Compiler PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
52+
target_compile_options(Luau.Ast PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
53+
target_compile_options(Luau.VM PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
54+
target_compile_options(Luau.Require PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
55+
target_compile_options(Luau.Config PRIVATE $<$<CONFIG:DebugTSan>:-fsanitize=thread>)
56+
57+
add_compile_options($<$<CONFIG:ReleaseTSan>:-O2>)
58+
add_compile_options($<$<CONFIG:DebugTSan>:-g3>)
3759
add_compile_options($<$<CONFIG:Release>:-O2>)
3860
add_compile_options($<$<CONFIG:Debug>:-O0>)
3961
add_compile_options($<$<CONFIG:Debug>:-g3>)
4062

4163
add_compile_options(-MP)
4264

43-
# incremental warning with LTCG
44-
add_link_options($<$<CONFIG:RelTracy>:-flto>)
65+
add_link_options($<$<CONFIG:ReleaseTSan>:-fsanitize=thread>)
66+
add_link_options($<$<CONFIG:ReleaseTSan>:-flto>)
67+
add_link_options($<$<CONFIG:DebugTSan>:-fsanitize=thread>)
4568
add_link_options($<$<CONFIG:Release>:-flto>)

‎phoenix.conf‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,6 @@
1818
"postfx_blurvignette": false,
1919
"postfx_distortion": false,
2020
"postfx_enabled": true,
21-
"postfx_gamma": 2.2,
21+
"postfx_gamma": 1.0,
2222
"render_glerrorsarefatal": false
2323
}

‎src/decl/Memory.hpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <unordered_set>
1010
#include <string>
1111
#include <vector>
12+
#include <atomic>
1213
#include <array>
1314

1415
#define MEMCAT(cat) Memory::Category::cat
@@ -46,8 +47,8 @@ namespace Memory
4647

4748
void FrameFinish();
4849

49-
inline std::array<size_t, static_cast<size_t>(Category::__count)> Counters;
50-
inline std::array<size_t, static_cast<size_t>(Category::__count)> Activity;
50+
inline std::array<std::atomic_size_t, static_cast<size_t>(Category::__count)> Counters;
51+
inline std::array<std::atomic_size_t, static_cast<size_t>(Category::__count)> Activity;
5152

5253
static inline const char* CategoryNames[] = {
5354
"Default",

‎src/decl/ThreadManager.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class ThreadManager
3636

3737
void m_StopThreads();
3838

39-
std::vector<std::thread> m_Workers;
39+
std::vector<std::jthread> m_Workers;
4040

4141
std::queue<Task> m_Tasks;
4242
std::mutex m_TasksMutex;

‎src/impl/Main.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,10 @@ static void drawDeveloperUI(double DeltaTime)
305305
static std::array<double[255], GraphDatapoints> TimersHist;
306306
static std::array<decltype(Memory::Counters), GraphDatapoints> HeapUsageHist;
307307
static std::array<decltype(Memory::Counters), GraphDatapoints> HeapActivityHist;
308+
static size_t GraphPointer = 0;
308309

309310
const double* times = Timing::FinalFrameTimes;
310-
const std::array<size_t, (size_t)Memory::Category::__count>& memcounts = Memory::Counters;
311+
const std::array<std::atomic_size_t, (size_t)Memory::Category::__count>& memcounts = Memory::Counters;
311312

312313
uint8_t numTimers = Timing::StaticMagicTimerThing::s_NumTimers;
313314

@@ -366,16 +367,15 @@ static void drawDeveloperUI(double DeltaTime)
366367

367368
if (!AreGraphsPaused)
368369
{
369-
for (size_t i = 0; i < GraphDatapoints - 1; i++)
370+
memcpy(TimersHist[GraphPointer], times, sizeof(double) * 255);
371+
372+
for (size_t i = 0; i < HeapUsageHist[GraphPointer].size(); i++)
370373
{
371-
memcpy(TimersHist[i], TimersHist[i+1], sizeof(double) * 255);
372-
HeapUsageHist[i] = HeapUsageHist[i+1];
373-
HeapActivityHist[i] = HeapActivityHist[i+1];
374+
HeapUsageHist[GraphPointer][i] = Memory::Counters[i].load();
375+
HeapActivityHist[GraphPointer][i] = Memory::Activity[i].load();
374376
}
375377

376-
memcpy(TimersHist[GraphDatapoints - 1], times, sizeof(double) * 255);
377-
HeapUsageHist[GraphDatapoints - 1] = Memory::Counters;
378-
HeapActivityHist[GraphDatapoints - 1] = Memory::Activity;
378+
GraphPointer = (GraphPointer + 1) % GraphDatapoints;
379379
}
380380

381381
ImGui::SeparatorText("Timers");
@@ -419,7 +419,7 @@ static void drawDeveloperUI(double DeltaTime)
419419
ImGuiTreeNodeFlags_OpenOnArrow | ImGuiTreeNodeFlags_SpanAvailWidth,
420420
"%s: %zi",
421421
Memory::CategoryNames[i],
422-
memcounts[i]
422+
memcounts[i].load()
423423
);
424424

425425
if (open)

‎src/impl/Memory.cpp‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void Memory::FrameFinish()
5858

5959
#else
6060

61-
static std::array<size_t, static_cast<size_t>(Memory::Category::__count)> s_ActivityWip{ 0 };
61+
static std::array<std::atomic_size_t, static_cast<size_t>(Memory::Category::__count)> s_ActivityWip{ 0 };
6262

6363
void* Memory::GetPointerInfo(void* Pointer, uint32_t* Size, uint8_t* Category)
6464
{
@@ -96,8 +96,8 @@ void* Memory::Alloc(uint32_t Size, Memory::Category MemCat)
9696
TracyAlloc(ptr, Size);
9797
#endif
9898

99-
Counters[memIndex] += Size;
100-
s_ActivityWip[memIndex] += Size;
99+
Counters[memIndex].fetch_add(Size, std::memory_order_relaxed);
100+
s_ActivityWip[memIndex].fetch_add(Size, std::memory_order_relaxed);
101101

102102
*(AllocHeader*)ptr = { .Size = Size, .Category = memIndex };
103103

@@ -149,9 +149,10 @@ void* Memory::ReAlloc(void* Pointer, uint32_t Size, Memory::Category MemCat)
149149
else
150150
TracyAlloc(ptr, Size);
151151
#endif
152-
Counters[memIndex] -= prevSize;
153-
Counters[memIndex] += Size;
154-
s_ActivityWip[memIndex] += prevSize + Size;
152+
153+
Counters[memIndex].fetch_sub(prevSize, std::memory_order_relaxed);
154+
Counters[memIndex].fetch_add(Size, std::memory_order_relaxed);
155+
s_ActivityWip[memIndex].fetch_add(prevSize + Size, std::memory_order_relaxed);
155156

156157
*(AllocHeader*)ptr = { .Size = Size, .Category = memIndex };
157158

@@ -182,16 +183,19 @@ void Memory::Free(void* Pointer)
182183
#endif
183184

184185
assert(Counters[memcat] > 0);
185-
Counters[memcat] -= size;
186-
s_ActivityWip[memcat] += size;
186+
Counters[memcat].fetch_sub(size, std::memory_order_relaxed);
187+
Counters[memcat].fetch_add(size, std::memory_order_relaxed);
187188

188189
free(Pointer);
189190
}
190191

191192
void Memory::FrameFinish()
192193
{
193-
Activity = s_ActivityWip;
194-
s_ActivityWip.fill(0);
194+
for (size_t i = 0; i < Activity.size(); i++)
195+
{
196+
Activity[i] = s_ActivityWip[i].load();
197+
s_ActivityWip[i] = 0;
198+
}
195199
}
196200

197201
void* operator new(size_t size)

‎src/impl/ThreadManager.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ void ThreadManager::Initialize(int NumThreadsOverride)
127127
Task task;
128128

129129
{
130-
std::unique_lock<std::mutex> lock{ m_TasksMutex };
130+
std::unique_lock<std::mutex> lock = std::unique_lock<std::mutex>(m_TasksMutex);
131131

132132
m_TasksCv.wait(
133133
lock,
@@ -177,7 +177,7 @@ void ThreadManager::Dispatch(std::function<void()> Task, bool IsCritical)
177177
}
178178

179179
{
180-
std::unique_lock<std::mutex> lock{ m_TasksMutex };
180+
std::unique_lock<std::mutex> lock = std::unique_lock<std::mutex>(m_TasksMutex);
181181
m_Tasks.emplace(std::move(Task), IsCritical);
182182
}
183183

@@ -189,23 +189,19 @@ void ThreadManager::m_StopThreads()
189189
ZoneScoped;
190190

191191
{
192-
std::unique_lock<std::mutex> lock{ m_TasksMutex };
192+
std::unique_lock<std::mutex> lock = std::unique_lock<std::mutex>(m_TasksMutex);
193193
m_Stop = true;
194194
}
195195

196196
m_TasksCv.notify_all();
197-
198-
for (std::thread& worker : m_Workers)
199-
if (worker.joinable())
200-
worker.join();
197+
m_Workers.clear();
201198
}
202199

203200
void ThreadManager::Shutdown()
204201
{
205202
ZoneScoped;
206203

207204
m_StopThreads();
208-
209205
s_Instance = nullptr;
210206
}
211207

0 commit comments

Comments
 (0)