Memory analysis and proposal

[johnkslang]

From looking at all the the issues/PRs labeled as Memory, and deciding what do actually do.

Analysis

Relevant History

• Glslang stopped using its original Win32 DLL model:

  • DllMain() originally called to dispatch DetachProcess() and DetachThread(),
  • This code stopped being called.

• Addition of TShader::preprocess():

  • this only fit the memory model if used as a substitute for TShader::parse()
  • no design for both: Externalized preprocessing was never part of the original memory design.

List of Existing Problems

• Final cleanup is not done by the exposed interfaces of FinalizeProcess() or ShFinalize()

  • 916

  • 928

  • 389

• No scheme to have both TShader::preprocess() and TShader::parse() in use together.

  • 766

  • 275

• Multiple independent uses of glslang within a process are cumbersome because, not knowing about each other, they each want to do one-time initialization and finalization.

• The TPragmaTable container itself can leak (the contents are properly pool managed)

  • 916

• Looking inside inUseList after executing destructor, before deallocating memory. Works, but technically not proper.

  • 705

• The original threadPoolAllocator is sometimes not freed up? Not yet clear which problem triggers this.

  • 916

Independent Improvements

• Move all the OS-dependent stuff to use C++11 features.

  • 826: how orthogonal is this to the above?

• Stop worrying about threads altogether in glslang, use an instancing model for the whole thing

  • 389 note, needs a different interface?

Proposed Plan

• [x] Fix the two existing interfaces glslang::FinalizeProcess() or ShFinalize() do to full clean up. The important thing to preserve is multiple independent uses get to share the one-time cost of the shared super-global symbol table.

• [x] Do the first one with a hidden mutexed reference count, such that multiple calls within a single process are okay.

• [x] Move TPragmaTable container to come from the pool, to avoid need for delete. Subset of #916.

• [x] Understand the issue if SetThreadPoolAllocator() is sometimes losing the original, and that needs to be preserved. (#916) Fix any underlying problems.

• [x] Fix the preprocess interface problem. Since this does not make an AST and cannot be linked, and is relatively new, there is probably a simple way of having it clean up it's memory either automatically or through an extra interface call. Don't yet have a specific design; but possibly just having two independent pools that both get deleted.

• [x] Work around the need to look inside inUseList after executing its destructor.

• [x] Move to using the C++11 tools instead of OS-specific tools. (#826)

• [ ] Consider an instancing model, but since this changes the interface, it is a more difficult thing to deal with.

[johnkslang]

It turns out the first step (finalize stuff) cannot be done until after proper pool tracking is done (4th in the list), much as in #916. I have a couple commits coming that first rationalize the interfaces a bit, and then fix the ownership/freeing issue with pools.

[johnkslang]

1147 is addressing the 1st and 4th items in the list.

[arcady-lunarg]

We've eliminated almost everything from the OSDependent directory now, in favor of standard C++11/C++17 mechanisms.