System lookups, options & burst compilation

[xVergilx]

Preivously in 0.51 I was using SystemBase with custom native containers inside. E.g. NativeOctree.

These containers were accessed inside other ISystems via SystemBase managed reference, dependency was combined to ensure safety, and then containers were used inside related job.

The question, how to properly store and access these types of containers?
ISystem no longer can contain managed reference types. Can store SystemHandle & resolve it via
state.WorldUnmanaged.GetUnsafeSystemRef<T>(), but there’s a big “no you’re doing something wrong” tooltip.

So, how to do it properly, and what are the options?

What I was thinking is something similar to ECB.Singleton binding of system fields to the IComponentData.
But, it requires an unsafe collection, and I’m not sure it will work. Plus, all safety would be lost.

In theory I could:

• Strip safety of the collection & make an unsafe version of it;
• Add it to the ISystem;
• “Register” a singleton entity with IComponentData that will store reference to the unsafe version of the collection;
• Access it similarly to the ECB.Singleton via SystemAPI;
• This should be BurstCompile-able for both lookup system & reading / writing system fetch
  (since systems no longer interact with each other directly);

But I’m not sure whether it would be safe to read / write it across multiple systems.
If that collection is only accessed via SystemAPI.GetSingletonRW() it should ensure that dependencies are combined / correct automatically, right?

Ideally, I’d like to do the following:

1. Bind persistent collection to system somehow;
2. Access it for Read / Writes in other systems in async way from jobs;
3. With at least some kind of safety;
4. (Optionally) No manual dependency management;

Any suggestions?

[DreamingImLatios]

I believe what you want is possible with singletons, but someone who actually understands that mess will have to explain it to you.

This is what I use: Latios-Framework-Documentation/Core/Collection and Managed Struct Components.md at main · Dreaming381/Latios-Framework-Documentation · GitHub
You can attach collection components to system entities, blackboard entities, or whatever other entities you want. And JobHandles are managed fully automatically be either merging into or reading from SystemState.Dependency behind the scenes. Works perfectly in Burst-compiled ISystem and you can pass the containers inside the collection components into jobs. And all the safety is there with the safe native collections.

[xVergilx]

@DreamingImLatios

I believe what you want is possible with singletons, but someone who actually understands that mess will have to explain it to you.

Yeah, that’s what I’ve been trying to figure out.

ECBSystem pending buffers are fetched and changed on the main thread.
I’d want to be able to access data from the job, like LookupEntity (from query) → ComponentLookup → Pointer → actual data.

CollectionComponent is cool concept, but it seems like have the same issue as just managing singletons.
That is - it needs to be fetched from main thread in order for safety to work properly. And they’re “managed”, so e.g. OnUpdate won’t be burst compiled either. Ideally, I’d want to burst compile the scheduling part as well.

Regarding Singletons / unsafe collections inside component.
If component type is added to the system, it should combine dependency based on read / write, right?

So why not just put unsafe collection pointer inside the IComponentData and let Entities manage dependencies instead on component access?

If pointer / collection is not shared elsewhere, shouldn’t it be pretty safe to do?
What other edge cases there might be other than pointers being shared to other components or changed by other logic?

[DreamingImLatios]

@xVergilx

That is - it needs to be fetched from main thread in order for safety to work properly.

This is always true, because you can’t use nested NativeContainers in jobs.

@xVergilx

nd they’re “managed”, so e.g. OnUpdate won’t be burst compiled either.

This is not true at all. This attribute ain’t there for show: https://github.com/Dreaming381/lsss-wip/blob/67f162c6135fc8fce1ecae3146a104b91bca131a/Assets/_Code/SubSystems/Gameplay/BulletVsWallSystem.cs#L29

@xVergilx

So why not just put unsafe collection pointer inside the IComponentData and let Entities manage dependencies instead on component access?

Because the job system doesn’t know what the pointer points to, and doesn’t know how to patch safety on the AtomicSafetyHandle.

@xVergilx

