MovieScene Schedule_PerAllocation is scheduling multiple Tasks on different Threads

Development Cinematics & Media
, ,
1

Hello!

I have a bug that I’m attempting to squash, and I may have uncovered unexpected behavior.

A couple years ago, I asked about this and this was the response:

Tasks scheduled through the Schedule_ api will schedule a single task that performs all the logic, whereas Fork_ tasks will schedule one task per allocation that matches the filter. We use Fork tasks for things like curve evaluation (where there are almost no dependencies, and we need to unblock downstream tasks asap).

I currently have a system that has 2 tasks in it:

  1. FGatherMyObjects
  2. FApplyOffsets

They’re defined something like this inside OnSchedulePersistentTasks

// Step 1: Get all my objects
    
FTaskID GatherTask = FEntityTaskBuilder()
.Read(BuiltInComponents->EvalTime)
.Read(BuiltInComponents->BoundObject)
.SetParams(FTaskParams(TEXT("Gather My Objects")).ForcePrePostTask())
//.SetDesiredThread(Linker->EntityManager.GetGatherThread()) // I noticed this was on a bunch of other Systems, and this does seem to force the same thread
.Schedule_PerAllocation<FGatherMyObjects>(&Linker->EntityManager, TaskScheduler, &MyArrayOfObjects);

// Make sure we're looking at a ComponentTransform and that it's set to absolute blend.
FEntityComponentFilter Filter;
Filter.All({ TracksComponents->ComponentTransform.PropertyTag, BuiltInComponents->Tags.AbsoluteBlend });
Filter.None({ BuiltInComponents->BlendChannelOutput });

// Step 2: Apply Offset

FTaskID ApplyTask = FEntityTaskBuilder()
.Read(BuiltInComponents->BoundObject)
.WriteOptional(BuiltInComponents->DoubleResult[0])
.WriteOptional(BuiltInComponents->DoubleResult[1])
.WriteOptional(BuiltInComponents->DoubleResult[2])
.WriteOptional(BuiltInComponents->DoubleResult[3])
.WriteOptional(BuiltInComponents->DoubleResult[4])
.WriteOptional(BuiltInComponents->DoubleResult[5])
// Must contain at least one double result
.FilterAny({ BuiltInComponents->DoubleResult[0], BuiltInComponents->DoubleResult[1], BuiltInComponents->DoubleResult[2],
    BuiltInComponents->DoubleResult[3], BuiltInComponents->DoubleResult[4], BuiltInComponents->DoubleResult[5] })
.CombineFilter(Filter)
.Fork_PerAllocation<FApplyOffsets>(&Linker->EntityManager, TaskScheduler, &MyArrayOfObjects);

TaskScheduler->AddPrerequisite(GatherTask, ApplyTask);

FGatherMyObjects looks something like this:

struct FGatherMyObjects
{
    TArray<UObject*>* MyArrayOfObjects;

    void PreTask() const
    {
       MyArrayOfObjects->Reset();
    }

    void ForEachAllocation(const FEntityAllocation* Allocation, TRead<FFrameTime> EvalTimes, TRead<UObject*> BoundObjects) const
    {
       const int32 NumAllocations = Allocation->Num();
       for (int32 Index = 0; Index < NumAllocations; ++Index)
       {
          // Evaluate the characterMeshOffset section at the current time to make sure it's enabled
          bool ShouldOffset = false;

          // -- Logic here to decide if we should offset this object --

          if (ShouldOffset)
          {
             MyArrayOfObjects->Add(BoundObjects[Index]);
          }
       }
       
       for (UObject* Object : *MyArrayOfObjects)
       {
          // Some logging here
          
          // This caused the Array iterator to tell me that the size changed while iterating!
       }
    }
};

I was getting odd behavior when applying the offset and noticed that the array didn’t seem to always be properly filled, despite logs telling me that it had been. I then added the for loop for debugging; I immediately hit an error saying that the size of the array had changed.

