NativeStream + IJobChunk

[ebon crane]

Currently to be able to write to a NativeStream.Writer in an IJobChunk requires using either PatchMinMaxRange or DisableParallelForRestriction, it seems to me like this could be fixed by patching NativeStream to use the unfilteredChunkIndex as the default aproach for writing.

[dusky knot]

cc: @quasi bison

[quasi bison]

Hello again, I'm working my way down my thread notifications now that I'm back from vacation. This one sounds interesting; can you give a more specific example of what you're trying to do, what fails, and your proposed solution? I know IJobChunk very well but am not intimately familiar with NativeStream.

[ebon crane]

Yeah so lets take the following code ```
EntityQuery someQuery = default;
var count = someQuery.CalculateChunkCountWithoutFiltering();
var nativeStream = new NativeStream(count, Allocator.TempJob);

        new MyJobChunk
        {
            writer = nativeStream.AsWriter()
        }.ScheduleParallel(someQuery, state.Dependency);
        
        
        [BurstCompile]
        public struct MyJobChunk : IJobChunk
        {
            public NativeStream.Writer writer;
        
            public void Execute(in ArchetypeChunk chunk, int unfilteredChunkIndex, bool useEnabledMask, in v128 chunkEnabledMask)
            {
                writer.PatchMinMaxRange(unfilteredChunkIndex);
                writer.BeginForEachIndex(unfilteredChunkIndex);
                writer.EndForEachIndex();
            }
        }```

If we remove the PatchMinMaxRange then I won't be able to write to the NativeStream, that's because it validates that the BeginForEachIndex(int foreachIndex) is within "range" for that parallel job

I think this does get patched correctly for IJobFor, but for IJobChunk it doesn't patch anything at all

So my proposed solution would be for IJobChunk to patch the NativeStream using the unfilteredChunkIndex, that way using PatchMinMaxRange/DisableParallelForRestriction wouldn't be necessary

Was my explanation clear? @quasi bison

[quasi bison]

I think so, yes. I vaguely recall some discussion a long way back about range patching in IJobChunk and what the default range should be (if any). The original pre-DOTS-1.0 IJobChunk and IJobEntityBatchWithIndex 🪦 both assumed that any native containers passed into the job had one element per eneity, so they patched the min/max range to each chunk's range of entity indices within the query, but the new IJobChunk doesn't have enough context to do that on its own, so it doesn't patch the min/max range by default.

I'm not sure that patching the range to the current chunk index is a reasonable default, though. Couldn't I just as reasonably be passing a NativeStream.Writer that expects one element per entity, not one element per chunk?

[ebon crane]

Yeah that's true, through currently it doesn't handle either case, so as long as it handles either the one element per entity or one per chunk I'd be satisfied

That or update the PatchMinMaxRange docs

[quasi bison]

I believe the conclusion at the time was that we can't really make any reasonable assumptions about how an IJobChunk is going to write to a container that's passed into the job, the way you can with per-entity jobs like IJobEntity or generic for-loop replacements like IJobFor. IJobChunk is sort of an ambiguous middle-ground; sometimes it's acting like a loop over chunks, and other times it's acting like a loop over entities. IJobEntity is already patching per-entity ranges where relevant; let me see what breaks if I change the default to "patch to unfiltered chunk index". Maybe that's a more reasonable default than "no patching at all, all container writes fail".

[quasi bison]

Yeah those docs aren't too great are they

[ebon crane]

"For internal use only."

[quasi bison]

Okay, we'll either fix the docs or add a non-empty default writable range to IJobChunk. Or maybe both. Thanks for the suggestion!