If pointer / collection is not shared elsewhere, shouldn’t it be pretty safe to do?

Yes, but the safety system doesn’t know that.

@xVergilx

What other edge cases there might be other than pointers being shared to other components or changed by other logic?

Parallel writers don’t get the thread index injected.

[xVergilx]

@DreamingImLatios

This is always true, because you can’t use nested NativeContainers in jobs.

Docs needs to be updated, but, that’s good to know.

Alright, makes sense. Basically, one “block” of memory would work if used sequentially, but not in parallel.
Currently I’ve managed to:

• Re-create most of the lookup data as native containers + unsafe counterparts;
• Attached to the system in following way:
  System → NativeContainer → Singleton → IComponentData + UnsafePtr (to the data storage);
• Grab SingletonEntity + ComponentLookup on the main thread;
• In job, access ComponentLookup via SingletonEntity;
• Grab actual [unsafe] data storage via ComponentData Ptr;
• Perform operations;
  TL;DR: Blobs, but mutable.

This is all theoretical compiled code, but I haven’t run anything properly and now I know why nobody does this.

Pros - it should BurstCompile and automatically manage dependencies.
Con - its absurdly hard to write & read cognitively and, as a result - error prone. Writing native containers is pure pain, especially at high quantity. Plus the need for two variants - one NativeContainer (for the system to keep pointer to the unsafe data storage to cleanup) + actual UnsafeData container. More data types == more code == more issues.

So in the end, its probably going to implode and burn down. I guess I’ll try on monday.
If nothing else works, got one extra hack instead of NativeContainers - DynamicBuffers as intermediate layer / storage for the pointer / data. Still super hacky, but DynamicHashmap works wonders. Main con - its always RW, due to DynamicBuffer being always RW.

In any case, Entities needs a better collection ↔ Entity relation.

[elliotc-unity]

Why don’t you do what we do, which is:

• Put NativeContainer on singleton
• Get singleton on main thread
• Get container out of singleton on main thread
• Schedule job against container
• Robert’s your father’s brother

?

[xVergilx]

@elliotc-unity

Why don’t you do what we do, which is:

• Put NativeContainer on singleton
• Get singleton on main thread
• Get container out of singleton on main thread
• Schedule job against container
• Robert’s your father’s brother

?

Well, that’s what I try to do. Except previous version data is not a NativeContainer.
Usually its a struct with a bunch of NativeContainers that aren’t physically allocated as one data reference pointer.

So now it has to be converted to NativeContainer first. That’s 8 x 2 containers.
x2 because can’t grab NativeContainers from entities in a job.

On topic of NativeContainers

Then there’s an inconsistent and internal gated allocation related API.
UnsafeList.Create exists.
UnsafeParallelSet.Create - does not.
Neither do UnsafeParallelMultiHashMap<TKey, TValue>.Create or UnsafeParallelHashMap<TKey, TValue>.Create.

Internally unsafe collections can be allocated with AllocatorHandle.
Which is not accessable in user code.

So, extension methods has to be written first.
Thankfully MallocTracked / FreeTracked now exists, so hopefully less random crashes.

Reading / Writing component directly on main thread == stalling previous jobs.
Which is why entity + IComponentData bypass.
Then systems with logic has to be rewritten to grab singletons.
And its ~50 systems ~ a lot of code.

Whole thing sucks majorly. Attaching collections to entities shouldn’t be this hard.
Sequential (non-parallel) reads / writes from different systems to the same data block shouldn’t be this hard.

Worst part - I don’t even know whether it will work, or just hard crash in the end.
Or get a random deadlock because of forgotten RW somewhere.

Judging by ECBSystem @ 1.0.11 state - nobody actually knows how to do this properly either.
A random pointer is thrown for pending buffers and that’s about as good solution as none.

It really feels like Collections are written to be a separate package from Entities.
And well, Entities are separate from Collections. There’s no inbetween.

[elliotc-unity]

