#How to use ExclusiveEntityTransaction + EntityQuery?

1 messages · Page 1 of 1 (latest)

glacial mountain
#

If i want to call for example EntityManager.RemoveComponent<T>(EntityQuery) within an EET what's the best way to handle this? Currently this results in the following exception:```InvalidOperationException: Must be called from the main thread
Unity.Entities.EntityDataAccess.AssertMainThread () (at ./Library/PackageCache/com.unity.entities@1.0.11/Unity.Entities/EntityDataAccess.cs:370)
Unity.Entities.EntityManager.RemoveComponent (Unity.Entities.EntityQuery entityQuery, Unity.Entities.ComponentType componentType) (at ./Library/PackageCache/com.unity.entities@1.0.11/Unity.Entities/EntityManager.cs:2138)
Unity.Entities.EntityManager.RemoveComponent[T] (Unity.Entities.EntityQuery entityQuery) (at ./Library/PackageCache/com.unity.entities@1.0.11/Unity.Entities/EntityManager.cs:2262)
DefaultNamespace.CreateEntitiesInJob.Execute () (at Assets/Scripts/ProcessEntities.cs:112)
Unity.Jobs.IJobExtensions+JobStruct`1[T].Execute (T& data, System.IntPtr additionalPtr, System.IntPtr bufferRangePatchData, Unity.Jobs.LowLevel.Unsafe.JobRanges& ranges, System.Int32 jobIndex) (at <c2d036c16ca64e0eb93703a3b13e733a>:0)

glacial mountain
#

@tacit panther perhaps I should report this as a bug?

tacit panther
#

yeah i think so

glacial mountain
#

Also tagging @gray forge and @raven cargo in case they know something 👀

glacial mountain
#

@tacit panther @south marsh Reported as IN-53056 👀

raven cargo
#

I actually don't see AssertMainThread() called anywhere any more, including from EntityManager.RemoveComponent(EntityQuery). It's possible this has already been fixed?

#

I'll look into what happened here. But in general, passing an EntityQuery to a job is definitely not supported. Queries (and the code that processes them) currently make all sorts of assumptions that they're on the main thread. You'll note that ExclusiveEntityTransaction doesn't expose any of the bulk structural change operations that target an EntityQuery, and this may be (at least partially) the reason.

#

It's possible the operation may appear to work if we remove the assert, but may also introduce a horrifying race condition.

glacial mountain
#

Not being able to use EntityQuery is a significant limitation, the alternative would be to use NativeArray<Entity> overloads

glacial mountain
raven cargo
#

I agree that it's a huge limitation; the per-entity methods are definitely not a great substitute, for anything performance-sensitive.

#

ExclusiveEntityTransaction is literally just a pass-through wrapper that's calling methods on an EntityManager. But that doesn't mean that it's safe for threads to call any EntityManager method they want. The ones exposed by ExclusiveEntityTransaction have test coverage and are expected to work; anything else is at best 🤷‍♂️ and at worst 💣

glacial mountain
raven cargo
#

Yes, and perhaps it shouldn't! (though that would be a breaking API change at this point, and unfortunately those are getting more difficult to make).

#

Here's one concrete example of what could go wrong if we just remove these main-thread asserts today: every EntityQuery stores a cached list of all the chunks it matches, which lets us avoid walking every single matching archetype every want to perform an operation on the query. Nearly all query operations use this cached chunk list internally. Currently, the code that invalidates / repopulates this cache has absolutely zero thread-safety checks, because queries are only intended to be used from the main thread. Invalidating a query's cached chunk list on the main thread while a worker thread is iterating over the same list virtually guarantees some form of quiet use-after-free bug.
We've tried just adding a lock around the chunk list cache updates, but the performance hit was way larger than we're comfortable with. We're exploring a better approach, but it will take some time.

#

There are probably several other issues like that; that's just the example I'm most immediately familiar with.

glacial mountain
glacial mountain
raven cargo
#

I can all but guarantee we won't. But I'll probably go add a comment to the field clarifying that calling EntityManager methods from jobs besides the ones exposed through ExclusiveEntityTransaction is not supported, and will result in undefined behavior.

Anything_is_Possible_scaled.png
glacial mountain
glacial mountain
#

Tagging @gray forge asking to please add better EET support 😢 UnityChanDown

gray forge
#

Can you be more specific with your request? What would suffice for better in this context?

glacial mountain
# gray forge Can you be more specific with your request? What would suffice for better in th...

Well ideally EntityQuerys supported with EET, but other than that match the EntityManager methods with the ExclusiveEntityTransaction methods, acording to Cort all the apis not present in ExclusiveEntityTransaction could have undefined behaviour so it'd be nice to have most if not all of the apis present in EntityManager, for example SetArchetype is missing in ExclusiveEntityTransaction. Chances are there are other missing methods.

#

Not sure if my explanation makes sense @gray forge

glacial mountain
#

@raven cargo @gray forge I noticed that in 1.0.16 it's no longer possible to schedule an IJobEntity as part of an ExclusiveEntityTransaction (at least no matter what I tried). Is this a bug or intentional? If it's intentional will it ever be possible to use IJobEntity + EET in the future?

raven cargo
#

I wouldn't count on it. The core problem, as I think I summarized in one of these threads, is that this is a special case of "using a query on the main thread (in this case, to schedule a job to run on the chunks that match that query) while some other thread is using an EET to potentially change the chunks that match the query". That is fundamentally not safe, and I don't see a clear path to make it safe.

glacial mountain
glacial mountain
#

@raven cargo 👀

raven cargo
#

Thank you for the reminder, and apologies for the delay; I wanted to have a clearer plan before I replied. I've been talking to other DOTS teams within Unity to see what their expectations & use cases for EETs are, to get a better sense of what we ultimately want to consider safe, supported behavior. We came up with a list of conditions that must be true for safe usage of EntityQuery in an EET, and it looks like with a few extra checks we can most likely expose some subset of useful operations for iteration (GetArchetypeChunkArray, ToEntityArray, etc.). I don't know yet if a straight-up idiomatic foreach with an EET on an IJob would be possible. There's some higher-urgency stuff to take care of first, so we're probably at least a few months away from starting this work. But, this is better than a hard No, right? 🙂

glacial mountain