[wip] Test RID_Owner thread safety

.github/workflows/linux_builds.yml

@@ -23,18 +23,6 @@ jobs:
       fail-fast: false
       matrix:
         include:
-          - name: Editor w/ Mono (target=editor)
-            cache-name: linux-editor-mono
-            target: editor
-            sconsflags: module_mono_enabled=yes
-            bin: ./bin/godot.linuxbsd.editor.x86_64.mono
-            build-mono: true
-            tests: false # Disabled due freeze caused by mix Mono build and CI
-            doc-test: true
-            proj-conv: true
-            api-compat: true
-            artifact: true
-
           - name: Editor with doubles and GCC sanitizers (target=editor, tests=yes, dev_build=yes, scu_build=yes, precision=double, use_asan=yes, use_ubsan=yes, linker=gold)
             cache-name: linux-editor-double-sanitizers
             target: editor
@@ -71,23 +59,6 @@ jobs:
             # Skip 2GiB artifact speeding up action.
             artifact: false
 
-          - name: Template w/ Mono (target=template_release, tests=yes)
-            cache-name: linux-template-mono
-            target: template_release
-            sconsflags: module_mono_enabled=yes
-            bin: ./bin/godot.linuxbsd.template_release.x86_64.mono
-            build-mono: false
-            tests: true
-            artifact: true
-
-          - name: Minimal template (target=template_release, tests=yes, everything disabled)
-            cache-name: linux-template-minimal
-            target: template_release
-            sconsflags: modules_enabled_by_default=no disable_3d=yes disable_advanced_gui=yes deprecated=no minizip=no
-            bin: ./bin/godot.linuxbsd.template_release.x86_64
-            tests: true
-            artifact: true
-
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -159,24 +130,6 @@ jobs:
         run: |
           ./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform=linuxbsd
 
-      - name: Prepare artifact
-        if: matrix.artifact
-        run: |
-          strip bin/godot.*
-          chmod +x bin/godot.*
-
-      - name: Upload artifact
-        uses: ./.github/actions/upload-artifact
-        if: matrix.artifact
-        with:
-          name: ${{ matrix.cache-name }}
-
-      - name: Dump Godot API
-        uses: ./.github/actions/godot-api-dump
-        if: matrix.api-dump
-        with:
-          bin: ${{ matrix.bin }}
-
       - name: Unit tests
         if: matrix.tests
         run: |
@@ -189,31 +142,9 @@ jobs:
         run: |
           dotnet test modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests
 
-      # Check class reference
-      - name: Check for class reference updates
-        if: matrix.doc-test
-        run: |
-          echo "Running --doctool to see if this changes the public API without updating the documentation."
-          echo -e "If a diff is shown, it means that your code/doc changes are incomplete and you should update the class reference with --doctool.\n\n"
-          ${{ matrix.bin }} --doctool --headless 2>&1 > /dev/null || true
-          git diff --color --exit-code && ! git ls-files --others --exclude-standard | sed -e 's/^/New doc file missing in PR: /' | grep 'xml$'
-
-      # Check API backwards compatibility
-      - name: Check for GDExtension compatibility
-        if: matrix.api-compat
-        run: |
-          ./misc/scripts/validate_extension_api.sh "${{ matrix.bin }}"
-
       # Download and run the test project
       - name: Test Godot project
         uses: ./.github/actions/godot-project-test
         if: matrix.proj-test
         with:
           bin: ${{ matrix.bin }}
-
-      # Test the project converter
-      - name: Test project converter
-        uses: ./.github/actions/godot-converter-test
-        if: matrix.proj-conv
-        with:
-          bin: ${{ matrix.bin }}

.github/workflows/macos_builds.yml

@@ -24,14 +24,7 @@ jobs:
             cache-name: macos-editor
             target: editor
             tests: true
-            bin: ./bin/godot.macos.editor.universal
-
-          - name: Template (target=template_release, tests=yes)
-            cache-name: macos-template
-            target: template_release
-            tests: true
-            sconsflags: debug_symbols=no
-            bin: ./bin/godot.macos.template_release.universal
+            bin: ./bin/godot.macos.editor.arm64
 
     steps:
       - name: Checkout
@@ -52,14 +45,6 @@ jobs:
         run: |
           sh misc/scripts/install_vulkan_sdk_macos.sh
 
-      - name: Compilation (x86_64)
-        uses: ./.github/actions/godot-build
-        with:
-          sconsflags: ${{ env.SCONSFLAGS }} arch=x86_64
-          platform: macos
-          target: ${{ matrix.target }}
-          tests: ${{ matrix.tests }}
-
       - name: Compilation (arm64)
         uses: ./.github/actions/godot-build
         with:
