From 648e47aae9bb078fc95fdc8234ac78378e7030f9 Mon Sep 17 00:00:00 2001 From: Stephen Seo Date: Fri, 10 Nov 2017 13:19:50 +0900 Subject: [PATCH] Fix crash bug, improve Unit Test Crash bug from improperly implemented std::lock_guard for multi-threaded usage of forMatchingSignatures fixed. --- src/EC/Manager.hpp | 27 ++++--- src/test/ECTest.cpp | 170 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 161 insertions(+), 36 deletions(-) diff --git a/src/EC/Manager.hpp b/src/EC/Manager.hpp index 378ca7a..15ff27c 100644 --- a/src/EC/Manager.hpp +++ b/src/EC/Manager.hpp @@ -24,6 +24,10 @@ #include #include +#ifndef NDEBUG + #include +#endif + #include "Meta/Combine.hpp" #include "Meta/Matching.hpp" #include "Bitset.hpp" @@ -966,8 +970,6 @@ namespace EC return forMatchingFunctions.erase(index) == 1; } - public: - /*! \brief Call multiple functions with mulitple signatures on all living entities. @@ -1074,8 +1076,10 @@ namespace EC & std::get(entities[eid])) == signatureBitsets[i]) { - std::lock_guard{sigsMutexes[i]}; + std::lock_guard guard( + sigsMutexes[i]); multiMatchingEntities[i].push_back(eid); + } } } @@ -1101,19 +1105,20 @@ namespace EC ForMatchingSignatureHelper<> >; using Index = EC::Meta::IndexOf; + constexpr std::size_t index = Index{}; if(threadCount <= 1) { - for(auto iter = multiMatchingEntities[Index{}].begin(); - iter != multiMatchingEntities[Index{}].end(); ++iter) + for(auto iter = multiMatchingEntities[index].begin(); + iter != multiMatchingEntities[index].end(); ++iter) { Helper::call(*iter, *this, - std::get(funcTuple)); + std::get(funcTuple)); } } else { std::vector threads(threadCount); - std::size_t s = multiMatchingEntities[Index{}].size() + std::size_t s = multiMatchingEntities[index].size() / threadCount; for(std::size_t i = 0; i < threadCount; ++i) { @@ -1121,7 +1126,7 @@ namespace EC std::size_t end; if(i == threadCount - 1) { - end = multiMatchingEntities[Index{}].size(); + end = multiMatchingEntities[index].size(); } else { @@ -1131,10 +1136,10 @@ namespace EC [this, &multiMatchingEntities, &funcTuple] (std::size_t begin, std::size_t end) { - for(std::size_t i = begin; i < end; ++i) + for(std::size_t j = begin; j < end; ++j) { - Helper::call(multiMatchingEntities[Index{}][i], - *this, std::get(funcTuple)); + Helper::call(multiMatchingEntities[index][j], + *this, std::get(funcTuple)); } }, begin, end); } diff --git a/src/test/ECTest.cpp b/src/test/ECTest.cpp index 8100264..2cf3b11 100644 --- a/src/test/ECTest.cpp +++ b/src/test/ECTest.cpp @@ -4,6 +4,8 @@ #include #include #include +#include +#include #include #include @@ -586,22 +588,35 @@ TEST(EC, ForMatchingSignatures) { EC::Manager manager; - auto e = { + std::size_t e[] = { + manager.addEntity(), + manager.addEntity(), + manager.addEntity(), manager.addEntity(), manager.addEntity(), manager.addEntity(), manager.addEntity() }; + auto& first = e[0]; + auto& last = e[6]; + for(auto id : e) { - manager.addComponent(id); - manager.addComponent(id); - manager.addTag(id); + if(id != first && id != last) + { + manager.addComponent(id); + manager.addComponent(id); + manager.addTag(id); - auto& c1 = manager.getEntityData(id); - c1.vx = 0; - c1.vy = 0; + auto& c1 = manager.getEntityData(id); + c1.vx = 0; + c1.vy = 0; + } + else + { + manager.addComponent(id); + } } using namespace EC::Meta; @@ -630,28 +645,85 @@ TEST(EC, ForMatchingSignatures) for(auto id : e) { - EXPECT_EQ(2, manager.getEntityData(id).x); - EXPECT_EQ(2, manager.getEntityData(id).y); - EXPECT_EQ(1, manager.getEntityData(id).vx); - EXPECT_EQ(1, manager.getEntityData(id).vy); + if(id != first && id != last) + { + EXPECT_EQ(2, manager.getEntityData(id).x); + EXPECT_EQ(2, manager.getEntityData(id).y); + EXPECT_EQ(1, manager.getEntityData(id).vx); + EXPECT_EQ(1, manager.getEntityData(id).vy); + } + else + { + EXPECT_EQ(1, manager.getEntityData(id).x); + EXPECT_EQ(1, manager.getEntityData(id).y); + } } + { + std::unordered_map cx; + std::unordered_map cy; + std::unordered_map c0x; + std::unordered_map c0y; + std::unordered_map c1vx; + std::unordered_map c1vy; + std::mutex cxM; + std::mutex cyM; + std::mutex c0xM; + std::mutex c0yM; + std::mutex c1vxM; + std::mutex c1vyM; + manager.forMatchingSignatures< TypeList, TypeList > > ( std::make_tuple( - [] (std::size_t eid, C0& c) { - EXPECT_EQ(2, c.x); - EXPECT_EQ(2, c.y); - c.x = 5; - c.y = 7; + [&first, &last, &cx, &cy, &cxM, &cyM] (std::size_t eid, C0& c) { + if(eid != first && eid != last) + { + { + std::lock_guard guard(cxM); + cx.insert(std::make_pair(eid, c.x)); + } + { + std::lock_guard guard(cyM); + cy.insert(std::make_pair(eid, c.y)); + } + c.x = 5; + c.y = 7; + } + else + { + { + std::lock_guard guard(cxM); + cx.insert(std::make_pair(eid, c.x)); + } + { + std::lock_guard guard(cyM); + cy.insert(std::make_pair(eid, c.y)); + } + c.x = 11; + c.y = 13; + } }, - [] (std::size_t eid, C0& c0, C1& c1) { - EXPECT_EQ(5, c0.x); - EXPECT_EQ(7, c0.y); - EXPECT_EQ(1, c1.vx); - EXPECT_EQ(1, c1.vy); + [&c0x, &c0y, &c1vx, &c1vy, &c0xM, &c0yM, &c1vxM, &c1vyM] + (std::size_t eid, C0& c0, C1& c1) { + { + std::lock_guard guard(c0xM); + c0x.insert(std::make_pair(eid, c0.x)); + } + { + std::lock_guard guard(c0yM); + c0y.insert(std::make_pair(eid, c0.y)); + } + { + std::lock_guard guard(c1vxM); + c1vx.insert(std::make_pair(eid, c1.vx)); + } + { + std::lock_guard guard(c1vyM); + c1vy.insert(std::make_pair(eid, c1.vy)); + } c1.vx += c0.x; c1.vy += c0.y; @@ -661,13 +733,61 @@ TEST(EC, ForMatchingSignatures) }), 3 ); + + for(auto iter = cx.begin(); iter != cx.end(); ++iter) + { + if(iter->first != first && iter->first != last) + { + EXPECT_EQ(2, iter->second); + } + else + { + EXPECT_EQ(1, iter->second); + } + } + for(auto iter = cy.begin(); iter != cy.end(); ++iter) + { + if(iter->first != first && iter->first != last) + { + EXPECT_EQ(2, iter->second); + } + else + { + EXPECT_EQ(1, iter->second); + } + } + for(auto iter = c0x.begin(); iter != c0x.end(); ++iter) + { + EXPECT_EQ(5, iter->second); + } + for(auto iter = c0y.begin(); iter != c0y.end(); ++iter) + { + EXPECT_EQ(7, iter->second); + } + for(auto iter = c1vx.begin(); iter != c1vx.end(); ++iter) + { + EXPECT_EQ(1, iter->second); + } + for(auto iter = c1vy.begin(); iter != c1vy.end(); ++iter) + { + EXPECT_EQ(1, iter->second); + } + } for(auto eid : e) { - EXPECT_EQ(1, manager.getEntityData(eid).x); - EXPECT_EQ(2, manager.getEntityData(eid).y); - EXPECT_EQ(6, manager.getEntityData(eid).vx); - EXPECT_EQ(8, manager.getEntityData(eid).vy); + if(eid != first && eid != last) + { + EXPECT_EQ(1, manager.getEntityData(eid).x); + EXPECT_EQ(2, manager.getEntityData(eid).y); + EXPECT_EQ(6, manager.getEntityData(eid).vx); + EXPECT_EQ(8, manager.getEntityData(eid).vy); + } + else + { + EXPECT_EQ(11, manager.getEntityData(eid).x); + EXPECT_EQ(13, manager.getEntityData(eid).y); + } } }