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

Generate script resource preview without parsing #87625

Merged

Conversation

YuriSizov
Copy link
Contributor

@YuriSizov YuriSizov commented Jan 26, 2024

During the last days of development of Godot 4.2 we were fighting a lot with deadlocks during the editor startup.

One of the sources of these deadlocks was the editor resource preview generator. Specifically, the one implementation responsible for generating previews for script files. Specifically specifically, GDScript files. The cause for this issue was in the way GDScript files resolve their dependencies when loaded.

Essentially, loading a script containing references to other resources, such as preloads of packed scenes, will trigger a load for those resources as well. And to generate a preview of a script we would load it completely and fully. Which means attempting to generate a preview of a script may trigger a load of a packed scene, which may need a preview of its own, which will deadlock the generator.

The sad part is that we don't need a fully loaded script. We only take the source code of the script (plus a reference to its ScriptLanguage). This is something we can do by simply reading the file as text.

Which is what this PR does. It makes sure that instead of loading a script resource we just read it as text and use the file's extension to get the corresponding ScriptLanguage, cheap and simple. The rest is the same as before.

There is no noticeable performance difference, unfortunately. At least not with an example I could fabricate to test this with. YMMV.

Before:

Generated 'res://OneBadScript.gd' preview in 5148 usec
Generated 'res://OneBadScript.gd' preview in 4171 usec
Generated 'res://OneBadScript.gd' preview in 5058 usec
Generated 'res://OneBadScript.gd' preview in 4574 usec

After:

Generated 'res://OneBadScript.gd' preview in 5484 usec
Generated 'res://OneBadScript.gd' preview in 4343 usec
Generated 'res://OneBadScript.gd' preview in 4933 usec
Generated 'res://OneBadScript.gd' preview in 4096 usec

Some deviations are possible depending on how busy the editor is in each particular moment.

There is no visible difference in the generated file either:

Before
before
After
after


I also added a small benchmark for the generator to test the timings. I decided to keep it for this PR, but as a separate commit and as verbose prints. If this is excessive I can drop it. Note that I couldn't use print_verbose here because print_verbose is undefined by the included variant_utility.h header. Probably something worth addressing?

@YuriSizov YuriSizov added this to the 4.3 milestone Jan 26, 2024
@YuriSizov YuriSizov requested a review from a team as a code owner January 26, 2024 18:21
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me. A much welcome improvement!

editor/editor_resource_preview.cpp Show resolved Hide resolved
editor/editor_resource_preview.cpp Show resolved Hide resolved
@akien-mga akien-mga merged commit 1774c17 into godotengine:master Feb 9, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@YuriSizov YuriSizov deleted the editor-lightweight-script-previews branch February 9, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants