Skip to content

Commit 45bde0e

Browse files
githubawngithubawn
committed
Replace string locks with atomic swaps
Update ref-counts to atomic operations
1 parent 38d6e0d commit 45bde0e

13 files changed

Lines changed: 70 additions & 68 deletions

File tree

‎Core/GameEngine/Include/Common/AsciiString.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class AsciiString
8787
#if defined(RTS_DEBUG)
8888
const char* m_debugptr; // just makes it easier to read in the debugger
8989
#endif
90-
unsigned short m_refCount; // reference count
90+
volatile long m_refCount; // reference count
9191
unsigned short m_numCharsAllocated; // length of data allocated
9292
// char m_stringdata[];
9393

@@ -101,7 +101,7 @@ class AsciiString
101101
#endif
102102

103103
protected:
104-
AsciiStringData* m_data; // pointer to ref counted string data
104+
AsciiStringData* volatile m_data; // pointer to ref counted string data
105105

106106
char* peek() const;
107107
void releaseBuffer();

‎Core/GameEngine/Include/Common/UnicodeString.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class UnicodeString
8787
#if defined(RTS_DEBUG)
8888
const WideChar* m_debugptr; // just makes it easier to read in the debugger
8989
#endif
90-
unsigned short m_refCount; // reference count
90+
volatile long m_refCount; // reference count
9191
unsigned short m_numCharsAllocated; // length of data allocated
9292
// WideChar m_stringdata[];
9393

@@ -101,7 +101,7 @@ class UnicodeString
101101
#endif
102102

103103
protected:
104-
UnicodeStringData* m_data; // pointer to ref counted string data
104+
UnicodeStringData* volatile m_data; // pointer to ref counted string data
105105

106106
WideChar* peek() const;
107107
void releaseBuffer();

‎Core/GameEngine/Source/Common/System/AsciiString.cpp‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
///////////////////////////////////////////////////////////////////////////////
4545
#include "PreRTS.h" // This must go first in EVERY cpp file in the GameEngine
4646

47-
#include "Common/CriticalSection.h"
48-
4947

5048
// -----------------------------------------------------
5149

@@ -86,9 +84,8 @@ inline char* skipNonWhitespace(char* p)
8684
// -----------------------------------------------------
8785
AsciiString::AsciiString(const AsciiString& stringSrc) : m_data(stringSrc.m_data)
8886
{
89-
ScopedCriticalSection scopedCriticalSection(TheAsciiStringCriticalSection);
9087
if (m_data)
91-
++m_data->m_refCount;
88+
InterlockedIncrement(&m_data->m_refCount);
9289
validate();
9390
}
9491

