Entt: is the entt::registry thread-safe?

[wenqzheng]

[skypjack]

It depends on what your goal is.
It doesn't make use of any facility so, for example, you cannot create components of the same type concurrently from multiple threads (unless you, as an user, put a guard on this ofc) but you can do it for different component types. Similarly, you can update different pools from different threads but it's not a good idea to do that concurrently for the same type.
All in all, the registry isn't inherently thread safe, mainly because in the vast majority of cases you can easily dispatch operations to multiple threads without using any facility when it comes to working with ecs stuff.

[wenqzheng]

Thanks. So I need a guard when creating entities concurrently from multiple threads.

[skypjack]

This or pre-assign a bunch of entities to a thread and let it be (see the range create for that), yes.
While component creation in different pools is inherently thread safe for obvious reasons, entity creation is not and should be guarded user side if you want to do that.
The fact is that guarding all cases no matter what can quickly become a perf penalty. Therefore, this is one of those things that should be in charge to users from my point of view (where I'm one of the users). They are the only one that know when, how and where to spend their cpu cycles correctly.

[nsubiron]

One thing I'd like to point out is that a const registry is not generally thread-safe, this is an assumption sometimes taken for granted (for instance STL containers offer this guarantee). The vector of pools is mutable and is modified by const methods. You can still work around it an ensure in your use case is not modified by preparing the pools you want to use though. That allows concurrent access to the registry without need for locking, as long as the registry is kept const.
IMO this would be a nice guarantee to offer, but "assure" method would need to fail (throw?) on const registries which changes the behaviour.

[wenqzheng]

I got it, many thanks.

[skypjack]

a const registry is not generally thread-safe [...] The vector of pools is mutable and is modified by const methods.

This is a tricky point @nsubiron
The advantage of having a list of pools that is dynamically created is obvious.
It makes possible to use a component type from literally everywhere without having to list all them in advance.

Most of the times we can reply to an API call without creating a pool though.
Imagine something like registry.empty<T>(). We don't really need to create the pool for T to know if it's empty. This is just convenient, since it's likely that you'll going to use that pool anyway sooner or later.

On the other side, there are things like group or view creation that do require to create pools in order to be consistent and const-friendly.
Let's suppose to invoke the const overload for view<T>. If the pool didn't exist and we didn't create it, the view would receive a null pointer that is never updated. This means that you can also store aside the view but it will keep returning no entities nor components, no matter if you create instances of T in the meantime.

In other terms, you'd move around the _problem_ you've right now rather than solving it.

[nsubiron]

In my case, I modified the assure method so asking for a view on a const registry throws an exception if the pool is missing, so you need to call "prepare" in advance on the components you plan to use. As you said, this is much less flexible since you have to declare in advance (at runtime though) which components you plan to use, but this way I make sure a const registry cannot be modified thus concurrent access is fine. It's a trade-off.

I'm not saying you should change the current behaviour, just wanted to point this out :slightly_smiling_face:

[Kerndog73]

A thread-safe const registry makes a lot of sense. Perhaps this behaviour could be enabled with a config macro?

[skypjack]

A thread-safe const registry makes a lot of sense. Perhaps this behaviour could be enabled with a config macro?

@Kerndog73 @nsubiron this could be an option actually. However, in this case, I'd trigger a compile-time error rather than a runtime exception or such. Could it work?
I think all is needed for that is to make the mutable keyword opt-in actually.

[skypjack]

I tried to make it opt-in/out but I discovered that either it's not that easy to do or that I should not try to do this kind of things at midnight. :smile:

[Kerndog73]

I had a go at this. Seems to work from my very limited testing.

diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp
index c5b765a5..b08310c1 100644
--- a/src/entt/entity/registry.hpp
+++ b/src/entt/entity/registry.hpp
@@ -185,7 +185,27 @@ class basic_registry {
     };

     template<typename Component>
-    const pool_handler<Component> & assure() const {
+    const pool_handler<Component> & find_pool() const {
+        static_assert(std::is_same_v<Component, std::decay_t<Component>>);
+
+        if constexpr(has_type_index_v<Component>) {
+            const auto index = type_index<Component>::value();
+            ENTT_ASSERT(index < pools.size());
+            auto &&pdata = pools[index];
+            ENTT_ASSERT(pdata.pool);
+            return static_cast<pool_handler<Component> &>(*pools[index].pool);
+        } else {
+            auto id_equal = [id = type_info<Component>::id()](const auto &pdata) {
+              return id == pdata.type_id;
+            };
+            auto it = std::find_if(pools.begin(), pools.end(), id_equal);
+            ENTT_ASSERT(it != pools.end());
+            return static_cast<pool_handler<Component> &>(*it->pool);
+        }
+    }
+
+    template<typename Component>
+    pool_handler<Component> & find_or_create_pool() {
         static_assert(std::is_same_v<Component, std::decay_t<Component>>);

         if constexpr(has_type_index_v<Component>) {
@@ -223,9 +243,18 @@ class basic_registry {
         }
     }

+    template<typename Component>
+    const pool_handler<Component> & assure() const {
+        #ifdef ENTT_THREADSAFE_CONST
+        return find_pool<Component>();
+        #else
+        return const_cast<basic_registry<Entity> &>(*this).find_or_create_pool<Component>();
+        #endif
+    }
+
     template<typename Component>
     pool_handler<Component> & assure() {
-        return const_cast<pool_handler<Component> &>(std::as_const(*this).template assure<Component>());
+        return find_or_create_pool<Component>();
     }

 public:
@@ -1779,7 +1808,7 @@ public:

 private:
     std::vector<group_data> groups{};
-    mutable std::vector<pool_data> pools{};
+    std::vector<pool_data> pools{};
     std::vector<entity_type> entities{};
     std::vector<variable_data> vars{};
     entity_type destroyed{null};

I swapped mutable for const_cast. Not sure if that really matters. Either one subverts the type system. Subverting the type system a little (mutable) and subverting the type system a lot (const_cast) is still subverting the type system.

[skypjack]

I swapped mutable for const_cast. Not sure if that really matters.

Well, technically speaking, a mutable on a data member clearly states that you're going to modify it in a const member.
The other way around isn't clear unless you go looking each and every member function to see if there is a const_cast somewhere.
So, imho that matters quite a lot in terms of cod maintainability.

[skypjack]

@Kerndog73 I must admit you triggered an idea though.
Probably I found a way to make it work without having to const_cast the assure function.

I was experimenting with it right now but it's 1AM, so I think we will know tomorrow if this works. :)

My only concern is that it won't be possible to create views and groups from a const registry if it doesn't contain the pools you need already.

[skypjack]

Confirmed. It _works_ apparently. I'm pushing it on experimental today or tomorrow, when also the doc is updated. :+1:
@nsubiron you won't have anymore to redefine assure. It will be enough to pass ENTT_THREAD_SAFE_CONST from now on. :wink:

[kix-mcotton]

So I was having some threading issues on my project and stumbled into this thread. My project has multiple independent simulations each running on their own thread. I figured it would be safe to have multiple registries for each of these simulations since they're completely isolated but I noticed that in const pool_handler<Component> & assure() const there is a static function variable static std::size_t index{pools.size()};
Obviously this isn't thread safe since each registry will be modifying it. Has this been changed recently? I can probably hack around it for now but it has me worried. Should it be safe to have multiple registries on their own threads provided they're only accessed from the one thread?

[Innokentiy-Alaytsev]

@kix-mcotton What version of EnTT are you using? There was an issue #449 related to the use of static variables. AFAIK, it should be resolved in v3.4.0.

[kix-mcotton]

Ah I'm using 3.3.2. I'll look at updating to 3.4.0. Cheers!

[blockspacer]

Regarding multiple independent simulations each running on their own thread and use of static variables please see https://github.com/skypjack/entt/issues/562 (you may want to define ENTT_MAYBE_ATOMIC=1 to use multiple independent simulations/dispatchers/etc. each running on their own thread)

[skypjack]

I'm about to push a _true const registry_ that returns empty views if the pools don't exist.
The only drawback is that a view with a broken pointer to a pool won't get it updated in any case. All we can do with such a view is to throw it away and to generate a new one the next tick. This is also the suggested use for views, so it seems consistent to me.
I've also added an operator bool to the views to know if they are properly initialized or not. Any thought or feedback?

[skypjack]

Personally, I think that this is a huge improvement overall.
I've also updated views so that they can be used as usual even if _broken_.
The codebase is a little trickier to maintain, because views now check if they are valid or not before returning or executing an each. Fortunately, in terms of performance it doesn't add anything at least (a pointer comparison).