From be209055828d215095fe956590d45386ceb52010 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sun, 12 Nov 2017 15:28:58 -0700 Subject: [PATCH 1/8] Memory: Non-Functional: Rationalize and improve encapsulation of TLS usage. This will make the next (functional) commit easier to see. --- OGLCompilersDLL/InitializeDll.cpp | 16 ++++-- OGLCompilersDLL/InitializeDll.h | 2 +- glslang/Include/InitializeGlobals.h | 3 +- glslang/Include/PoolAlloc.h | 9 +-- glslang/MachineIndependent/PoolAlloc.cpp | 67 +++++++++++++---------- glslang/MachineIndependent/ShaderLang.cpp | 24 ++++---- glslang/Public/ShaderLang.h | 7 +-- 7 files changed, 70 insertions(+), 58 deletions(-) diff --git a/OGLCompilersDLL/InitializeDll.cpp b/OGLCompilersDLL/InitializeDll.cpp index 2eb912c4..c1f03c09 100644 --- a/OGLCompilersDLL/InitializeDll.cpp +++ b/OGLCompilersDLL/InitializeDll.cpp @@ -45,6 +45,10 @@ namespace glslang { OS_TLSIndex ThreadInitializeIndex = OS_INVALID_TLS_INDEX; +// Per-process initialization. +// Needs to be called at least once before parsing, etc. is done. +// Will also do thread initialization for the calling thread; other +// threads will need to do that explicitly. bool InitProcess() { glslang::GetGlobalLock(); @@ -85,7 +89,9 @@ bool InitProcess() return true; } - +// Per-thread scoped initialization. +// Must be called at least once by each new thread sharing the +// symbol tables, etc., needed to parse. bool InitThread() { // @@ -109,7 +115,9 @@ bool InitThread() return true; } - +// Thread-scoped tear down. Needs to be done by all but the last +// thread, which calls DetachProcess() instead. +// NB. TODO: Not currently being executed by each thread. bool DetachThread() { bool success = true; @@ -126,13 +134,13 @@ bool DetachThread() success = false; } - FreeGlobalPools(); - + FreeMemoryPools(); } return success; } +// Process-scoped tear down. Needs to be done by final thread in process. bool DetachProcess() { bool success = true; diff --git a/OGLCompilersDLL/InitializeDll.h b/OGLCompilersDLL/InitializeDll.h index 60b2b159..281f3b1a 100644 --- a/OGLCompilersDLL/InitializeDll.h +++ b/OGLCompilersDLL/InitializeDll.h @@ -40,7 +40,7 @@ namespace glslang { bool InitProcess(); bool InitThread(); -bool DetachThread(); +bool DetachThread(); // TODO: use this or remove it; ideally make it unneeded bool DetachProcess(); } // end namespace glslang diff --git a/glslang/Include/InitializeGlobals.h b/glslang/Include/InitializeGlobals.h index 4cf2dca7..dd7554b6 100644 --- a/glslang/Include/InitializeGlobals.h +++ b/glslang/Include/InitializeGlobals.h @@ -38,7 +38,8 @@ namespace glslang { void InitializeMemoryPools(); -void FreeGlobalPools(); +void FreeMemoryPools(); + bool InitializePoolIndex(); void FreePoolIndex(); diff --git a/glslang/Include/PoolAlloc.h b/glslang/Include/PoolAlloc.h index 69bacb15..0e237a6a 100644 --- a/glslang/Include/PoolAlloc.h +++ b/glslang/Include/PoolAlloc.h @@ -250,15 +250,8 @@ private: // different times. But a simple use is to have a global pop // with everyone using the same global allocator. // -typedef TPoolAllocator* PoolAllocatorPointer; extern TPoolAllocator& GetThreadPoolAllocator(); - -struct TThreadMemoryPools -{ - TPoolAllocator* threadPoolAllocator; -}; - -void SetThreadPoolAllocator(TPoolAllocator& poolAllocator); +void SetThreadPoolAllocator(TPoolAllocator* poolAllocator); // // This STL compatible allocator is intended to be used as the allocator diff --git a/glslang/MachineIndependent/PoolAlloc.cpp b/glslang/MachineIndependent/PoolAlloc.cpp index 4007c386..5077a26b 100644 --- a/glslang/MachineIndependent/PoolAlloc.cpp +++ b/glslang/MachineIndependent/PoolAlloc.cpp @@ -40,35 +40,40 @@ namespace glslang { +// Process-wide TLS index OS_TLSIndex PoolIndex; -void InitializeMemoryPools() +// Per-thread structure holding pool pointers. +struct TThreadMemoryPools { - TThreadMemoryPools* pools = static_cast(OS_GetTLSValue(PoolIndex)); - if (pools) - return; + TPoolAllocator* threadPoolAllocator; // the current pool +}; - TPoolAllocator *threadPoolAllocator = new TPoolAllocator(); - - TThreadMemoryPools* threadData = new TThreadMemoryPools(); - - threadData->threadPoolAllocator = threadPoolAllocator; - - OS_SetTLSValue(PoolIndex, threadData); +// Return the thread-specific pool pointers. +TThreadMemoryPools* GetThreadMemoryPools() +{ + return static_cast(OS_GetTLSValue(PoolIndex)); } -void FreeGlobalPools() +// Set the thread-specific pool pointers. +void SetThreadMemoryPools(TThreadMemoryPools* pools) { - // Release the allocated memory for this thread. - TThreadMemoryPools* globalPools = static_cast(OS_GetTLSValue(PoolIndex)); - if (! globalPools) - return; - - GetThreadPoolAllocator().popAll(); - delete &GetThreadPoolAllocator(); - delete globalPools; + OS_SetTLSValue(PoolIndex, pools); } +// Return the thread-specific current pool. +TPoolAllocator& GetThreadPoolAllocator() +{ + return *GetThreadMemoryPools()->threadPoolAllocator; +} + +// Set the thread-specific current pool. +void SetThreadPoolAllocator(TPoolAllocator* poolAllocator) +{ + GetThreadMemoryPools()->threadPoolAllocator = poolAllocator; +} + +// Process-wide set up of the TLS pool storage. bool InitializePoolIndex() { // Allocate a TLS index. @@ -78,24 +83,30 @@ bool InitializePoolIndex() return true; } +// Process-wide tear down of the TLS pool storage. void FreePoolIndex() { // Release the TLS index. OS_FreeTLSIndex(PoolIndex); } -TPoolAllocator& GetThreadPoolAllocator() +// Per-thread set up of the memory pools. +void InitializeMemoryPools() { - TThreadMemoryPools* threadData = static_cast(OS_GetTLSValue(PoolIndex)); - - return *threadData->threadPoolAllocator; + if (GetThreadMemoryPools() == nullptr) { + SetThreadMemoryPools(new TThreadMemoryPools()); + SetThreadPoolAllocator(new TPoolAllocator()); + } } -void SetThreadPoolAllocator(TPoolAllocator& poolAllocator) +// Per-thread tear down of the memory pools. +void FreeMemoryPools() { - TThreadMemoryPools* threadData = static_cast(OS_GetTLSValue(PoolIndex)); - - threadData->threadPoolAllocator = &poolAllocator; + if (GetThreadMemoryPools() != nullptr) { + GetThreadPoolAllocator().popAll(); + delete &GetThreadPoolAllocator(); + delete GetThreadMemoryPools(); + } } // diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index c8e954ce..8de5297b 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -217,7 +217,7 @@ enum EPrecisionClass { TSymbolTable* CommonSymbolTable[VersionCount][SpvVersionCount][ProfileCount][SourceCount][EPcCount] = {}; TSymbolTable* SharedSymbolTables[VersionCount][SpvVersionCount][ProfileCount][SourceCount][EShLangCount] = {}; -TPoolAllocator* PerProcessGPA = 0; +TPoolAllocator* PerProcessGPA = nullptr; // // Parse and add to the given symbol table the content of the given shader string. @@ -361,7 +361,7 @@ bool AddContextSpecificSymbols(const TBuiltInResource* resources, TInfoSink& inf // pool allocator intact, so: // - Switch to a new pool for parsing the built-ins // - Do the parsing, which builds the symbol table, using the new pool -// - Switch to the process-global pool to save a copy the resulting symbol table +// - Switch to the process-global pool to save a copy of the resulting symbol table // - Free up the new pool used to parse the built-ins // - Switch back to the original thread's pool // @@ -388,8 +388,8 @@ void SetupBuiltinSymbolTable(int version, EProfile profile, const SpvVersion& sp // Switch to a new pool TPoolAllocator& previousAllocator = GetThreadPoolAllocator(); - TPoolAllocator* builtInPoolAllocator = new TPoolAllocator(); - SetThreadPoolAllocator(*builtInPoolAllocator); + TPoolAllocator* builtInPoolAllocator = new TPoolAllocator; + SetThreadPoolAllocator(builtInPoolAllocator); // Dynamically allocate the local symbol tables so we can control when they are deallocated WRT when the pool is popped. TSymbolTable* commonTable[EPcCount]; @@ -403,7 +403,7 @@ void SetupBuiltinSymbolTable(int version, EProfile profile, const SpvVersion& sp InitializeSymbolTables(infoSink, commonTable, stageTables, version, profile, spvVersion, source); // Switch to the process-global pool - SetThreadPoolAllocator(*PerProcessGPA); + SetThreadPoolAllocator(PerProcessGPA); // Copy the local symbol tables from the new pool to the global tables using the process-global pool for (int precClass = 0; precClass < EPcCount; ++precClass) { @@ -430,7 +430,7 @@ void SetupBuiltinSymbolTable(int version, EProfile profile, const SpvVersion& sp delete stageTables[stage]; delete builtInPoolAllocator; - SetThreadPoolAllocator(previousAllocator); + SetThreadPoolAllocator(&previousAllocator); glslang::ReleaseGlobalLock(); } @@ -1196,7 +1196,7 @@ int ShInitialize() if (! InitProcess()) return 0; - if (! PerProcessGPA) + if (PerProcessGPA == nullptr) PerProcessGPA = new TPoolAllocator(); glslang::TScanContext::fillInKeywordMap(); @@ -1288,10 +1288,10 @@ int __fastcall ShFinalize() } } - if (PerProcessGPA) { + if (PerProcessGPA != nullptr) { PerProcessGPA->popAll(); delete PerProcessGPA; - PerProcessGPA = 0; + PerProcessGPA = nullptr; } glslang::TScanContext::deleteKeywordMap(); @@ -1708,7 +1708,7 @@ bool TShader::parse(const TBuiltInResource* builtInResources, int defaultVersion return false; pool = new TPoolAllocator(); - SetThreadPoolAllocator(*pool); + SetThreadPoolAllocator(pool); if (! preamble) preamble = ""; @@ -1732,7 +1732,7 @@ bool TShader::preprocess(const TBuiltInResource* builtInResources, return false; pool = new TPoolAllocator(); - SetThreadPoolAllocator(*pool); + SetThreadPoolAllocator(pool); if (! preamble) preamble = ""; @@ -1789,7 +1789,7 @@ bool TProgram::link(EShMessages messages) bool error = false; pool = new TPoolAllocator(); - SetThreadPoolAllocator(*pool); + SetThreadPoolAllocator(pool); for (int s = 0; s < EShLangCount; ++s) { if (! linkStage((EShLanguage)s, messages)) diff --git a/glslang/Public/ShaderLang.h b/glslang/Public/ShaderLang.h index 6fadfbf0..6e22bdd7 100644 --- a/glslang/Public/ShaderLang.h +++ b/glslang/Public/ShaderLang.h @@ -68,15 +68,14 @@ #endif // -// Driver must call this first, once, before doing any other -// compiler/linker operations. +// Call before doing any other compiler/linker operations. // // (Call once per process, not once per thread.) // SH_IMPORT_EXPORT int ShInitialize(); // -// Driver should call this at process shutdown. +// Call this at process shutdown to clean up memory. // SH_IMPORT_EXPORT int __fastcall ShFinalize(); @@ -290,7 +289,7 @@ SH_IMPORT_EXPORT int ShGetUniformLocation(const ShHandle uniformMap, const char* // Deferred-Lowering C++ Interface // ----------------------------------- // -// Below is a new alternate C++ interface that might potentially replace the above +// Below is a new alternate C++ interface, which deprecates the above // opaque handle-based interface. // // The below is further designed to handle multiple compilation units per stage, where From 4ceaab166cd1af6b582faa4cf51036e00a0d88e3 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sun, 12 Nov 2017 15:43:03 -0700 Subject: [PATCH 2/8] Memory: Move to a normal model of ownership of memory pools, for new/delete. Addresses step 4 of #976, overlaps #916. For each pool, now, it is newed, remembered, and freed by the same entity, rather than having a mix (thread finalize freeing current pool) that could lead to double freeing of the same pool. It is quite rational and simple now. This will enable reinstalling process and thread tear down. --- glslang/MachineIndependent/PoolAlloc.cpp | 11 ++++++++--- glslang/MachineIndependent/ShaderLang.cpp | 9 ++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/glslang/MachineIndependent/PoolAlloc.cpp b/glslang/MachineIndependent/PoolAlloc.cpp index 5077a26b..57943b1f 100644 --- a/glslang/MachineIndependent/PoolAlloc.cpp +++ b/glslang/MachineIndependent/PoolAlloc.cpp @@ -47,6 +47,7 @@ OS_TLSIndex PoolIndex; struct TThreadMemoryPools { TPoolAllocator* threadPoolAllocator; // the current pool + TPoolAllocator* initialMemoryPool; // the original pool owned by this thread (this file), to be freed here as well }; // Return the thread-specific pool pointers. @@ -80,6 +81,8 @@ bool InitializePoolIndex() if ((PoolIndex = OS_AllocTLSIndex()) == OS_INVALID_TLS_INDEX) return false; + SetThreadMemoryPools(nullptr); + return true; } @@ -95,7 +98,8 @@ void InitializeMemoryPools() { if (GetThreadMemoryPools() == nullptr) { SetThreadMemoryPools(new TThreadMemoryPools()); - SetThreadPoolAllocator(new TPoolAllocator()); + GetThreadMemoryPools()->initialMemoryPool = new TPoolAllocator(); + SetThreadPoolAllocator(GetThreadMemoryPools()->initialMemoryPool); } } @@ -103,9 +107,10 @@ void InitializeMemoryPools() void FreeMemoryPools() { if (GetThreadMemoryPools() != nullptr) { - GetThreadPoolAllocator().popAll(); - delete &GetThreadPoolAllocator(); + if (GetThreadMemoryPools()->initialMemoryPool != nullptr) + delete GetThreadMemoryPools()->initialMemoryPool; delete GetThreadMemoryPools(); + SetThreadMemoryPools(nullptr); } } diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index 8de5297b..d1dec88b 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -1602,8 +1602,9 @@ public: }; TShader::TShader(EShLanguage s) - : pool(0), stage(s), lengths(nullptr), stringNames(nullptr), preamble("") + : stage(s), lengths(nullptr), stringNames(nullptr), preamble("") { + pool = new TPoolAllocator; infoSink = new TInfoSink; compiler = new TDeferredCompiler(stage, *infoSink); intermediate = new TIntermediate(s); @@ -1707,7 +1708,6 @@ bool TShader::parse(const TBuiltInResource* builtInResources, int defaultVersion if (! InitThread()) return false; - pool = new TPoolAllocator(); SetThreadPoolAllocator(pool); if (! preamble) preamble = ""; @@ -1731,7 +1731,6 @@ bool TShader::preprocess(const TBuiltInResource* builtInResources, if (! InitThread()) return false; - pool = new TPoolAllocator(); SetThreadPoolAllocator(pool); if (! preamble) preamble = ""; @@ -1752,8 +1751,9 @@ const char* TShader::getInfoDebugLog() return infoSink->debug.c_str(); } -TProgram::TProgram() : pool(0), reflection(0), ioMapper(nullptr), linked(false) +TProgram::TProgram() : reflection(0), ioMapper(nullptr), linked(false) { + pool = new TPoolAllocator; infoSink = new TInfoSink; for (int s = 0; s < EShLangCount; ++s) { intermediate[s] = 0; @@ -1788,7 +1788,6 @@ bool TProgram::link(EShMessages messages) bool error = false; - pool = new TPoolAllocator(); SetThreadPoolAllocator(pool); for (int s = 0; s < EShLangCount; ++s) { From ff8e59f5105bc04d5e26fd5766efc30c8f1ce6ff Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sun, 12 Nov 2017 15:55:22 -0700 Subject: [PATCH 3/8] Memory: Do process and 1st thread tear down. Addresses #928, #389, and 1st item in #976. Overlaps #916. This had been dropped, when moving away from the old Win32 DLL model. Issue: per-thread tear down for other threads. --- OGLCompilersDLL/InitializeDll.cpp | 2 -- glslang/MachineIndependent/ShaderLang.cpp | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/OGLCompilersDLL/InitializeDll.cpp b/OGLCompilersDLL/InitializeDll.cpp index c1f03c09..67bf15f9 100644 --- a/OGLCompilersDLL/InitializeDll.cpp +++ b/OGLCompilersDLL/InitializeDll.cpp @@ -148,8 +148,6 @@ bool DetachProcess() if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) return true; - ShFinalize(); - success = DetachThread(); FreePoolIndex(); diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index d1dec88b..43a94b79 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -1299,7 +1299,7 @@ int __fastcall ShFinalize() glslang::HlslScanContext::deleteKeywordMap(); #endif - return 1; + return DetachProcess() ? 1 : 0; } // From cb42541e513a3d2d023730e07c5a05a0c608062c Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Sun, 12 Nov 2017 23:12:57 -0700 Subject: [PATCH 4/8] Memory: Remove the need for per-thread tear down. Make key objects using the memory pool own their own pool and delete it, such that there is not generic per-thread pool to manage. --- OGLCompilersDLL/InitializeDll.cpp | 49 ++------------------- OGLCompilersDLL/InitializeDll.h | 2 - glslang/Include/InitializeGlobals.h | 4 -- glslang/Include/ShHandle.h | 7 ++- glslang/MachineIndependent/PoolAlloc.cpp | 53 +---------------------- glslang/MachineIndependent/ShaderLang.cpp | 31 +++---------- glslang/OSDependent/Unix/ossource.cpp | 1 - 7 files changed, 17 insertions(+), 130 deletions(-) diff --git a/OGLCompilersDLL/InitializeDll.cpp b/OGLCompilersDLL/InitializeDll.cpp index 67bf15f9..d4e1b9f5 100644 --- a/OGLCompilersDLL/InitializeDll.cpp +++ b/OGLCompilersDLL/InitializeDll.cpp @@ -38,8 +38,8 @@ #include "InitializeDll.h" #include "../glslang/Include/InitializeGlobals.h" - #include "../glslang/Public/ShaderLang.h" +#include "../glslang/Include/PoolAlloc.h" namespace glslang { @@ -105,57 +105,14 @@ bool InitThread() if (OS_GetTLSValue(ThreadInitializeIndex) != 0) return true; - InitializeMemoryPools(); - if (! OS_SetTLSValue(ThreadInitializeIndex, (void *)1)) { assert(0 && "InitThread(): Unable to set init flag."); return false; } + glslang::SetThreadPoolAllocator(nullptr); + return true; } -// Thread-scoped tear down. Needs to be done by all but the last -// thread, which calls DetachProcess() instead. -// NB. TODO: Not currently being executed by each thread. -bool DetachThread() -{ - bool success = true; - - if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) - return true; - - // - // Function is re-entrant and this thread may not have been initialized. - // - if (OS_GetTLSValue(ThreadInitializeIndex) != 0) { - if (!OS_SetTLSValue(ThreadInitializeIndex, (void *)0)) { - assert(0 && "DetachThread(): Unable to clear init flag."); - success = false; - } - - FreeMemoryPools(); - } - - return success; -} - -// Process-scoped tear down. Needs to be done by final thread in process. -bool DetachProcess() -{ - bool success = true; - - if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) - return true; - - success = DetachThread(); - - FreePoolIndex(); - - OS_FreeTLSIndex(ThreadInitializeIndex); - ThreadInitializeIndex = OS_INVALID_TLS_INDEX; - - return success; -} - } // end namespace glslang diff --git a/OGLCompilersDLL/InitializeDll.h b/OGLCompilersDLL/InitializeDll.h index 281f3b1a..3f27ce9b 100644 --- a/OGLCompilersDLL/InitializeDll.h +++ b/OGLCompilersDLL/InitializeDll.h @@ -40,8 +40,6 @@ namespace glslang { bool InitProcess(); bool InitThread(); -bool DetachThread(); // TODO: use this or remove it; ideally make it unneeded -bool DetachProcess(); } // end namespace glslang diff --git a/glslang/Include/InitializeGlobals.h b/glslang/Include/InitializeGlobals.h index dd7554b6..95d0a40e 100644 --- a/glslang/Include/InitializeGlobals.h +++ b/glslang/Include/InitializeGlobals.h @@ -37,11 +37,7 @@ namespace glslang { -void InitializeMemoryPools(); -void FreeMemoryPools(); - bool InitializePoolIndex(); -void FreePoolIndex(); } // end namespace glslang diff --git a/glslang/Include/ShHandle.h b/glslang/Include/ShHandle.h index 64ba6d63..df07bd8e 100644 --- a/glslang/Include/ShHandle.h +++ b/glslang/Include/ShHandle.h @@ -56,11 +56,14 @@ class TUniformMap; // class TShHandleBase { public: - TShHandleBase() { } - virtual ~TShHandleBase() { } + TShHandleBase() { pool = new glslang::TPoolAllocator; } + virtual ~TShHandleBase() { delete pool; } virtual TCompiler* getAsCompiler() { return 0; } virtual TLinker* getAsLinker() { return 0; } virtual TUniformMap* getAsUniformMap() { return 0; } + virtual glslang::TPoolAllocator* getPool() const { return pool; } +private: + glslang::TPoolAllocator* pool; }; // diff --git a/glslang/MachineIndependent/PoolAlloc.cpp b/glslang/MachineIndependent/PoolAlloc.cpp index 57943b1f..c42057c2 100644 --- a/glslang/MachineIndependent/PoolAlloc.cpp +++ b/glslang/MachineIndependent/PoolAlloc.cpp @@ -43,35 +43,16 @@ namespace glslang { // Process-wide TLS index OS_TLSIndex PoolIndex; -// Per-thread structure holding pool pointers. -struct TThreadMemoryPools -{ - TPoolAllocator* threadPoolAllocator; // the current pool - TPoolAllocator* initialMemoryPool; // the original pool owned by this thread (this file), to be freed here as well -}; - -// Return the thread-specific pool pointers. -TThreadMemoryPools* GetThreadMemoryPools() -{ - return static_cast(OS_GetTLSValue(PoolIndex)); -} - -// Set the thread-specific pool pointers. -void SetThreadMemoryPools(TThreadMemoryPools* pools) -{ - OS_SetTLSValue(PoolIndex, pools); -} - // Return the thread-specific current pool. TPoolAllocator& GetThreadPoolAllocator() { - return *GetThreadMemoryPools()->threadPoolAllocator; + return *static_cast(OS_GetTLSValue(PoolIndex)); } // Set the thread-specific current pool. void SetThreadPoolAllocator(TPoolAllocator* poolAllocator) { - GetThreadMemoryPools()->threadPoolAllocator = poolAllocator; + OS_SetTLSValue(PoolIndex, poolAllocator); } // Process-wide set up of the TLS pool storage. @@ -81,39 +62,9 @@ bool InitializePoolIndex() if ((PoolIndex = OS_AllocTLSIndex()) == OS_INVALID_TLS_INDEX) return false; - SetThreadMemoryPools(nullptr); - return true; } -// Process-wide tear down of the TLS pool storage. -void FreePoolIndex() -{ - // Release the TLS index. - OS_FreeTLSIndex(PoolIndex); -} - -// Per-thread set up of the memory pools. -void InitializeMemoryPools() -{ - if (GetThreadMemoryPools() == nullptr) { - SetThreadMemoryPools(new TThreadMemoryPools()); - GetThreadMemoryPools()->initialMemoryPool = new TPoolAllocator(); - SetThreadPoolAllocator(GetThreadMemoryPools()->initialMemoryPool); - } -} - -// Per-thread tear down of the memory pools. -void FreeMemoryPools() -{ - if (GetThreadMemoryPools() != nullptr) { - if (GetThreadMemoryPools()->initialMemoryPool != nullptr) - delete GetThreadMemoryPools()->initialMemoryPool; - delete GetThreadMemoryPools(); - SetThreadMemoryPools(nullptr); - } -} - // // Implement the functionality of the TPoolAllocator class, which // is documented in PoolAlloc.h. diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index 43a94b79..ba6e8c44 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -722,9 +722,6 @@ bool ProcessDeferred( const std::string sourceEntryPointName = "", const TEnvironment* environment = nullptr) // optional way of fully setting all versions, overriding the above { - if (! InitThread()) - return false; - // This must be undone (.pop()) by the caller, after it finishes consuming the created tree. GetThreadPoolAllocator().push(); @@ -1299,7 +1296,7 @@ int __fastcall ShFinalize() glslang::HlslScanContext::deleteKeywordMap(); #endif - return DetachProcess() ? 1 : 0; + return 1; } // @@ -1332,6 +1329,8 @@ int ShCompile( if (compiler == 0) return 0; + SetThreadPoolAllocator(compiler->getPool()); + compiler->infoSink.info.erase(); compiler->infoSink.debug.erase(); @@ -1389,6 +1388,8 @@ int ShLinkExt( TShHandleBase* base = reinterpret_cast(linkHandle); TLinker* linker = static_cast(base->getAsLinker()); + SetThreadPoolAllocator(linker->getPool()); + if (linker == 0) return 0; @@ -1423,9 +1424,6 @@ void ShSetEncryptionMethod(ShHandle handle) // const char* ShGetInfoLog(const ShHandle handle) { - if (!InitThread()) - return 0; - if (handle == 0) return 0; @@ -1449,9 +1447,6 @@ const char* ShGetInfoLog(const ShHandle handle) // const void* ShGetExecutable(const ShHandle handle) { - if (!InitThread()) - return 0; - if (handle == 0) return 0; @@ -1474,9 +1469,6 @@ const void* ShGetExecutable(const ShHandle handle) // int ShSetVirtualAttributeBindings(const ShHandle handle, const ShBindingTable* table) { - if (!InitThread()) - return 0; - if (handle == 0) return 0; @@ -1496,9 +1488,6 @@ int ShSetVirtualAttributeBindings(const ShHandle handle, const ShBindingTable* t // int ShSetFixedAttributeBindings(const ShHandle handle, const ShBindingTable* table) { - if (!InitThread()) - return 0; - if (handle == 0) return 0; @@ -1517,9 +1506,6 @@ int ShSetFixedAttributeBindings(const ShHandle handle, const ShBindingTable* tab // int ShExcludeAttributes(const ShHandle handle, int *attributes, int count) { - if (!InitThread()) - return 0; - if (handle == 0) return 0; @@ -1541,9 +1527,6 @@ int ShExcludeAttributes(const ShHandle handle, int *attributes, int count) // int ShGetUniformLocation(const ShHandle handle, const char* name) { - if (!InitThread()) - return 0; - if (handle == 0) return -1; @@ -1707,8 +1690,8 @@ bool TShader::parse(const TBuiltInResource* builtInResources, int defaultVersion { if (! InitThread()) return false; - SetThreadPoolAllocator(pool); + if (! preamble) preamble = ""; @@ -1730,8 +1713,8 @@ bool TShader::preprocess(const TBuiltInResource* builtInResources, { if (! InitThread()) return false; - SetThreadPoolAllocator(pool); + if (! preamble) preamble = ""; diff --git a/glslang/OSDependent/Unix/ossource.cpp b/glslang/OSDependent/Unix/ossource.cpp index 24b77e16..9c16dc40 100644 --- a/glslang/OSDependent/Unix/ossource.cpp +++ b/glslang/OSDependent/Unix/ossource.cpp @@ -56,7 +56,6 @@ namespace glslang { // static void DetachThreadLinux(void *) { - DetachThread(); } // From 94f28eb61a77d349a36474d55cc368c7bd8b4f18 Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Mon, 13 Nov 2017 01:32:06 -0700 Subject: [PATCH 5/8] Memory: Add loop around main, to test tear-down and reuse, and monitor memory changes. --- StandAlone/StandAlone.cpp | 44 +++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/StandAlone/StandAlone.cpp b/StandAlone/StandAlone.cpp index 87d1f5b7..91ed5f1b 100644 --- a/StandAlone/StandAlone.cpp +++ b/StandAlone/StandAlone.cpp @@ -127,6 +127,9 @@ void InfoLogMsg(const char* msg, const char* name, const int num); bool CompileFailed = false; bool LinkFailed = false; +// array of unique places to leave the shader names and infologs for the asynchronous compiles +std::vector> WorkItems; + TBuiltInResource Resources; std::string ConfigFile; @@ -1022,14 +1025,10 @@ void CompileAndLinkShaderFiles(glslang::TWorklist& Worklist) FreeFileData(const_cast(it->text[0])); } -int C_DECL main(int argc, char* argv[]) +int singleMain() { - // array of unique places to leave the shader names and infologs for the asynchronous compiles - std::vector> workItems; - ProcessArguments(workItems, argc, argv); - glslang::TWorklist workList; - std::for_each(workItems.begin(), workItems.end(), [&workList](std::unique_ptr& item) { + std::for_each(WorkItems.begin(), WorkItems.end(), [&workList](std::unique_ptr& item) { assert(item); workList.add(item.get()); }); @@ -1061,8 +1060,8 @@ int C_DECL main(int argc, char* argv[]) } if (Options & EOptionStdin) { - workItems.push_back(std::unique_ptr{new glslang::TWorkItem("stdin")}); - workList.add(workItems.back().get()); + WorkItems.push_back(std::unique_ptr{new glslang::TWorkItem("stdin")}); + workList.add(WorkItems.back().get()); } ProcessConfigFile(); @@ -1100,11 +1099,11 @@ int C_DECL main(int argc, char* argv[]) CompileShaders(workList); // Print out all the resulting infologs - for (size_t w = 0; w < workItems.size(); ++w) { - if (workItems[w]) { - if (printShaderNames || workItems[w]->results.size() > 0) - PutsIfNonEmpty(workItems[w]->name.c_str()); - PutsIfNonEmpty(workItems[w]->results.c_str()); + for (size_t w = 0; w < WorkItems.size(); ++w) { + if (WorkItems[w]) { + if (printShaderNames || WorkItems[w]->results.size() > 0) + PutsIfNonEmpty(WorkItems[w]->name.c_str()); + PutsIfNonEmpty(WorkItems[w]->results.c_str()); } } @@ -1119,6 +1118,25 @@ int C_DECL main(int argc, char* argv[]) return 0; } +int C_DECL main(int argc, char* argv[]) +{ + ProcessArguments(WorkItems, argc, argv); + + int ret = 0; + + // Loop over the entire init/finalize cycle to watch memory changes + const int iterations = 1; + if (iterations > 1) + glslang::OS_DumpMemoryCounters(); + for (int i = 0; i < iterations; ++i) { + ret = singleMain(); + if (iterations > 1) + glslang::OS_DumpMemoryCounters(); + } + + return ret; +} + // // Deduce the language from the filename. Files must end in one of the // following extensions: From 74bde98778affc95d37f9002cc8d43be898e792b Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Mon, 13 Nov 2017 22:19:21 -0700 Subject: [PATCH 6/8] Memory: Restore unused interfaces, in case other tools called them. --- OGLCompilersDLL/InitializeDll.cpp | 47 +++++++++++++++++++++++++++ OGLCompilersDLL/InitializeDll.h | 2 ++ glslang/OSDependent/Unix/ossource.cpp | 1 + 3 files changed, 50 insertions(+) diff --git a/OGLCompilersDLL/InitializeDll.cpp b/OGLCompilersDLL/InitializeDll.cpp index d4e1b9f5..abea9108 100644 --- a/OGLCompilersDLL/InitializeDll.cpp +++ b/OGLCompilersDLL/InitializeDll.cpp @@ -115,4 +115,51 @@ bool InitThread() return true; } +// Not necessary to call this: InitThread() is reentrant, and the need +// to do per thread tear down has been removed. +// +// This is kept, with memory management removed, to satisfy any exiting +// calls to it that rely on it. +bool DetachThread() +{ + bool success = true; + + if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) + return true; + + // + // Function is re-entrant and this thread may not have been initialized. + // + if (OS_GetTLSValue(ThreadInitializeIndex) != 0) { + if (!OS_SetTLSValue(ThreadInitializeIndex, (void *)0)) { + assert(0 && "DetachThread(): Unable to clear init flag."); + success = false; + } + } + + return success; +} + +// Not necessary to call this: InitProcess() is reentrant. +// +// This is kept, with memory management removed, to satisfy any exiting +// calls to it that rely on it. +// +// Users of glslang should call shFinalize() or glslang::FinalizeProcess() for +// process-scoped memory tear down. +bool DetachProcess() +{ + bool success = true; + + if (ThreadInitializeIndex == OS_INVALID_TLS_INDEX) + return true; + + success = DetachThread(); + + OS_FreeTLSIndex(ThreadInitializeIndex); + ThreadInitializeIndex = OS_INVALID_TLS_INDEX; + + return success; +} + } // end namespace glslang diff --git a/OGLCompilersDLL/InitializeDll.h b/OGLCompilersDLL/InitializeDll.h index 3f27ce9b..661cee4d 100644 --- a/OGLCompilersDLL/InitializeDll.h +++ b/OGLCompilersDLL/InitializeDll.h @@ -40,6 +40,8 @@ namespace glslang { bool InitProcess(); bool InitThread(); +bool DetachThread(); // not called from standalone, perhaps other tools rely on parts of it +bool DetachProcess(); // not called from standalone, perhaps other tools rely on parts of it } // end namespace glslang diff --git a/glslang/OSDependent/Unix/ossource.cpp b/glslang/OSDependent/Unix/ossource.cpp index 9c16dc40..24b77e16 100644 --- a/glslang/OSDependent/Unix/ossource.cpp +++ b/glslang/OSDependent/Unix/ossource.cpp @@ -56,6 +56,7 @@ namespace glslang { // static void DetachThreadLinux(void *) { + DetachThread(); } // From fc3e86472a757255b62df236c72aac553c68e59c Mon Sep 17 00:00:00 2001 From: LoopDawg Date: Mon, 13 Nov 2017 15:54:12 -0700 Subject: [PATCH 7/8] Implement OS_DumpMemoryCounters for Linux --- glslang/OSDependent/Unix/ossource.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/glslang/OSDependent/Unix/ossource.cpp b/glslang/OSDependent/Unix/ossource.cpp index 24b77e16..f59bbceb 100644 --- a/glslang/OSDependent/Unix/ossource.cpp +++ b/glslang/OSDependent/Unix/ossource.cpp @@ -43,6 +43,9 @@ #include #include #include +#include +#include +#include namespace glslang { @@ -184,8 +187,18 @@ void ReleaseGlobalLock() pthread_mutex_unlock(&gMutex); } +// #define DUMP_COUNTERS + void OS_DumpMemoryCounters() { +#ifdef DUMP_COUNTERS + struct rusage usage; + + if (getrusage(RUSAGE_SELF, &usage) == 0) + printf("Working set size: %ld\n", usage.ru_maxrss * 1024); +#else + printf("Recompile with DUMP_COUNTERS defined to see counters.\n"); +#endif } } // end namespace glslang From 5da8ead7036c06eaaa87402f276a7b81f7f7f7be Mon Sep 17 00:00:00 2001 From: John Kessenich Date: Tue, 14 Nov 2017 15:19:41 -0700 Subject: [PATCH 8/8] Memory: Remove redundant pool popAll(), which is potentially confusing. --- glslang/MachineIndependent/ShaderLang.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/glslang/MachineIndependent/ShaderLang.cpp b/glslang/MachineIndependent/ShaderLang.cpp index ba6e8c44..ec5327d9 100644 --- a/glslang/MachineIndependent/ShaderLang.cpp +++ b/glslang/MachineIndependent/ShaderLang.cpp @@ -1286,7 +1286,6 @@ int __fastcall ShFinalize() } if (PerProcessGPA != nullptr) { - PerProcessGPA->popAll(); delete PerProcessGPA; PerProcessGPA = nullptr; }