[Refactor] (MG_Util, MG_Test): retire FastSTL for ska::flat_hash_map, the table MobileGlues settled on

.gitmodules

@@ -7,9 +7,6 @@
 [submodule "3rdparty/SPIRV-Cross"]
 	path = 3rdparty/SPIRV-Cross
 	url = https://github.com/KhronosGroup/SPIRV-Cross.git
-[submodule "include/FastSTL"]
-	path = include/FastSTL
-	url = https://github.com/MobileGL-Dev/FastSTL.git
 [submodule "3rdparty/tracy"]
 	path = 3rdparty/tracy
 	url = https://github.com/wolfpld/tracy.git
@@ -34,3 +31,6 @@
 [submodule "3rdparty/asio"]
 	path = 3rdparty/asio
 	url = https://github.com/chriskohlhoff/asio.git
+[submodule "include/ska"]
+	path = include/ska
+	url = https://github.com/MobileGL-Dev/flat_hash_map.git

MobileGL/Includes.h

@@ -49,8 +49,8 @@
 #include <stacktrace>
 #endif
 
-// Include FastSTL
+// Include ska::flat_hash_map
-#include <FastSTL/UnorderedMap.h>
+#include <ska/flat_hash_map.hpp>
 
 // Include xxHash
 #include <xxhash.h>

MobileGL/MG_Test/SanityTest.cpp

@@ -33,7 +33,7 @@
 #include <MG_Util/ShaderTranspiler/ShaderCompiler.h>
 #include <MG_Util/ShaderTranspiler/ShaderSourceProcessor.h>
 #include <MG_Util/Debug/Log.h>
-#include <FastSTL/UnorderedMap.h>
+#include <MG_Util/Types.h>
 
 namespace {
     class DynamicParameterBackend final : public MobileGL::MG_Backend::BackendObject {
@@ -1998,16 +1998,20 @@ TEST(DirectGLESStateGuards, DefaultFramebufferBindGoesThroughShadow) {
     EXPECT_EQ(mocks.log.Count("BindFramebuffer:"), 3u);
 }
 
-// FastSTL::unordered_map::erase(iterator) regression coverage. The open-addressing
+// UnorderedMap::erase(iterator) contract coverage. Erase-while-iterating sweeps
-// iterator constructor snaps forward from a tombstoned slot to the successor, so
+// (pipeline/program cache eviction) depend on `it = map.erase(it)` naming the next
-// erase must NOT advance the rebuilt iterator again: the old double-advance skipped
+// live element exactly once: a sweep that skips entries leaks them, and one that
-// one live element per erase, and erasing the element in the highest occupied
+// runs off the end feeds garbage handles to vkDestroyPipeline (device crash on the
-// bucket pushed the returned index past bucket_count where it never compared equal
+// first mass eviction during world load - the failure FastSTL's double-advancing
-// to end() again - erase-while-iterating sweeps (pipeline/program cache eviction)
+// erase actually produced before it was fixed).
-// then ran off the bucket array and fed garbage handles to vkDestroyPipeline
+//
-// (device crash on first mass eviction during world load).
+// These pin the behaviour the call sites rely on, not one map's implementation, so
-TEST(FastSTLSanity, EraseWhileIteratingVisitsEveryElementExactlyOnce) {
+// they are written against MobileGL::UnorderedMap and survive changing what it
-    FastSTL::unordered_map<MobileGL::Uint64, MobileGL::Uint64> map;
+// names. Under ska::flat_hash_map the mechanism is different - erase backward-shifts
+// the rest of the probe cluster into the hole and hands back the same slot, which
+// now holds the shifted-in successor - but the observable contract is the same.
+TEST(UnorderedMapSanity, EraseWhileIteratingVisitsEveryElementExactlyOnce) {
+    MobileGL::UnorderedMap<MobileGL::Uint64, MobileGL::Uint64> map;
     constexpr MobileGL::Uint64 kCount = 1000;
     for (MobileGL::Uint64 key = 0; key < kCount; ++key) {
         map.emplace(key * 0x9e3779b97f4a7c15ull, key);
@@ -2024,8 +2028,8 @@ TEST(FastSTLSanity, EraseWhileIteratingVisitsEveryElementExactlyOnce) {
     EXPECT_EQ(map.size(), 0u);
 }
 
-TEST(FastSTLSanity, EraseReturnsTheSuccessorElement) {
+TEST(UnorderedMapSanity, EraseReturnsTheSuccessorElement) {
-    FastSTL::unordered_map<MobileGL::Uint32, MobileGL::Uint32> map;
+    MobileGL::UnorderedMap<MobileGL::Uint32, MobileGL::Uint32> map;
     for (MobileGL::Uint32 key = 1; key <= 64; ++key) {
         map.emplace(key, key);
     }
@@ -2048,10 +2052,16 @@ TEST(FastSTLSanity, EraseReturnsTheSuccessorElement) {
     EXPECT_EQ(map.size(), 64u - erased);
 }
 
-TEST(FastSTLSanity, ErasingTheOnlyElementReturnsEnd) {
+TEST(UnorderedMapSanity, ErasingTheOnlyElementReturnsEnd) {
-    FastSTL::unordered_map<MobileGL::Uint32, MobileGL::Uint32> map;
+    using Map = MobileGL::UnorderedMap<MobileGL::Uint32, MobileGL::Uint32>;
+    Map map;
     map.emplace(42u, 1u);
-    auto next = map.erase(map.begin());
+
+    // Spell the type: erase(iterator) hands back a proxy that is convertible to an
+    // iterator but is not one, because finding the next element is not free and the
+    // callers that discard the result should not pay for it. `auto next = ...` binds
+    // the proxy instead, and then nothing it is compared against compiles.
+    Map::iterator next = map.erase(map.begin());
     EXPECT_EQ(next, map.end());
     EXPECT_TRUE(map.empty());
 }

MobileGL/MG_Util/Types.h

@@ -55,9 +55,23 @@ namespace MobileGL {
     using SizeT = std::size_t;
     template <typename T, SizeT N>
     using Array = std::array<T, N>;
+    // ska::flat_hash_map, the same table MobileGlues settled on, at the same commit.
+    //
+    // Open addressing with robin-hood probing, so a rehash moves the elements: any
+    // insert, emplace, operator[], reserve or rehash invalidates every iterator,
+    // reference and pointer into the map. Erase does too, and less obviously -
+    // deletion shifts the rest of the probe cluster backwards, so erasing one key
+    // can move a DIFFERENT key's element. Where a mapped value's address has to
+    // outlive later mutation, the map holds a UniquePtr/SharedPtr and the pointee
+    // stays put; those sites say so where they are declared.
+    //
+    // Its value_type is pair<Key, T> with the key exposed mutably, so `it->first =`
+    // compiles and silently corrupts the table - the one sharp edge this map has
+    // that a node-based one does not. Note the Allocator default matches that
+    // value_type: pair<Key, T>, not pair<const Key, T>.
     template <typename Key, typename T, class Hash = std::hash<Key>, class KeyEqual = std::equal_to<Key>,
-              class Allocator = std::allocator<std::pair<const Key, T>>>
+              class Allocator = std::allocator<std::pair<Key, T>>>
-    using UnorderedMap = FastSTL::unordered_map<Key, T, Hash, KeyEqual, Allocator>;
+    using UnorderedMap = ska::flat_hash_map<Key, T, Hash, KeyEqual, Allocator>;
     template <typename T>
     inline constexpr std::remove_reference_t<T>&& Move(T&& t) noexcept {
         return static_cast<std::remove_reference_t<T>&&>(t);