Issue/#133 inspector lag

[niggo1243]

Performance improvement v1 by fetching all relevant properties and attributes on "OnEnable" and not every OnInspectorGUI call.

There is still room for improvement by using delegates or checking validation and visibility every x seconds and fetching the info per object and not for all (although the unity api makes it difficult) but its a noticeable start.

Tested on the Test Scene but peer review would be appreciated!

Thanks for the awesome project!

[dbrizov]

Thanks, I will test it. BTW, I am no longer supporting v1, but thanks for the update there as well

[niggo1243]

No Problem,
with v1 I meant this perf improvement/ issue fix, since the memory part can be optimized lateron aswell (sorry for the confusion). The fix is based on the latest master commit of your (NaughtyAttributes) v2.

[dbrizov]

#133 for reference

[snorrsi]

@dbrizov is this going to merged soon ?

[oprel]

I have been using this branch daily for the last couple of months and it works great! Really sped up the editor and almost no bugs to be found (The only thing I noticed is weird issues with having more than one ValidateInput tag in the same class).

Just want to say I recommend merging this into main!

[niggo1243]

@oprel
Thanks!
I also found some small bugs (animator param not updating until inspector reselected for example) and fixed them in my fork master branch but the validateinput one is new to me.
Can you give me some reproduction steps so I can fix it in my fork.

[oprel]

@niggo1243
No, no, thank you for writing this!

I was away from work for a bit, so I'm sorry for the late response. Here's an example script to reproduce the validator issue:

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
using NaughtyAttributes;

public class exampleScriptableObject : ScriptableObject
{
    [ValidateInput("validatorOne","One is Invalid!")]
    public int fieldOne;

    [ValidateInput("validatorTwo","Two is Invalid!")]
    public int fieldTwo;

    public bool validatorOne(){
        return false;
    }

    public bool validatorTwo(){
        return true;
    }
}

Which results in the following:

[niggo1243]

@oprel
I have checked my recent commits and could not reproduce it:

Do you have my latest fork commit/ release?
If you are using it via the manifest.json, try to delete the package cache AND the lock section/ packages-lock.json file in the Unity Packages folder and restart Unity.

Let me know, if it still occurs on the newest version.

[oprel]

@niggo1243 Ah, I'm terribly sorry. I thought I was updated to the most recent commit, but after checking it seems like I was actually behind. Downloading your latest commits made the problem go away instantly. Thank you very much for the quick response!

Once again, highly recommend merging this into main!

[snorrsi]

@niggo1243 It seems your approach is still causing a lot of GC Allocation even when nothing changes in UI.
I looked into this slowness at some time ago myself and again last weekend.
I think it's possible to fix this slowness in much simpler way than your are doing, the main thing is caching the reflection, not really needed to create to create NaughtyProperty (which required changes in many files which might be one of the reason why the owner of this project has not still merged this pull request)

Unity uses some cache in it's display such as those listed here https://github.com/Unity-Technologies/UnityCsReference/blob/2020.3/Editor/Mono/ScriptAttributeGUI/ScriptAttributeUtility.cs

[niggo1243]

@snorrsi
Yes, the general issue was caching the reflection calls to retrieve the attributes.
And in order to access those (multiple) attribute objects, I created the NaughtyProperty wrapper class that stores those objects per field. This, of course needed a lot of changes and im not sure, if there is any simpler way to make those multiple per field cached attributes accessible in all the other classes.
That doesnt mean that there isnt any other solution, I am open to suggestions or forks or commits that fix my issues, since this was my first attempt.
And as of the GC allocation, I did not yet have time to look into it (the GC alloc itself) but a solution idea, if its caused by this implementation would be to release the cached attributes at some point. Feel free to add your implementation to this one or the main one, if it performs better in terms of performance, gc and code architecture/ maintenance.
I would also love this to be merged in the master of the main project and would offer my time to merge this together with the main dev.

[snorrsi]

@niggo1243
I cleaned up my minor changes and added them to my branch https://github.com/snorrsi/NaughtyAttributes/tree/v2_ss

There are more improvements to be made, I will add more to the branch when I have time to profile it better.
It's a bit annoying when trying to cache some data, gc alloc is always created.
It's possible to cache the Reflection result with type name and property.propertyPath, propertyPath can cause gc alloc though.
C# is a bit horrible when it comes to gc alloc.
For Reflection there seems to be no way around it, unless caching the result, but still with some gc alloc.

[niggo1243]

Hi @snorrsi
I have tested your version (v2_ss) with my "huge" test script superficially and it also doesn't introduce any major lags!
You can use this to test the performance of many NaughtyAttributes for one "huge" setting:
https://github.com/niggo1243/NaughtyAttributes/blob/master/Assets/NaughtyAttributes/Scripts/Test/HugeMixPerformanceTest.cs

So your version might be the way to go in terms of less maintenance needed compared to mine, at least at first glance.
I still didn't have the time yet to compare the gc allocs and exact performance differences but your version looks promising.

[celojevic]

thanks for this, significantly less laggy!