@@ -171,8 +168,14 @@ void AsciiString::ensureUniqueBufferOfSize(int numCharsNeeded, Bool preserveData
171168
if (strToCat)
172169
strcat(newData->peek(), strToCat);
173170

174-
releaseBuffer();
175-
m_data = newData;
171+
AsciiStringData* oldData = (AsciiStringData*)InterlockedExchangePointer((PVOID*)&m_data, newData);
172+
if (oldData)
173+
{
174+
if (InterlockedDecrement(&oldData->m_refCount) == 0)
175+
{
176+
TheDynamicMemoryAllocator->freeBytes(oldData);
177+
}
178+
}
176179

177180
validate();
178181
}
@@ -181,18 +184,15 @@ void AsciiString::ensureUniqueBufferOfSize(int numCharsNeeded, Bool preserveData
181184
// -----------------------------------------------------
182185
void AsciiString::releaseBuffer()
183186
{
184-
ScopedCriticalSection scopedCriticalSection(TheAsciiStringCriticalSection);
185-
186187
validate();
187-
if (m_data)
188+
AsciiStringData* data = (AsciiStringData*)InterlockedExchangePointer((PVOID*)&m_data, nullptr);
189+
if (data)
188190
{
189-
if (--m_data->m_refCount == 0)
191+
if (InterlockedDecrement(&data->m_refCount) == 0)
190192
{
191-
TheDynamicMemoryAllocator->freeBytes(m_data);
193+
TheDynamicMemoryAllocator->freeBytes(data);
192194
}
193-
m_data = nullptr;
194195
}
195-
validate();
196196
}
197197

198198
// -----------------------------------------------------
@@ -220,15 +220,23 @@ AsciiString::AsciiString(const char* s, int len) : m_data(nullptr)
220220
// -----------------------------------------------------
221221
void AsciiString::set(const AsciiString& stringSrc)
222222
{
223-
ScopedCriticalSection scopedCriticalSection(TheAsciiStringCriticalSection);
224-
225223
validate();
226224
if (&stringSrc != this)
227225
{
228-
releaseBuffer();
229-
m_data = stringSrc.m_data;
230-
if (m_data)
231-
++m_data->m_refCount;
226+
AsciiStringData* newData = stringSrc.m_data;
227+
if (newData)
228+
{
229+
InterlockedIncrement(&newData->m_refCount);
230+
}
231+
232+
AsciiStringData* oldData = (AsciiStringData*)InterlockedExchangePointer((PVOID*)&m_data, newData);
233+
if (oldData)
234+
{
235+
if (InterlockedDecrement(&oldData->m_refCount) == 0)
236+
{
237+
TheDynamicMemoryAllocator->freeBytes(oldData);
238+
}
239+
}
232240
}
233241
validate();
234242
}

‎Core/GameEngine/Source/Common/System/UnicodeString.cpp‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@
4444
///////////////////////////////////////////////////////////////////////////////
4545
#include "PreRTS.h" // This must go first in EVERY cpp file in the GameEngine
4646

47-
#include "Common/CriticalSection.h"
48-
4947

5048
// -----------------------------------------------------
5149

@@ -65,9 +63,8 @@ void UnicodeString::validate() const
6563
// -----------------------------------------------------
6664
UnicodeString::UnicodeString(const UnicodeString& stringSrc) : m_data(stringSrc.m_data)
6765
{
68-
ScopedCriticalSection scopedCriticalSection(TheUnicodeStringCriticalSection);
6966
if (m_data)
70-
++m_data->m_refCount;
67+
InterlockedIncrement(&m_data->m_refCount);
7168
validate();
7269
}
7370

@@ -122,8 +119,14 @@ void UnicodeString::ensureUniqueBufferOfSize(int numCharsNeeded, Bool preserveDa
122119
if (strToCat)
123120
wcscat(newData->peek(), strToCat);
124121

125-
releaseBuffer();
126-
m_data = newData;
122+
UnicodeStringData* oldData = (UnicodeStringData*)InterlockedExchangePointer((PVOID*)&m_data, newData);
123+
if (oldData)
124+
{
125+
if (InterlockedDecrement(&oldData->m_refCount) == 0)
126+
{
127+
TheDynamicMemoryAllocator->freeBytes(oldData);
128+
}
129+
}
127130

128131
validate();
129132
}
@@ -132,16 +135,14 @@ void UnicodeString::ensureUniqueBufferOfSize(int numCharsNeeded, Bool preserveDa
132135
// -----------------------------------------------------
133136
void UnicodeString::releaseBuffer()
134137
{
135-
ScopedCriticalSection scopedCriticalSection(TheUnicodeStringCriticalSection);
136-
137138
validate();
138-
if (m_data)
139+
UnicodeStringData* data = (UnicodeStringData*)InterlockedExchangePointer((PVOID*)&m_data, nullptr);
140+
if (data)
139141
{
140-
if (--m_data->m_refCount == 0)
142+
if (InterlockedDecrement(&data->m_refCount) == 0)
141143
{
142-
TheDynamicMemoryAllocator->freeBytes(m_data);
144+
TheDynamicMemoryAllocator->freeBytes(data);
143145
}
144-
m_data = nullptr;
145146
}
146147
}
147148

@@ -169,15 +170,23 @@ UnicodeString::UnicodeString(const WideChar* s, int len) : m_data(nullptr)
169170
// -----------------------------------------------------
170171
void UnicodeString::set(const UnicodeString& stringSrc)
171172
{
172-
ScopedCriticalSection scopedCriticalSection(TheUnicodeStringCriticalSection);
173-
174173
validate();
175174
if (&stringSrc != this)
176175
{
177-
releaseBuffer();
178-
m_data = stringSrc.m_data;
179-
if (m_data)
180-
++m_data->m_refCount;
176+
UnicodeStringData* newData = stringSrc.m_data;
177+
if (newData)
178+
{
179+
InterlockedIncrement(&newData->m_refCount);
180+
}
181+
182+
UnicodeStringData* oldData = (UnicodeStringData*)InterlockedExchangePointer((PVOID*)&m_data, newData);
183+
if (oldData)
184+
{
185+
if (InterlockedDecrement(&oldData->m_refCount) == 0)
186+
{
187+
TheDynamicMemoryAllocator->freeBytes(oldData);
188+
}
189+
}
181190
}
182191
validate();
183192
}

‎Core/GameEngineDevice/Source/W3DDevice/GameClient/Water/W3DWaterTracks.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -957,7 +957,7 @@ void WaterTracksRenderSystem::saveTracks()
957957
if (!TheTerrainLogic)
958958
return;
959959

960-
AsciiString fileName=TheTerrainLogic->getSourceFilename();
960+
const AsciiString& fileName=TheTerrainLogic->getSourceFilename();
961961
char path[256];
962962

963963
strlcpy(path, fileName.str(), ARRAY_SIZE(path));
@@ -993,7 +993,7 @@ void WaterTracksRenderSystem::loadTracks()
993993
if (!TheTerrainLogic)
994994
return;
995995

996-
AsciiString fileName=TheTerrainLogic->getSourceFilename();
996+
const AsciiString& fileName=TheTerrainLogic->getSourceFilename();
997997
char path[256];
998998

999999
strlcpy(path, fileName.str(), ARRAY_SIZE(path));

‎Generals/Code/GameEngine/Include/Common/CriticalSection.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ class ScopedCriticalSection
9494

9595
// These should be null on creation then non-null in WinMain or equivalent.
9696
// This allows us to be silently non-threadsafe for WB and other single-threaded apps.
97-
extern CriticalSection *TheAsciiStringCriticalSection;
98-
extern CriticalSection *TheUnicodeStringCriticalSection;
9997
extern CriticalSection *TheDmaCriticalSection;
10098
extern CriticalSection *TheMemoryPoolCriticalSection;
10199
extern CriticalSection *TheDebugLogCriticalSection;

‎Generals/Code/GameEngine/Source/Common/System/CriticalSection.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@
2727
#include "Common/CriticalSection.h"
2828

2929
// Definitions.
30-
CriticalSection *TheAsciiStringCriticalSection = nullptr;
31-
CriticalSection *TheUnicodeStringCriticalSection = nullptr;
3230
CriticalSection *TheDmaCriticalSection = nullptr;
3331
CriticalSection *TheMemoryPoolCriticalSection = nullptr;
3432
CriticalSection *TheDebugLogCriticalSection = nullptr;

‎Generals/Code/GameEngine/Source/GameLogic/Map/TerrainLogic.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ Bridge::Bridge(Object *bridgeObj)
377377
m_bridgeInfo.bridgeObjectID = bridgeObj->getID();
378378

379379
// get the template of the bridge
380-
AsciiString bridgeTemplateName = bridgeObj->getTemplate()->getName();
380+
const AsciiString& bridgeTemplateName = bridgeObj->getTemplate()->getName();
381381
TerrainRoadType *bridgeTemplate = TheTerrainRoads->findBridge( bridgeTemplateName );
382382
if( bridgeTemplate == nullptr ) {
383383
DEBUG_LOG(( "*** Bridge Template Not Found '%s'.", bridgeTemplateName.str() ));

‎Generals/Code/Main/WinMain.cpp‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ static Bool initializeAppWindows( HINSTANCE hInstance, Int nCmdShow, Bool runWin
769769
}
770770

771771
// Necessary to allow memory managers and such to have useful critical sections
772-
static CriticalSection critSec1, critSec2, critSec3, critSec4, critSec5;
772+
static CriticalSection critSec1, critSec2, critSec3;
773773

774774
// UnHandledExceptionFilter ===================================================
775775
/** Handler for unhandled win32 exceptions. */
@@ -808,11 +808,9 @@ Int APIENTRY WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,
808808
// default in a DevStudio project
809809
//
810810

811-
TheAsciiStringCriticalSection = &critSec1;
812-
TheUnicodeStringCriticalSection = &critSec2;
813-
TheDmaCriticalSection = &critSec3;
814-
TheMemoryPoolCriticalSection = &critSec4;
815-
TheDebugLogCriticalSection = &critSec5;
811+
TheDmaCriticalSection = &critSec1;
812+
TheMemoryPoolCriticalSection = &critSec2;
813+
TheDebugLogCriticalSection = &critSec3;
816814

817815
// initialize the memory manager early
818816
initMemoryManager();
@@ -923,8 +921,6 @@ Int APIENTRY WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,
923921
#ifdef RTS_ENABLE_CRASHDUMP
924922
MiniDumper::shutdownMiniDumper();
925923
#endif
926-
TheAsciiStringCriticalSection = nullptr;
927-
TheUnicodeStringCriticalSection = nullptr;
928924
TheDmaCriticalSection = nullptr;
929925
TheMemoryPoolCriticalSection = nullptr;
930926

‎GeneralsMD/Code/GameEngine/Include/Common/CriticalSection.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,6 @@ class ScopedCriticalSection
9494

9595
// These should be null on creation then non-null in WinMain or equivalent.
9696
// This allows us to be silently non-threadsafe for WB and other single-threaded apps.
97-
extern CriticalSection *TheAsciiStringCriticalSection;
98-
extern CriticalSection *TheUnicodeStringCriticalSection;
9997
extern CriticalSection *TheDmaCriticalSection;
10098
extern CriticalSection *TheMemoryPoolCriticalSection;
10199
extern CriticalSection *TheDebugLogCriticalSection;

0 commit comments

Comments
 (0)