I think your situation is slightly different from the ECB system one. For ECB systems, we really do want to wait for all the jobs that are currently writing to ECB’s registered to the current ECB system, because when we play back, we need them to be done, and we really are about to play back, and playback happens on the main thread. So I think it’s as optimal as can be from that perspective, and also because the systems recording ECB’s use GetSingleton without RW, their recording jobs can run in parallel.

(ECB systems would also totally work if m_pendingBuffers was a nativelist instead of an unsafelist, I just never changed it back, because it didn’t hurt anything how it was.)

(Structural changes are still bad, I’m just saying the singleton scheme isn’t causing problems in this particular case.)

Reading / Writing component directly on main thread == stalling previous jobs.

I agree it seems like this would happen, but it isn’t actually what happens with singletons, because we hacked it to not do that. It’s very confusing, but GetSingletonRW does not actually complete the jobs from the systems accessing that singleton that came before it, because we tried that, and our stuff was too slow if we did that.

So you don’t actually need the bypass, and if you do like I said, you can schedule a chain of jobs against the same container without any sync points.

Moreover, then you can keep your multiple containers on the same singleton, and you don’t have to combine them all or anything.

This is still obviously a hack, because singletons themselves are a hack, but we couldn’t come up with a better way before the 1.0 deadline, and it does at least allow you to chain jobs reasonably with a reasonable amount of safety, if you can tolerate how confusing and irritating it is. We’re working on various things to try to reduce this badness in the future.

[xVergilx]

@elliotc-unity

I agree it seems like this would happen, but it isn’t actually what happens with singletons, because we hacked it to not do that. It’s very confusing, but GetSingletonRW does not actually complete the jobs from the systems accessing that singleton that came before it, because we tried that, and our stuff was too slow if we did that.

So you don’t actually need the bypass, and if you do like I said, you can schedule a chain of jobs against the same container without any sync points.

Moreover, then you can keep your multiple containers on the same singleton, and you don’t have to combine them all or anything.

That’s the point I’ve missed completely. Thank you.

I’ll try putting NativeContainers directly.
Previously containers inside IComponentData wasn’t supported, so I thought this is still the case.
SingletonRW hack is definitely useful to grab those references.

[xVergilx]

@elliotc-unity

So you don’t actually need the bypass, and if you do like I said, you can schedule a chain of jobs against the same container without any sync points.

GetSingletonRW doesn’t combine dependencies if different systems write to the native container.
How to address this case?
Store JobHandle inside the struct itself or there’s a better way to chain them?

E.g.

      public void OnUpdate(ref SystemState state) {
          ...

         var spawnProjectileRequests = _spawnProjectileRequestsQuery.GetSingletonRW<SpawnProjectileRequests>().ValueRW;
    
         new SpawnWeaponProjectileJob
         {
            AimLookup = lookup,
            SpawnRequests = spawnProjectileRequests
         }.Schedule();

         ...
      }

Will cause:

InvalidOperationException: The previously scheduled job SpawnWeaponProjectileJob writes to the Unity.Collections.NativeList`1[Projectiles.ProjectileSpawnRequest] SpawnWeaponProjectileJob.JobData.SpawnRequests.Requests. You are trying to schedule a new job SetupRequestsJob, which writes to the same Unity.Collections.NativeList`1[Projectiles.ProjectileSpawnRequest] (via SetupRequestsJob.SpawnRequests.Requests). To guarantee safety, you must include SpawnWeaponProjectileJob as a dependency of the newly scheduled job.

When attempted to be read / written from a different system.

Manual points to EntityManager.CompleteDependencyBeforeRW.
Thing is, I don’t want to force complete jobs. I want to chain them one after another without main thread stalls.

[elliotc-unity]

What kind of jobs are SpawnWeaponProjectileJob and SetupRequestsJob? If they’re not both IJobEntity, they will not automatically hook themselves up with state.Dependency when you call Schedule with no arguments, and so yeah there’ll be a safety error. If you make both jobs depend on their respective systems’ state.Dependency, and then assign their handles back to state.Dependency when you schedule them, and you do that every time you schedule a job against that nativelist from a system that accesses that singleton, I would think you would be in business.

