ECB in Entities.ForEach

  1. Is this (code below) how ECB supposed to be used in ForEach code gen jobs? Looks just a little awkward, I’m curious if it would be possible to do it without declaring a field.
  2. Is ECB disposed of after playback?
  3. Is World.GetOrCreateSystem<>() a cheap call?
  4. Oh! Forgot something important. What job index should I use in ecb.Destroy(inex, entity)?
public class DeathSystem : JobComponentSystem
{
    [ReadOnly] EntityCommandBuffer.Concurrent ECB;

    protected override JobHandle OnUpdate(JobHandle inputDependencies)
    {
        var ecb = ECB = World.GetOrCreateSystem<BeginSimulationEntityCommandBufferSystem>().CreateCommandBuffer().ToConcurrent();
        float deltaTime = Time.DeltaTime;
        return Entities
            .WithName("TestJob")
            .ForEach((in Health health, in Entity e) =>
             {
                 if (health <= 0)
                 {
                     ecb.DestroyEntity(1, e);
                 }
             })
            .Schedule(inputDependencies);
    }
}
1 Like

Nooo, You should store the EntityCommandBufferSystem as a field and use it to create the EntityCommandBuffer, the index should be a parameter (int nativeThreadIndex) and it should came before any in/ref parameters and the JobHandle should be passed to EntityCommandBuffer.AddJobHandleForProducer.

Example:

using Unity.Entities;
using Unity.Jobs;

public class LifeTimeSystem : JobComponentSystem
{
   EntityCommandBufferSystem EntityCommandBufferSystem;

   protected override void OnCreate()
   {
      EntityCommandBufferSystem = World.GetExistingSystem<EndSimulationEntityCommandBufferSystem>();
   }

   protected override JobHandle OnUpdate(JobHandle InputDeps)
   {
      float DeltaTime = Time.DeltaTime;
      var EntityCommandBuffer = EntityCommandBufferSystem.CreateCommandBuffer().ToConcurrent();
    
      var JobHandle = Entities.WithBurst().ForEach((Entity Entity, int nativeThreadIndex, ref LifeTime Cooldown) =>
      {
         Cooldown.Value -= DeltaTime;

         if (Cooldown.Value <= 0.0)
         {
            EntityCommandBuffer.DestroyEntity(nativeThreadIndex, Entity);
         }
      }).Schedule(InputDeps);

      EntityCommandBufferSystem.AddJobHandleForProducer(JobHandle);

      return JobHandle;
   }
}

EDIT: Maybe WithBurst isn’t necessary, because it uses burst by default

3 Likes

Thank you. Okay, interesting, nativeThreadIndex is a magic argument…

So the World.GetExistingSystem<MyECBSystem>() is an expensive call and that’s the reason it should be cached?

var ecb = World.GetExistingSystem<MyECBSystem>().CreateCommandBuffer().ToConcurrent(); I could swear this code didn’t work before so I had to create [ReadOnly] field because it told me to. Now it works and I’m just wondering if caching (5 extra lines of code) is necessary.

1 Like

World.GetExistingSystem iterates through all systems from the World and returns the first system where the type T is assignable from its type. So, I’d say it’s necessary.

EDIT: but only if you defined UNITY_DOTSPLAYER, otherwise it’s gonna use a look up table to get the system, so that’s something that you should try and test to know if it worths or not

3 Likes

I just profiled and 10+ calls to World.GetExistingSystem<MyECBSystem>() (unless optimized away by the compiler) didn’t make a dent in a performance of a system that just schedules one job. But one call per system surely doesn’t seem to make any visible difference.

So just my personal preference is to not use caching, for the sake of simplicity. I can always change it later, and I’ll check if it will get worse when I add more systems.

Thanks again.

1 Like

It’s not. Systems alredy cached in type dictionary

Dictionary<Type, ComponentSystemBase> m_SystemLookup = new Dictionary<Type, ComponentSystemBase>();
```, and GetExistingSystem not iterating systems it's just wrapper (also it has collections checks but only in editor) over Dictionary.TryGetValue wich practically approaches an O(1) operation.
1 Like

I mentioned it

if UNITY_DOTSPLAYER is defined
5245412--523841--upload_2019-12-4_12-8-40.png
otherwise
5245412--523844--upload_2019-12-4_12-9-12.png

1 Like

Note that you should use the entityInQueryIndex as the the ECB jobIndex parameter, not nativeThreadIndex. Using nativeThreadIndex can lead to slower ECB performance.

.WithBurst() only needs to be used if you want to set the Burst options.(Use .WithoutBurst() to disable Burst.)

https://docs.unity3d.com/Packages/com.unity.entities@0.2/manual/entities_job_foreach.html#using-entitiesforeach-with-an-entitycommandbuffer

4 Likes

Yes it’s what I’m talking about - in default build it will be second one - type lookup from dictionary. You define UNITY_DOTSPLAYER by yourself only if your build not using UnityEngine at all. (Or it will be setted from new BuildPipeline for pure DOTS builds in future, currently I didn’t see related things in build pipeline)