FlushAsyncLoading causes significant stall via UpdateAllPrimitiveSceneInfos

My team is still having an issue similar to what Christian reported here:

[Content removed]

We’re on 5.6 and seeing this behavior where ~750ms cycles of FMaterialUpdateContext and FlushRenderingCommands are repeatedly being called as seen here. There’s definitely some work to do on our end to avoid the LoadSynchronous call higher in the stack but I am wondering if there are changes we can pull to improve this?

[Image Removed]

[Attachment Removed]

Steps to Reproduce
Presumably a synchronous load of an object with a number of Niagara assets in a large level would repro this but I have not tried to set up a repro.

[Attachment Removed]

Hi Joe, and thank you for your question. It’s landed on the simulation queue, so I’ve just reassigned it to the Niagara team who will be able to help.

All the best

Geoff Stacey

Developer Relations

EPIC Games

[Attachment Removed]

Can you integrate CL 42740809 to see if that helps at all?

Thanks,

Stu

[Attachment Removed]

Unfortunately we had already pulled that change a resolution for some other issue so that code was already present when I saw this behavior.

[Attachment Removed]

Can you confirm if the systems in question have any static bool bindings on the renderers at all?

I’m assuming this is from updating the static permutations on the material, could confirm that inside NiagaraRendererProperties.cpp?

If it is from that, we’ve made some changes to this process in 5.7 and you may need to integrate the changes in this file.

Thanks,

Stu

[Attachment Removed]

I just confirmed that we do have some static bool bindings on some renderers though I haven’t dug deep enough to understand if that’s directly causing this behavior. If you let me know which changes you had in mind I can grab them and see if they fix this.

[Attachment Removed]

We just hit this as well (Hello Stu!). We are seeing 10s+ stalls during PIE sessions when we first use a Niagara effect,. For us, compiling the effect results in 140K primitive component render states being recreated. I trapped one of the cases and yes, it is because a Static Parameter is being changed. Callstack below. I added a stat marker to

UNiagaraSystem::CacheFromCompiledData() [Image Removed]

[UnrealEditor-Engine.dll] UPrimitiveComponent::CreateRenderState_Concurrent(FRegisterComponentContext *) PrimitiveComponent.cpp:652
[UnrealEditor-Engine.dll] UStaticMeshComponent::CreateRenderState_Concurrent(FRegisterComponentContext *) StaticMeshComponent.cpp:953
[Inlined] [UnrealEditor-Engine.dll] FComponentRecreateRenderStateContext::{dtor}() ActorComponent.cpp:451
[UnrealEditor-Engine.dll] DestructItems<FComponentRecreateRenderStateContext, int>(FComponentRecreateRenderStateContext *, int) MemoryOps.h:93
[Inlined] [UnrealEditor-Engine.dll] TArray<FComponentRecreateRenderStateContext, TSizedDefaultAllocator<32> >::Empty(int) Array.h:2281
[UnrealEditor-Engine.dll] FGlobalComponentRecreateRenderStateContext::~FGlobalComponentRecreateRenderStateContext() ActorComponent.cpp:531
[UnrealEditor-Engine.dll] FMaterialUpdateContext::~FMaterialUpdateContext() MaterialShared.cpp:4977
[UnrealEditor-Engine.dll] UMaterialInstance::UpdateStaticPermutation(const FStaticParameterSet &, FMaterialInstanceBasePropertyOverrides &, const bool, FMaterialUpdateContext *) MaterialInstance.cpp:4592
[UnrealEditor-Engine.dll] UMaterialInstance::UpdateStaticPermutation(const FStaticParameterSet &, FMaterialUpdateContext *) MaterialInstance.cpp:4643
[UnrealEditor-Niagara.dll] UNiagaraRendererProperties::UpdateMaterialParametersMIC(const FNiagaraRendererMaterialParameters &, TArrayView<UMaterialInterface *, int>, TArray<TObjectPtr<UMaterialInstanceConstant>, TSizedDefaultAllocator<32> > &) NiagaraRendererProperties.cpp:722
[UnrealEditor-Niagara.dll] UNiagaraRendererProperties::UpdateMaterialParametersMIC(const FNiagaraRendererMaterialParameters &, TObjectPtr<UMaterialInterface> &, TObjectPtr<UMaterialInstanceConstant> &) NiagaraRendererProperties.cpp:635
[Inlined] [UnrealEditor-Niagara.dll] UNiagaraRibbonRendererProperties::UpdateMICs() NiagaraRibbonRendererProperties.cpp:534
[UnrealEditor-Niagara.dll] UNiagaraRibbonRendererProperties::CacheFromCompiledData(const FNiagaraDataSetCompiledData *) NiagaraRibbonRendererProperties.cpp:407
[UnrealEditor-Niagara.dll] FVersionedNiagaraEmitterData::CacheFromCompiledData(const FNiagaraDataSetCompiledData *, const UNiagaraEmitter &) NiagaraEmitter.cpp:1644
[UnrealEditor-Niagara.dll] UNiagaraSystem::CacheFromCompiledData() NiagaraSystem.cpp:2475
[UnrealEditor-Niagara.dll] UNiagaraSystem::QueryCompileComplete(bool) NiagaraSystem.cpp:3426
[UnrealEditor-Niagara.dll] UNiagaraSystem::PollForCompilationComplete(bool) NiagaraSystem.cpp:3101
[Inlined] [UnrealEditor-Niagara.dll] PollSystemCompilations() NiagaraModule.cpp:576
[UnrealEditor-Niagara.dll] INiagaraModule::OnWorldTickStart(UWorld *, ELevelTick, float) NiagaraModule.cpp:588

[Attachment Removed]

It is worth noting that both of our teams are using an internal plugin that introduces new types that derive from UNiagaraRendererProperties. These new properties have behavior in their PostLoad that can potentially make modifications to the MaterialParameters object. After learning that Chris’ team was hitting this as well I did a bit more digging and noticed that all of the materials that are hitting this FMaterialUpdateContext within UMaterialInstance::UpdateStaticPermutation are of the type used by this internal plugin. I’ve reached out to the original author of this original plugin to take a look but if you have any advice about where it might be safer to make changes like this other than within the PostLoad I’d love to hear your recommendation.

As an example of what this is doing currently …

void UNiagaraBuiltInMaterialRibbonRendererProperties::PostLoad()
{
#if WITH_EDITOR
	// Make changes to the MaterialParameters to simplify setup for artists. Ex: Ensure various modules are present and ensure various texture samplers are using the right settings.
        ...
#endif //WITH_EDITOR
	Super::PostLoad();
}

[Attachment Removed]

Hey Chris, we meet again :slight_smile:

So I think you could test with making a FMaterialUpdateContext which has no flags and pass that into MIC->UpdateStaticPermutation. Niagara will automatically reregister the Niagara components post cache so we don’t need EOptions::RecreateRenderStates, and looking inside UpdateStaticPermutation it generates a context with only that flag (unless I’ve missed something).

The other option would be to pass in a FMaterialUpdateContext from the cache to the renderers so at least there’s a single sync vs one per renderer.

Let me know if that works out as I don’t have time to test it locally right now, but I’ll add it to my things to look at list :slight_smile:

Cheers,

Stu

[Attachment Removed]

Thanks Stu, yeah providing a MaterialUpdateContext without the RecreateRenderStates flag does fix it, so we’ll go with that assuming it won’t have any bad/worse side effects.

I still see about 370ms for the worst case which I’m going to dig into - at first glance I think this might be legit because we have so many objects that use the materials. Either way, this isn’t a deal breaker for PIE.

[Image Removed]

Thanks for the help,

Chris.

[Attachment Removed]

So I found one that still took almost 800ms which is getting a bit high. I think moving the MaterialUpdateContext into CacheFromCompiledData outside the emitter loop will be worthwhile because we typically have 4-5 emitters. That will touch a lot of files to pass the context around, so I want to make sure there are no issues before I proceed. Stu - do you foresee any problems deferring the MaterialUpdates to the end of UNiagaraSystem::CacheFromCompiledData?

[Attachment Removed]

There should be no problem with deferring the updates to the end of the cache. However I think there’s a bit of logic inside FMaterialUpdateContext we could be skipping here, I’m assming it’s going inside bUpdateStaticDrawLists and that is redundant in the case of Niagara. We are pretty much always dynamic draws and our components should already be invalidated as part of this. I’m not sure there’s a good way to avoid it currently however beyond adding a flag, could be worthwhile doing a quick test if you have the time? It will be less effort than having to pass that context around the cache chain :slight_smile:

There is a bigger question of why the assets haven’t been saved in a state with the MICs are up to date, this could mean a resave would ‘fix’ it or something is noodling with the flags in a way that the saved MICs are not the correct state.

If you find adding an extra flag works out I can get that into the engine after some negotation with the materials team :slight_smile:

Cheers,

Stu

[Attachment Removed]

Ok great, glad I held off - that sounds much better and easier! Will give that a try next week. Thanks again.

[Attachment Removed]

Did you get chance to test this out at all Chris?

Cheers,

Stu

[Attachment Removed]

Not yet! Will definitely get to it this week though :slight_smile:

[Attachment Removed]

I just tried this and it seems to work. Specifically I added a new flag SkipUpdateStaticDrawList to FMaterialUpdateContext::EOptions, and skipped everything in the destructor except ReloadNaniteFixedFunctionBins(). Can we skip that as well? Wondering if I should just make it a general “Do Nothing” flag. Doesn’t matter much - we’re down to worst case 40ms, and typically less than 1ms, now from the original 13s so I’ll take it :slight_smile:

[Attachment Removed]

Awesome, thanks for testing Chris. Pretty sure we can skip the Nanite part also, but I’ll confirm that with some other folks, when I have a CL I’ll pass it on also.

[Attachment Removed]

Barring any issues this went into UE5 Main in CL 51976471. I am not skipping quite as much as you are, I may revisit it at a later date.

Cheers,

Stu

[Attachment Removed]