Prevent concurrent write access to unbuffered components

diff --git a/source/game/system.h b/source/game/system.h
index d4ba13e9cdf39789b8a7fe9fa3f9f31f732228fe..2882e8ceb7c1ae570c68aa4a9fc06a2a8bafef51 100644 (file)
--- a/source/game/system.h
+++ b/source/game/system.h
@@ -22,9 +22,10 @@ public:
                READ_OLD = 1,
                READ_FRESH = 3,
                WRITE = 4,
+               UNBUFFERED = 32,
                UPDATE = READ_OLD | WRITE,
                CHAINED_UPDATE = READ_FRESH | WRITE,
-               DATA_MASK = 7,
+               DATA_MASK = 39,
                RUN_BEFORE = 8,
                RUN_AFTER = 16,
                ORDER_MASK = 24
@@ -96,6 +97,8 @@ protected:
 template<typename T>
 inline void System::declare_dependency(DependencyFlags flags)
 {
+       if(flags&UNBUFFERED)
+               throw std::invalid_argument("System::declare_dependency");
        if((flags&DATA_MASK) && !std::is_base_of_v<Component, T>)
                throw std::invalid_argument("System::declare_dependency");
        if((flags&ORDER_MASK) && !std::is_base_of_v<System, T>)
@@ -111,23 +114,28 @@ inline void System::declare_dependency(DependencyFlags flags)
 
        Dependency &dep = (i!=dependencies.end() ? *i : dependencies.emplace_back(type));
        dep.flags = flags;
-       if constexpr(requires(T &c) { typename T::Data; c.prepare_tick(); c.commit_tick(); })
+       if constexpr(std::is_base_of_v<Component, T>)
        {
+               if constexpr(requires(T &c) { typename T::Data; c.prepare_tick(); c.commit_tick(); })
+               {
 #ifdef DEBUG
-               dep.unblock = +[](DependencyFlags f){
-                       if(f&READ_OLD) AccessGuard::get_instance().unblock<AccessGuard::Read<typename T::Data>>();
-                       if(f&WRITE) AccessGuard::get_instance().unblock<AccessGuard::Write<typename T::Data>>();
-               };
-               dep.block = +[](DependencyFlags f){
-                       if(f&READ_OLD) AccessGuard::get_instance().block<AccessGuard::Read<typename T::Data>>();
-                       if(f&WRITE) AccessGuard::get_instance().block<AccessGuard::Write<typename T::Data>>();
-               };
+                       dep.unblock = +[](DependencyFlags f){
+                               if(f&READ_OLD) AccessGuard::get_instance().unblock<AccessGuard::Read<typename T::Data>>();
+                               if(f&WRITE) AccessGuard::get_instance().unblock<AccessGuard::Write<typename T::Data>>();
+                       };
+                       dep.block = +[](DependencyFlags f){
+                               if(f&READ_OLD) AccessGuard::get_instance().block<AccessGuard::Read<typename T::Data>>();
+                               if(f&WRITE) AccessGuard::get_instance().block<AccessGuard::Write<typename T::Data>>();
+                       };
 #endif
-               if(flags&WRITE)
-               {
-                       dep.prepare = +[](Stage &s){ s.iterate_objects<T>([](T &c){ c.prepare_tick(); }); };
-                       dep.commit = +[](Stage &s){ s.iterate_objects<T>([](T &c){ c.commit_tick(); }); };
+                       if((flags&(WRITE|UNBUFFERED))==WRITE)
+                       {
+                               dep.prepare = +[](Stage &s){ s.iterate_objects<T>([](T &c){ c.prepare_tick(); }); };
+                               dep.commit = +[](Stage &s){ s.iterate_objects<T>([](T &c){ c.commit_tick(); }); };
+                       }
                }
+               else
+                       dep.flags = static_cast<DependencyFlags>(dep.flags|UNBUFFERED);
        }
 }
 

diff --git a/source/game/systemscheduler.cpp b/source/game/systemscheduler.cpp
index 40a951eaa2fe2db6f23011e5e957a5b525592b82..1cc78a4330276d0c25b589beb0f33379dd27e2a9 100644 (file)
--- a/source/game/systemscheduler.cpp
+++ b/source/game/systemscheduler.cpp
@@ -90,14 +90,18 @@ int SystemScheduler::get_data_order(const GraphNode &node1, const GraphNode &nod
                        else
                                continue;
 
+                       bool unbuffered = (flags1|flags2)&System::UNBUFFERED;
+                       flags1 &= ~System::UNBUFFERED;
+                       flags2 &= ~System::UNBUFFERED;
+
                        int order = 0;
                        // Read-only access to fresh data comes after any writes
                        if(flags1==System::READ_FRESH && (flags2&System::WRITE))
                                order = 1;
                        else if(flags2==System::READ_FRESH && (flags1&System::WRITE))
                                order = -1;
-                       // Other read-only access situations are not ordered
-                       else if(!(flags1&System::WRITE) || !(flags2&System::WRITE))
+                       // Reads can happen concurrently to other access with buffering
+                       else if(!(flags1&flags2&System::WRITE) && !unbuffered)
                                order = 0;
                        // Chained updates always come after non-chained writes
                        else if(flags1==System::CHAINED_UPDATE && (flags2==System::WRITE || flags2==System::UPDATE))

diff --git a/tests/scheduler.cpp b/tests/scheduler.cpp
index 7b34ebf2d0f986b4a7de56105fe2d224cbb7ab62..24bbe1bfc5043bf0c00acf84d2fc53538f95a4a3 100644 (file)
--- a/tests/scheduler.cpp
+++ b/tests/scheduler.cpp
@@ -15,7 +15,7 @@ public:
 private:
        void unrelated_components();
        void chained_update();
-       void parallel_reads();
+       void parallel_access();
        void ambiguous_data_order();
 };
 
@@ -35,9 +35,12 @@ struct Env
        { }
 };
 
-struct A: Game::Component { };
-struct B: Game::Component { };
-struct C: Game::Component { };
+struct AData { };
+struct A: Game::BufferedComponent<AData> { };
+struct BData { };
+struct B: Game::BufferedComponent<BData> { };
+struct CData { };
+struct C: Game::BufferedComponent<CData> { };
 
 template<typename T, Game::System::DependencyFlags F>
 struct Dep
@@ -75,7 +78,7 @@ SchedulerTests::SchedulerTests()
 {
        add(&SchedulerTests::unrelated_components, "Unrelated components");
        add(&SchedulerTests::chained_update, "Chained update");
-       add(&SchedulerTests::parallel_reads, "Parallel reads");
+       add(&SchedulerTests::parallel_access, "Parallel access");
        add(&SchedulerTests::ambiguous_data_order, "Ambiguous data order").expect_throw<Game::scheduling_error>();
 }
 
@@ -120,11 +123,11 @@ void SchedulerTests::chained_update()
        EXPECT_EQUAL(graph[2].prerequisites, 3);
 }
 
-void SchedulerTests::parallel_reads()
+void SchedulerTests::parallel_access()
 {
        Env env;
 
-       auto &sys1 = env.stage.add_system<Sys<Dep<A, READ_OLD>>>();
+       auto &sys1 = env.stage.add_system<Sys<Dep<A, UPDATE>>>();
        auto &sys2 = env.stage.add_system<Sys<Dep<A, READ_OLD>, Dep<B, UPDATE>>>();
        auto &sys3 = env.stage.add_system<Sys<Dep<A, READ_OLD>, Dep<C, UPDATE>>>();
        auto &sys4 = env.stage.add_system<Sys<Dep<B, READ_FRESH>, Dep<C, READ_FRESH>>>();