If not, I’d like to see what’s going on at the schedule site of SetupRequestsJob. I agree that force completing should not be necessary here, because yeah chaining is required for speed.

[xVergilx]

@elliotc-unity

What kind of jobs are SpawnWeaponProjectileJob and SetupRequestsJob? If they’re not both IJobEntity, they will not automatically hook themselves up with state.Dependency when you call Schedule with no arguments, and so yeah there’ll be a safety error. If you make both jobs depend on their respective systems’ state.Dependency, and then assign their handles back to state.Dependency when you schedule them, and you do that every time you schedule a job against that nativelist from a system that accesses that singleton, I would think you would be in business.

If not, I’d like to see what’s going on at the schedule site of SetupRequestsJob. I agree that force completing should not be necessary here, because yeah chaining is required for speed.

Its a simple IJob with ComponentLookups to fetch & update random access data.

Since there’s no way to tell what job needs, I’ve ended up adding Dependency field to the “LookupReference” struct, and manual dependency management works.

Still awkward manual dependency management but, at least it burst compiles and [partially] decoupled from the system.

Overall, safe RW access look like this:

ref var lookupRef = ref _lookupQuery.GetSingletonRW<TLookup>().ValueRW;
// Minimum RO
state.Dependency = JobHandle.CombineDependencies(state.Dependency, lookupRef.Dependency);
     
state.Dependency = new SomeJob
{
   Lookup = lookupRef.Lookup
}.Schedule(state.Dependency);
     
// For RW
lookupRef.AddWriteDependency(state.Dependency);

Also, yeah, empty IJob.Schedule() vs IJobEntity.Schedule() is a separate codegen gotcha.

[elliotc-unity]

I don’t think you should have to put the jobhandle in a component, since the fact that both systems are registered as writing to the same singleton should mean that their jobs get combined into each other’s state.Dependency.

If you do the state.Dependency = new job.schedule(state.Dependency) thing in both places, but you don’t do the manual dependency management with the addwritedependency business, and both systems do GetSingletonRW on the same singleton type, does it still throw?

[xVergilx]

@elliotc-unity

If you do the state.Dependency = new job.schedule(state.Dependency) thing in both places, but you don’t do the manual dependency management with the addwritedependency business, and both systems do GetSingletonRW on the same singleton type, does it still throw?

Yep, it does throw (e.g. if dependency read taken out, or RW taken out when RW is required).
I’m pretty sure _query.GetSingletonRW doesn’t codegen anything dependency related to the singleton at all.
Maybe its due to its hack nature.

Second system is IJobEntity system, generated code shows it adds state.Dependency automatically.

state.Dependency = __ScheduleViaJobChunkExtension_0(new SpawnWeaponProjectileJob { AimLookup = lookup, SpawnRequests = spawnProjectileRequests }, __TypeHandle.__Weapons_SpawnWeaponProjectileJob_WithDefaultQuery_JobEntityTypeHandle.DefaultQuery, state.Dependency, ref state, false);

Plus, there are more jobs that could potentially write to the same container (through GetSingletonRW) which fails without dependency management.

Previously I was using SystemBase.Dependency directly to combine them and updated Dependency via custom method. Same approach, different JobHandle location. No big deal.

[xVergilx]

Got one more question.
Shouldn’t this be safe to read before write happens, but I guess not anymore or this case wasn’t covered previously?

The following lookup case looks like this:

If RO job isn’t added as dependency, safety freaks out:

InvalidOperationException: The previously scheduled job ProcessExplosion_SetupCollisionTestsJob reads from the Octree.NativeOctree`1[HitsGathering.Targetable] ProcessExplosion_SetupCollisionTestsJob.JobData.Lookup. You are trying to schedule a new job GatherDynamicTargetable_BulkInsertJob, which writes to the same Octree.NativeOctree`1[HitsGathering.Targetable] (via GatherDynamicTargetable_BulkInsertJob.Lookup). To guarantee safety, you must include ProcessExplosion_SetupCollisionTestsJob as a dependency of the newly scheduled job.

