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

Slow Create Resource Dialog in a project with a lot of class_names #34205

Closed
HeartoLazor opened this issue Dec 8, 2019 · 9 comments · Fixed by #86447
Closed

Slow Create Resource Dialog in a project with a lot of class_names #34205

HeartoLazor opened this issue Dec 8, 2019 · 9 comments · Fixed by #86447

Comments

@HeartoLazor
Copy link
Contributor

HeartoLazor commented Dec 8, 2019

Godot version:
Bug Introduced in 3.1.2 stable

OS/device including version:
Windows 10

Issue description:
When the user try to set a resource in inspector, the editor takes ages to show the context menu, this happens in projects with a lot of class_names, so should be related to #27566
@willnationsdev
Here is a video of the problem:
https://gfycat.com/RegalNeatIrishdraughthorse
In the video the loading takes almost a full dancing pikachu loop.

This issue was introduced in godot stable 3.1.2, in 3.1.1 only happened with the add node button.

Steps to reproduce:
Create a project with multiple class_names (100+)
Try to set a resource in the inspector by clicking it (drag a resource on top don't reproduce the problem)

@willnationsdev
Copy link
Contributor

There is a PR for caching the Scripts when building the Tree rendered in the CreateDialog that has already been merged in 3.2. I have further ideas for optimizing the Tree building process, but I won't be able to get them in until 4.0. as far as I can tell though, caching the Scripts has already improved the performance by a lot.

@ghost
Copy link

ghost commented Dec 28, 2019

@willnationsdev Which PR?

@willnationsdev
Copy link
Contributor

@avencherus #33387

@ghost
Copy link

ghost commented Dec 29, 2019

Thanks.

@willnationsdev
Copy link
Contributor

willnationsdev commented Jan 1, 2020

Oh, I just realized that the CreateDialog script caching doesn't really have any bearing on the Resource dropdown performance. My bad. But yeah, we could take a similar approach there, in addition to the other stuff I was thinking of.

The main thing that I want to do is start making the inheritance data get cached. Either caching things in the EditorData (in a generic way), or tailoring the caching operations to the specific types of nodes that need them (CreateDialog, EditorPropertyResource) which can keep a static HashMap of whatever caching structure(s) we use.

That, and there are certain classes that aren't supposed to show up in those lists like core bind classes (prefixed with "_"), Editor classes (prefixed with "Editor"), and classes on CreateDialog's blacklist (which iirc includes Editor classes that are exposed in the editor context, but happen to not be prefixed with "Editor"). In the current implementation, a lot of that stuff is built into a list and then cleared and re-added frequently, even though it is static information per engine build.

Also, the CreateDialog is iterating over a list which is updated every time the CreateDialog is opened (or something like that). It would be better if it were a vector that updated only at engine start and when script classes are updated, that way the processing to build the list and all the caching of needed sub-inheritance-hierarchies can be done before the CreateDialog and EditorPropertyResource objects are even accessed/displayed.

So, if you know ahead of time that you are going to need Node and Resource inheritance hierarchies (which we do), then we can add those to the list of cached hierarchies that are calculated with every script class update. And if you export a resource on a script, then the specific type of Resource can be added to the list of types to cache inheritance hierarchies for. That's the kind of thing I was thinking.

That pretty much sums up all of my plans regarding these.

@Calinou Calinou closed this as completed Apr 11, 2020
@Calinou Calinou reopened this Apr 11, 2020
@Calinou
Copy link
Member

Calinou commented Apr 11, 2020

Related to #27333.

@Xrayez
Copy link
Contributor

Xrayez commented Jun 23, 2020

I'm not quite sure what exact changes has caused this in 3.1.2, but I stumbled upon this PR: #25676.

The PR used:

script->get_language()->get_global_class_name(script->get_path(), NULL, &current_icon_path);

Which, according to my profiling, causes significant hiccups when searching the classes, but not when opening the create dialog.

And get_global_class_name for GDScriptLanguage is slow in and of itself because it forces to parse an entire script each time, as described by @vnen in #27333 (comment).

Reverting #25676 gives performance boost for searching classes from 1.0 second to 0.1 second on typing (with 1000 global scripts), but reintroduces #24657.

@clemens-tolboom
Copy link

@HeartoLazor thanks for the video. Made me smile.

image

This hits me too in 3.3-rc8 while testing with just 1 AssetLib:gdUnit3 MikeSchulze/gdUnit3#51 in case ppl want to reproduce the easier way.

@clemens-tolboom
Copy link

Having the plugin disabled/not enabled produce this (~20secs) issue.

After enabling the plugin performance improves by a factor of 10 (~2secs).

@akien-mga akien-mga added this to the 4.3 milestone May 15, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in 4.x Priority Issues May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
7 participants