Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IMGUI: performance drops considerably in latest update on persistent floaty inspector window #98

Open
laurentopia opened this issue Nov 14, 2024 · 16 comments
Assignees
Labels
enhancement New feature or request

Comments

@laurentopia
Copy link

when you press alt+p you get a floaty inspector
if that floaty window has a saintsfield [] it'll tank editor performance
image
image
when i fold the component
image
I tried other components that use these [] and the perf hit isn't bad.
was these:
image
they've gone bonker perf wise
doesn't seem to come from the unity getcomponentinchildren<>
image

@laurentopia
Copy link
Author

errata: it seems to happen at the gameobject level, so if you have 20 go with that component and only 1 shows its inspector then the performance hits like 20 objects, not just one

@TylerTemp
Copy link
Owner

TylerTemp commented Nov 14, 2024

Yeah this is a flaw.

When developing GetByXPath (which is the actual logic for Auto Getter attributes) I was considering the performance issue, especially for single field (rather than on a list/array), but decide to set it later because of its complexity.

Let's put this in the next milestone. I need to focus on the current one first.

Off the topic: when I was developing Auto Getters I talked to my friend, and said, "this is gonna be the best feature of this project, and the biggest crap-load too" :) Yeah it's very useful I'm using it on my daily development, but indeed it need some improvement, and it needs it right now LOL.

@TylerTemp TylerTemp self-assigned this Nov 14, 2024
@TylerTemp TylerTemp added the enhancement New feature or request label Nov 14, 2024
@TylerTemp TylerTemp added this to the Night on the Sun milestone Nov 14, 2024
@laurentopia
Copy link
Author

and it needs it right now LOL.

It does and I don't know why suddenly everything is super slow, but an overhaul takes time so I was thinking that a temporary fix can be done, maybe by boxing the logic that makes GetComponent* costly in a if (!Application.isPlaying && !editingprefab).
Added benefit: it's also more consistent with build behavior.

@TylerTemp
Copy link
Owner

Hi,

in version 3.5.1, the resource fetching is greatly reduced.

If there are 100 resources that match the requirement by an auto-getter, a single field will only fetch the 1st one. (Previous version would fetch 100 and choose the first one)

For list/array:

  1. For UI Toolkit, the decorator will only fetch 100 items, and fill the list/array
  2. For IMGUI, the decorator will fetch 1 + 2 + 3 ... + 100 times, and fill the list/array (previous version will fetch 100 * 100 times)

IMGUI still need to optimize but, at this point I'll set it up later...

For IMGUI, please also change the delay to 1000 (1 second). If the resources are way too many, you can change it to zero to disable the real time checking. The default value is way too frequent for IMGUI, so please CHANGE IT in config.

image

About other things: If you're using GetPrefabWithComponent etc, it's better to use GetByXPath if your resources are in some specific folders, this will avoid the getter to search the whole project.

Lastly: there are not much space for optimize for this kind of attributes atm, and I don't see any document from Unity about a better way (except QuikeSearch), so this topic might not be working on very long tho.

@laurentopia
Copy link
Author

laurentopia commented Nov 25, 2024

N(N+1)/2 < NN but I don't feel the difference at 100ms. I look in the deep profiler and see 13k reflection calls so that's clearly better. Switching to 1 second debounce does nothing, I thought it did but that's because the inspector had focus on the saints config. So I'll keep collapsing components.

Out of curiosity, why did IMGUI start doing N^2 after that update? And not UITK?
is N the number of gameobjects children or the total number of components?

Back when I used ongui for game UI, it would fire up the same code multiple time per frame. I had to use a particular event, redraw, but at most it would 3ple the number of calls so I don't think that's your case.

@laurentopia
Copy link
Author

I also noticed that sliders are now very slow

@TylerTemp
Copy link
Owner

Out of curiosity, why did IMGUI start doing N^2 after that update? And not UITK? is N the number of gameobjects children or the total number of components?

For UI Toolkit, the item can targeting the list drawer, thus only one element can do all the heavy lifting, and all the rest element in array/list just wait to get feed.

IMGUI is not the case.

Reflection calls is in-avoidable because there are only two ways in Unity that have good performace: TypeCache, and QuickSearch. However, both are not the case for the auto getters.

