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

Speed up GDScriptLanguage::finish #94505

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

mihe
Copy link
Contributor

@mihe mihe commented Jul 18, 2024

(The issue here is more or less a duplicate of #85435, but with the number of scripts cranked up, as well as more complex dependencies.)

Despite the optimizations introduced in #85603 to lessen the exponentially slower shutdown time in GDScriptLanguage::finish, this exponential slowdown still very much remains an issue when the number of scripts in your project goes into the hundreds, with complex (potentially circular) dependencies, which is not unreasonable to find in larger projects.

Here is an MRP to illustrate the problem, with an accompanying Python script to generate the scripts: slow-gdscript-shutdown.zip

On my machine this MRP spends 12 seconds in GDScriptLanguage::finish when quitting the editor.

This slowdown is seemingly caused by every script having GDScript::clear called on it in GDScriptLanguage::finish, which in turn causes it to go looking for every dependency that it itself must clear, thereby calling GDScript::get_all_dependencies, which itself ends up calling GDScript::get_dependencies for every single script in the project, which is fairly costly on top of the obvious exponential complexity of this whole thing.

As far as I can tell this work (in the context of GDScriptLanguage::finish specifically) is entirely redundant. The whole point of the must_clear_dependencies stuff in GDScript::clear is, as mentioned above, to call clear on any of its must_clear_dependencies, but seeing as how every GDScript instance currently alive will have clear called on it in GDScriptLanguage::finish anyway, this work seems to be entirely redundant (again, in the context of GDScriptLanguage::finish specifically).

This PR speeds things up by simply checking in GDScript::clear if we're currently in GDScriptLanguage::finish and skips the transitive/recursive clear for its dependencies if that's the case, which brings the time of GDScriptLanguage::finish in the MRP down from 12 seconds to 6 milliseconds on my machine.

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.

Seems like an easy win worth having for the sake of complex projects.

@akien-mga akien-mga modified the milestones: 4.x, 4.3 Jul 18, 2024
@akien-mga akien-mga merged commit 60966f5 into godotengine:master Jul 18, 2024
18 checks passed
@akien-mga
Copy link
Member

Thanks!

@mihe mihe deleted the speed-up-gdscript-shutdown branch July 18, 2024 14:19
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.

4 participants