Improved performance in data visitors

[johang88]

PR Details

Description

Added checks for value type arrays in a few data visitors that does not act on primitive type items. This improves performance in the editor by not having to iterate through all the collection members unless needed.

Motivation and Context

Game studio can be quite slow when updating array asset members, this provides improved performance for arrays of value types as these do not have to be iterated.

Types of changes

• [ ] Docs change / refactoring / dependency upgrade
• [X ] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

All directly related tests seems to pass. Quite a few tests fail, but this seems to be the case even without this change.

• [ ] My change requires a change to the documentation.
• [ ] I have added tests to cover my changes.
• [ ] All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[Eideren]

Thanks ! Is there a particular reason why you decided to override functions from the types deriving from DataVisitorBase and AssetVisitorBase instead of those two types directly ? I barely looked at that namespace tbf so bare with me, from what I understood of this post you don't need to traverse arrays of structs because they wouldn't contain references to assets (that would have to be updated after refreshing assets), am I right in thinking that's what this PR is based upon ? If that's the case, in c# value types can still hold references, unmanaged structs don't though so you should filter based on that instead.

[johang88]

I figured that some implementations of the visitors might want to look at primitive types so that's why I changed the visitors that do not override VisitPrimitive. You are right about the value types, I will change it so that it only checks for primitive types, this will allow structs with asset references.

[johang88]

Only checking for primitive types now. Did notice that DefaultNodeBuilder has an additional list for checking native math types etc, perhaps this could be extended to a more generic implementation?

I have analyzed the code a bit more and found that the following places currently handles primitive types in some way: VisitPrimtive

• AssetMemberVisitorBase, abstracts class does not seem to be implemented anywhere

VisitArrayItem

• AssetMemberVisitorBase, abstracts class does not seem to be implemented anywhere
• AssetReferenceVistor
• ReloaderVisitorBase (and UnloadableObjectRemover, UnloadingVisitor), used for hot reloading assemblies. These can probably be a bit touchy but primitive types should be safe to ignore

Most of them can probably be optimized, but might be better to do that separately? The modifications I have currently done affect real time operations (asset update in editor) while these seems to be on stuff that is already kind of slow.

I will continue checking this out, will mark the PR as draft for now :)

[Eideren]

Typo in IsArrayOfPrimitveType, missing an i :) We should probably support all unmanaged types, not just primitive ones, i don't see any reason not to.