While investigating slow PIE startup, I’ve found that one of the issues is that `FPoseSearchDatabaseAsyncCacheTask::StartNewRequestIfNeeded` calculates the DDC key in a non-deterministic way, which in some cases means that every time you restart an editor, the expensive derived data calculations will be re-done. The output would then be saved into the shared DDC with a unique key, needlessly wasting storage.
The way the DDC key is calculated is by serializing the root UObject and all its transitive dependencies using a hashing archive.
In my case, several of the animation sequences (`UAnimSequence` class) in the dependency chain had one of the elements in their `UAnimSequenceBase::Notifies` array saved with zero `FAnimNotifyEvent::Guid`. On every load, the `FAnimNotifyEvent::PostSerialize` function would detect that and assign the new randomly-generated value to this guid. This would get propagated to the hasher, which would ensure that every time the DDC key of the motion matching pose search asset would be different.
Is there any reason why the `FAnimNotifyEvent::Guid` field is not marked as transient? Or at least with ExcludeFromHash meta? Looking through the code, I think there’s no reason for the guid to be non-transient, as it’s only used to refer to the structure while editing it, and there’s nothing that persists the guid references.
Hi, sorry for the delay getting back to you on this. I was out of office the last couple of days last week so that slowed things down.
I don’t see any reason why we can’t make the Guid property transient or ExcludeFromHash (that would likely be the preference since it avoids changing serialization for the anim sequence assets). I’ll get a change made for this which will go into 5.7.
Having said that, I’d be interested to get more info from you as to why you’re seeing the Guid being regenerated on every load. That property should be initialized when a notify is created (in SAnimNotifyTrack) and then serialized, so in theory it should be a consistent value. If you resave the sequences after loading them, does that give a consistent Guid or do you still see them being regenerated in PostSerialize?
The GUIDs are regenerated every load, because the serialized data has zeros - and it’s hard to say how they got saved with zeros. We have a small subset of sequences exhibiting this problem, resaving them fixes the issue for them. I’ve considered just resaving these problematic sequences - but it doesn’t guarantee that the problem doesn’t reappear again in future for some other assets.
Re. transient vs ExcludeFromHash - while the latter would solve the DDC key calculation issue, it would still mean that cook results for such sequences will be non-deterministic. We’re currently pushing hard to make our cooks deterministic. Is there a reason _not_ to keep them transient, and regenerate new values on every load, just to avoid the possibility of ever having some codepath that fails to initialize it for new structures?
Ok, so it sounds like the root of the problem is that somewhere a notify event is being created and the Guid isn’t being set. We shouldn’t allow that, but unfortunately, there’s nothing in the current API that enforces it. I did a quick pass and found one location where we’re potentially creating notify event and not setting the Guid. And if you have any custom animation modifies, etc, that might create them, you could easily run into the same problem.
The reluctance to make the property transient is just the risk associated with changing the serialized data. We would have to make sure that there isn’t any other code that is relying on the Guid being consistent between editor sessions - we take copies of the notifies in a few places and use the Guid to tell that the notify is the same, so if that ever became desynchronized, we would run into problems. The good news is that the property is within a WITH_EDITORONLY_DATA block, so it shouldn’t actually be affecting your cooked data.
Ok thanks makes sense - thanks!
Hi, just a heads up that I’ve added the ExcludeFromHash meta tag as we discussed previously. So this shouldn’t be an issue for you in the 5.7 release.