I set breakpoints and noticed that this task was being run in two threads (Background Worker #0 and Foreground Worker #0). The end result is that I’m adding to the array from two different threads.

So now to my question: Am I using Schedule incorrectly? What should I be doing differently? If this is indeed expected, how should I gather?

I think I can work around this by forcing this task to run on the Game/Gather Thread, but that seems antithetical to the way “Schedule_” is supposed to work.

Thanks!

-Nathaniel

[Attachment Removed]

2

Hi Nathaniel,

It looks like you’re passing a pointer to a shared array into the FGatherMyObjects task and then adding to it during. The iteration over allocation that happens there won’t necessarily run in one single loop on a single thread. This is because the allocations are stored based on th exact set of components an entity has. The .Read lines you have allow the task to pattern match over any entity that has the set of components you mention but the entities may have any number of varying other sets and components. Those entities with different sets of components will be stored in multiple locations and there will be a separate allocation loop for each set of entities with the exact same allocation of components. So effectively that means your array of objects will be getting modified on multiple task threads. You can fix this by forcing it to the game thread or a specific task thread as you note in a comment. Hope that helps.

-David

[Attachment Removed]

3

Hi Nathaniel,

Not quite. And I realize I missed a key detail that invalidates my explanation. First, I was correct in saying that the ForEachAllocation function called by the scheduler can be called multiple times from multiple threads. This is because each block of memory with the same number and type of components is run through it separately- this isn’t because of different code paths, just based on how the ECS stores entities and their components. HOWEVER, the schedule per allocation also guarantees that only one of those ForEachAllocation calls can happen at the same time. So to be clear the Gather Task ForEachAllocation can be run multiple times, which would explain why it wouldn’t necessarily be fully filled if you step through, but it shouldn’t cause multiple to be run at the same time. It’s possible we have a bug here, but can I ask if anything else (other than the subsequent Apply Task) modifies that array?

[Attachment Removed]

4

Are you able to look at the Threaded Callstacks to see what is modifying the array?

[Attachment Removed]

5

Hi Nathaniel, can you try setting bSerialTasks to true in TEntityTaskComponents::SchedulePerAllocation?

[Attachment Removed]

6

Hi Nathaniel,

I’ve now checked in that change after code review at our main CL 52467179. I think that should resolve your issue.

[Attachment Removed]

7

Sorry about that, the CL in UE5 Main is 52467188 but it’s basically what I suggested above.

I don’t suppose you have a way of reproducing the race condition in a Vanilla Unreal setup using a basic test project? Or have you had a chance to look at Threaded Callstacks view to get some callstacks as to how this is being modified in parallel? Thanks very much.

[Attachment Removed]

8

Hi Nathaniel,

Sending a long overdue followup on this. Is this issue still a problem for you? If so, do you have any new data- threaded callstacks, a repro in a vanilla unreal project we can take a look at? Thanks.

[Attachment Removed]

9

Thanks David! Just to confirm my understanding with the above information, the Schedule_ API will spin up only one task for that allocation, but if the task is scheduled in two different paths, it can be performed across multiple tasks. The Fork_ API will spin up one task for each allocation when this task is encountered, whether through one path or many paths. Is that correct?

I used MovieSceneSkeletalAnimationSystem as my reference when making this System, and in that system, FGatherSkeletalAnimations does not specify a Gather thread. FEvaluateSkeletalAnimations is set to be on the Game Thread, which makes sense. According to what you said above, FGatherSkeletalAnimations could be open to this race-condition as well, right? Or is there an implicit guarantee somewhere that means that this Gather task will only run once?

Thanks again David!

-Nathaniel

[Attachment Removed]

10

Oh interesting! Okay, that aligns with what Andrew Rodham was saying in the quote I pasted from here: [Content removed] . That’s what caused me to think this might be a bug, so perhaps that is the case after all.

To answer your question, only this task modifies the array. (The example is very close to what I have locally, with only the logic removed in the example).

I unfortunately don’t have the callstack handy any more as this race condition was tough to catch to begin with. However, I did see with the debugger that two FGatherMyObjects tasks were running at the same time: one on Background Worker #0 and one on Foreground Worker #0.

[Attachment Removed]

11

Hi David,

Unfortunately my repro for the race condition is no longer valid, so I can’t easily test that part. I did try setting bSerialTasks to true to see what would happen. In my repro case, Schedule_PerAllocation runs in multiple threads, but the task so far has not run in two threads simultaneously.

[Attachment Removed]

12

Hi David, I had a chance to test the race condition with bSerialTasks = true. Unfortunately, that did not solve the issue. The race condition still occurs.

I also tried to look at that CL, but it appears to be part of the Fortnite depot.

[Attachment Removed]

13

I haven’t had a chance to try it out in vanilla Unreal, unfortunately. Although in this case, as far as I’m aware, I don’t have any engine changes that would cause the task ordering to change. The allocations appear to have just happened to split the task graph in such a way that this task can run at the same time in two different threads. Perhaps there’s a way to give you access to our project for debugging this.

When I looked at the threaded callstacks before, it was the same task in two different threads that caused the array to be modified. In other words, line 22 was running in thread 1 while line 26-31 was running thread 2. This triggered the “array size changed during for loop” ensure (this is how I caught the race condition to begin with).

[Attachment Removed]