@@ -74,18 +59,6 @@ jobs:
           cache-name: ${{ matrix.cache-name }}
         continue-on-error: true
 
-      - name: Prepare artifact
-        run: |
-          lipo -create ./bin/godot.macos.${{ matrix.target }}.x86_64 ./bin/godot.macos.${{ matrix.target }}.arm64 -output ./bin/godot.macos.${{ matrix.target }}.universal
-          rm ./bin/godot.macos.${{ matrix.target }}.x86_64 ./bin/godot.macos.${{ matrix.target }}.arm64
-          strip bin/godot.*
-          chmod +x bin/godot.*
-
-      - name: Upload artifact
-        uses: ./.github/actions/upload-artifact
-        with:
-          name: ${{ matrix.cache-name }}
-
       - name: Unit tests
         if: matrix.tests
         run: |

.github/workflows/runner.yml

@@ -6,53 +6,14 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  # First stage: Only static checks, fast and prevent expensive builds from running.
-
-  static-checks:
-    if: '!vars.DISABLE_GODOT_CI'
-    name: 📊 Static checks
-    uses: ./.github/workflows/static_checks.yml
-
-  # Second stage: Run all the builds and some of the tests.
-
-  android-build:
-    name: 🤖 Android
-    needs: static-checks
-    uses: ./.github/workflows/android_builds.yml
-
-  ios-build:
-    name: 🍏 iOS
-    needs: static-checks
-    uses: ./.github/workflows/ios_builds.yml
-
   linux-build:
     name: 🐧 Linux
-    needs: static-checks
     uses: ./.github/workflows/linux_builds.yml
 
   macos-build:
     name: 🍎 macOS
-    needs: static-checks
     uses: ./.github/workflows/macos_builds.yml
 
   windows-build:
     name: 🏁 Windows
-    needs: static-checks
     uses: ./.github/workflows/windows_builds.yml
-
-  web-build:
-    name: 🌐 Web
-    needs: static-checks
-    uses: ./.github/workflows/web_builds.yml
-
-  # Third stage: Run auxiliary tests using build artifacts from previous jobs.
-
-  # Can be turned off for PRs that intentionally break compat with godot-cpp,
-  # until both the upstream PR and the matching godot-cpp changes are merged.
-  godot-cpp-test:
-    name: 🪲 Godot CPP
-    # This can be changed to depend on another platform, if we decide to use it for
-    # godot-cpp instead. Make sure to move the .github/actions/godot-api-dump step
-    # appropriately.
-    needs: linux-build
-    uses: ./.github/workflows/godot_cpp_test.yml

.github/workflows/windows_builds.yml

@@ -32,21 +32,6 @@ jobs:
             bin: ./bin/godot.windows.editor.x86_64.exe
             artifact: true
 
-          - name: Editor w/ clang-cl (target=editor, tests=yes, use_llvm=yes)
-            cache-name: windows-editor-clang
-            target: editor
-            tests: true
-            sconsflags: debug_symbols=no windows_subsystem=console use_llvm=yes
-            bin: ./bin/godot.windows.editor.x86_64.llvm.exe
-
-          - name: Template (target=template_release, tests=yes)
-            cache-name: windows-template
-            target: template_release
-            tests: true
-            sconsflags: debug_symbols=no
-            bin: ./bin/godot.windows.template_release.x86_64.console.exe
-            artifact: true
-
     steps:
       - name: Checkout
         uses: actions/checkout@v4
@@ -93,17 +78,6 @@ jobs:
           cache-name: ${{ matrix.cache-name }}
         continue-on-error: true
 
-      - name: Prepare artifact
-        if: ${{ matrix.artifact }}
-        run: |
-          Remove-Item bin/* -Include *.exp,*.lib,*.pdb -Force
-
-      - name: Upload artifact
-        if: ${{ matrix.artifact }}
-        uses: ./.github/actions/upload-artifact
-        with:
-          name: ${{ matrix.cache-name }}
-
       - name: Unit tests
         if: matrix.tests
         run: |

tests/core/templates/test_rid.h

@@ -31,7 +31,10 @@
 #ifndef TEST_RID_H
 #define TEST_RID_H
 
+#include "core/os/thread.h"
+#include "core/templates/local_vector.h"
 #include "core/templates/rid.h"
+#include "core/templates/rid_owner.h"
 
 #include "tests/test_macros.h"
 
@@ -96,6 +99,115 @@ TEST_CASE("[RID] 'get_local_index'") {
 	CHECK(RID::from_uint64(4'294'967'295).get_local_index() == 4'294'967'295);
 	CHECK(RID::from_uint64(4'294'967'297).get_local_index() == 1);
 }
+
+TEST_CASE("[RID_Owner] Thread safety") {
+	static const size_t CACHE_LINE_BYTES = std::hardware_constructive_interference_size;
+	struct alignas(CACHE_LINE_BYTES) DataHolder {
+		uint32_t value = 0;
+	};
+
+	struct RID_OwnerTester {
+		RID_Owner<DataHolder, true> rid_owner;
+		TightLocalVector<Thread> threads;
+		SafeNumeric<uint32_t> next_thread_idx;
+		TightLocalVector<RID> rids;
+		// Using std::atomic directly since SafeNumeric doesn't support relaxed ordering.
+		std::atomic<uint32_t> sync[2] = {};
+		std::atomic<uint32_t> correct = 0;
+
+		// A latch that doesn't introduce memory ordering constraints, only compiler ones.
+		// The idea is not to cause any sync traffic that would make the code we want to test
+		// seem correct as a side effect.
+		void lockstep(uint32_t p_step) {
+			uint32_t buf_idx = p_step % 2;
+			uint32_t target = (p_step / 2 + 1) * threads.size();
+			std::atomic_signal_fence(std::memory_order_seq_cst);
+			sync[buf_idx].fetch_add(1, std::memory_order_relaxed);
+			do {
+				std::atomic_signal_fence(std::memory_order_seq_cst);
+			} while (sync[buf_idx].load(std::memory_order_relaxed) != target);
+			std::atomic_signal_fence(std::memory_order_seq_cst);
+		}
+
+		explicit RID_OwnerTester(bool p_chunk_for_all, bool p_chunks_preallocated) :
+				rid_owner(sizeof(DataHolder) * (p_chunk_for_all ? OS::get_singleton()->get_processor_count() : 1)) {
+			threads.resize(OS::get_singleton()->get_processor_count());
+			rids.resize(threads.size());
+			if (p_chunks_preallocated) {
+				LocalVector<RID> prealloc_rids;
+				for (uint32_t i = 0; i < (p_chunk_for_all ? 1 : threads.size()); i++) {
+					prealloc_rids.push_back(rid_owner.make_rid());
+				}
+				for (uint32_t i = 0; i < prealloc_rids.size(); i++) {
+					rid_owner.free(prealloc_rids[i]);
+				}
+			}
+		}
+
+		~RID_OwnerTester() {
+			for (uint32_t i = 0; i < threads.size(); i++) {
+				rid_owner.free(rids[i]);
+			}
+		}
+
+		void test() {
+			for (uint32_t i = 0; i < threads.size(); i++) {
+				threads[i].start(
+						[](void *p_data) {
+							RID_OwnerTester *rot = (RID_OwnerTester *)p_data;
+
+							// 1. Each thread gets a zero-based index.
+							uint32_t th_idx = rot->next_thread_idx.postincrement();
+
+							rot->lockstep(0);
+
+							// 2. Each thread makes a RID holding its index.
+							const DataHolder initial_data = { th_idx };
+							rot->rids[th_idx] = rot->rid_owner.make_rid(initial_data);
+
+							rot->lockstep(1);
+
+							// 3. Each thread verifies all the others.
+							uint32_t local_correct = 0;
+							for (uint32_t i = 0; i < rot->threads.size(); i++) {
+								DataHolder *data = rot->rid_owner.get_or_null(rot->rids[i]);
+								if (data->value == i) {
+									local_correct++;
+								}
+							}
+
+							rot->lockstep(2);
+
+							rot->correct.fetch_add(local_correct, std::memory_order_acq_rel);
+						},
+						this);
+			}
+
+			for (uint32_t i = 0; i < threads.size(); i++) {
+				threads[i].wait_to_finish();
+			}
+
+			CHECK_EQ(correct.load(), threads.size() * threads.size());
+		}
+	};
+
+	SUBCASE("All items in one chunk, pre-allocated") {
+		RID_OwnerTester tester(true, true);
+		tester.test();
+	}
+	SUBCASE("All items in one chunk, NOT pre-allocated") {
+		RID_OwnerTester tester(true, false);
+		tester.test();
+	}
+	SUBCASE("One item per chunk, pre-allocated") {
+		RID_OwnerTester tester(false, true);
+		tester.test();
+	}
+	SUBCASE("One item per chunk, NOT pre-allocated") {
+		RID_OwnerTester tester(false, false);
+		tester.test();
+	}
+}
 } // namespace TestRID
 
 #endif // TEST_RID_H