Correct solution is to move read system (group in this case) after PostTransformationGroup, but I’m still curious.

Theoretically read may somehow happen after write even if first system RO scheduling first before second RW system?

Edit: Its probably not safe because job system cannot assume time period before read and write happens.
Even if group is moved to be later on (RW first, RO later) RO still requires to be added as dependency. Weird.

[elliotc-unity]

Could you paste the schedule lines from both systems? Is the first schedule being assigned back to state.Dependency, and is the second schedule depending on state.Dependency?

[xVergilx]

@elliotc-unity

Could you paste the schedule lines from both systems? Is the first schedule being assigned back to state.Dependency, and is the second schedule depending on state.Dependency?

First one is a SystemBase (ProcessExplosionSystem [RO]):

ref var lookupRef = ref _targetableLookupQuery.GetSingletonRW<TargetableLookup>().ValueRW;
ref var markupLookupRef = ref _markupLookupsQuery.GetSingletonRW<ColliderMarkupLookups>().ValueRW;
    
 Dependency = Dependency.CombineWith(lookupRef.Dependency);
          
new ProcessExplosion_SetupCollisionTestsJob
         {
            Lookup = lookupRef.Lookup,
    
            RayCmds = rayCmds,
            QueryParameters = queryParameters,
            MaxExplosionHits = MaxExplosionHits
}.Schedule();


    ...

    var deps = Dependency;
  
   // Will trigger safety if removed, basically combines lookupRef.Dependency + deps via JobHandle.CombineDependencies
   lookupRef.AddDependency(deps);

Generated code for it looks like this:

ref var markupLookupRef = ref _markupLookupsQuery.GetSingletonRW<ColliderMarkupLookups>().ValueRW;
Dependency = Dependency.CombineWith(lookupRef.Dependency);
Dependency = Dependency.CombineWith(markupLookupRef.Dependency);  
this.CheckedStateRef.Dependency = __ScheduleViaJobChunkExtension_0(new ProcessExplosion_SetupCollisionTestsJob { Lookup = lookupRef.Lookup, RayCmds = rayCmds, QueryParameters = queryParameters, MaxExplosionHits = MaxExplosionHits }, __TypeHandle.__Damage_ProcessExplosion_SetupCollisionTestsJob_WithDefaultQuery_JobEntityTypeHandle.DefaultQuery, this.CheckedStateRef.Dependency, ref this.CheckedStateRef, false);

So dependencies are included.

GatherDynamicTargetableSystem [RW] looks like this:

ref var lookupRef = ref _lookupQuery.GetSingletonRW<TargetableLookup>().ValueRW;
state.Dependency = state.Dependency.CombineWith(lookupRef.Dependency);

state.Dependency = new GatherDynamicTargetable_BulkInsertJob
                            {
                               Buffer = buffer.AsDeferredJobArray(),
                               Lookup = lookupRef.Lookup
                            }.Schedule(state.Dependency);
 
lookupRef.AddDependency(state.Dependency);

Which generates pretty much identical code:

ref var lookupRef = ref _lookupQuery.GetSingletonRW<TargetableLookup>().ValueRW;
state.Dependency = state.Dependency.CombineWith(lookupRef.Dependency);
state.Dependency = new GatherDynamicTargetable_BulkInsertJob
            {
                Buffer = buffer.AsDeferredJobArray(),
                Lookup = lookupRef.Lookup
            }.Schedule(state.Dependency);

lookupRef.AddDependency(state.Dependency);

Overall it works, but I have no clue why it requires RO job to be part of the chain if it goes RW → RO.
RO → RW makes at least some sense why would it trigger safety (in theory).

My best guess is to safeguard against jobs which run for too long & persist to the beginning of the next frame.
I might be wrong though, and it could just be false positive.