@TylerTemp TylerTemp changed the title performance drops considerably in latest update on persistent floaty inspector window IMGUI: performance drops considerably in latest update on persistent floaty inspector window Nov 26, 2024
@TylerTemp
Copy link
Owner

At the moment I'd close this issue as "won't fix" for the following reasons:

  1. In IMGUI, Unity uses a shared property drawer for every element inside an array/list. This makes data cache difficult.
  2. For SaintsField's IMGUI code, SaintsField needs to create some cached multiple drawer instances, which makes the attributes stuck and fallback work. This also makes the data cache more difficult to maintain.

In IMGUI SaintsField uses some cache to avoid some issues, these caches are mostly lightweight for small situations. However, for this specific issue, the cache logic will make it way too complex.

Things might get better when I work on the QuickSearch API, but that'll be some other APIs/Decorators, even it's also in the auto getters family.

There are so many things with IMGUI, and honestly I mostly work on UI Toolkit side. Considering my time and energy, I'll close this issue, and will consider it again much later (when I feel necessary, or when many ppl need it, or someone can give a better idea of how to do it...). Before that, one possible way is, the old version's auto getter are quite neat and standalone, you can always grab it, do a simple rename, and put it in your project. For me, I'd avoid to maintain a separated version only for that.

Oh, and, in IMGUI the ListDrawerSettings is weirdly slow and I have not much clue why. Maintaining IMGUI is... painful...

@TylerTemp TylerTemp closed this as not planned Won't fix, can't repro, duplicate, stale Nov 26, 2024
@laurentopia
Copy link
Author

I hear ya. The problem with abandoning IMGUI is that UITK became somewhat stable only in 2022, and 2022 added a slew of major performance regressions, we're talking about 20% perf drop on Quest! U6 is even worse.

An earlier version of SaintsField is much faster: "today.comes.saintsfield": "https://github.com/TylerTemp/SaintsField.git#3.3.9".

@TylerTemp
Copy link
Owner

Thanks for your understanding.

I've add a discussion. Not sure how many people will anticipate, but please consider vote there (Click the smile icon, and choose thumb down 👎 to vote) so I can know if I need to make IMGUI a bit higher priority

@laurentopia
Copy link
Author

I don't understand why you're reverting the changes that caused that regression in the first place. 3.3.9 worked fine.

@TylerTemp
Copy link
Owner

Hi,

could you help to have a check on the new version with:

window - Saints - Create or Editor SaintsField Config, then set Update Interval (ms) to 0 (means disabled).

This is the default behavior of the old version, which checks the value on first inspector, and will not constantly run the value checker.

Still, some auto getters will consume a bit more than the previous good old version, but that should be not a big impact.

@TylerTemp
Copy link
Owner

Hi,

I'm back to IMGUI fixing as I'm feeling better (physically). Could you please try 3.6.1 and check if things are better in this version.

There are still some improvement can be done for IMGUI. But I need some time to make it happen, step by step.

And, thanks for your continuously support!

@TylerTemp TylerTemp reopened this Dec 4, 2024
@laurentopia
Copy link
Author

Hi,

Thanks for not giving up on IMGUI.

I just tried it and see no difference with 3.3.9 which is a good thing.

What did you do?

@TylerTemp
Copy link
Owner

Hi,

Thanks for your feedback.

While developing #95 , it gives me some ideas about how should I deal with IMGUI under the situation that Unity will reuse the Drawer, making the caching nearly impossible.

The new idea, idealy, should benefit both UI Toolkit & IMGUI. But due to the limitation of my time, the thing will happens as:

  • Finish [Milestone] Handles in the scene #95
  • Make an auto-runner for Required and auto-getters for scenes, a project, and for Addressable, with a config to allow specify assets to be checked also (I need this for my own project...)
  • Some small work to make [Request] More ELayout.Horizontal options and Customizable backgrounds #107 work
  • Auto Getters Improvement: change the general logic -----> this should give a little bit performance improvement
  • Auto Getters Improvement: change the logic of how things are found and cached ----> this should give some boost of performance

Please stay tuned. And thanks for your patience :)

@laurentopia
Copy link
Author

That's great! Do you think that could make [ShowInInspector] working when doing this?
base. is nuking non serialized things.
public override void OnInspectorGUI()
{
base.OnInspectorGUI();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants