[3.x] Add theme item cache to Control

[jordo]

Back port of #64314

Part of a few PRs to address: #64241

To quick and dirty benchmark an affected control function say get_color, you can add the following tool script to any Control node. This will give you a run_toggle flag in the inspector, and will print the execution time in millis to the editor console.

tool
extends Control

export var run_toggle := false

func _ready():
	pass # Replace with function body.


# Called every frame. 'delta' is the elapsed time since the previous frame.
func _process(delta):
	if(run_toggle):
		run_toggle = false
		var cpu_start_time := OS.get_ticks_msec()
		for i in range(100000):
			get_color("error_color", "SomeClass")
		var cpu_end_time := OS.get_ticks_msec()
		print("Time in millis: " + String( cpu_end_time - cpu_start_time))

The performance increase will be dependent on any particular project. As the complexity of theme computation in a project will differ in each project and on any particular scene tree... But the following lines are expensive due to complexity and memory allocations in the List<> itself and what ends up getting populated in the theme_types list:

	List<StringName> theme_types;
	_get_theme_type_dependencies(p_theme_type, &theme_types);

The cache in this PR avoids this hotspot, and in our project, the timing from the above gdscript tool results on average a 6x speedup improvement in most of the affected control functions:

With the cache:

Without any caching:

The above were tested in editor with release_debug. The actual improvements in our project are even higher (roughly 10-20x), when benchmarked outside of gdscript when called from c++ (which we can't really provide a MRP for) and on non-editor pure release builds.

[Anutrix]

Is this PR complementary to #64314 or was it superseded by it?

[YuriSizov]

It's a backport to 3.x

[jordo]

bump, @YuriSizov @akien-mga This was missed for the 3.5.1 release. Is there anything else I can do here?

[Calinou]

This was missed for the 3.5.1 release. Is there anything else I can do here?

To minimize the risk for regressions and accidental compatibility breakages, 3.5.x usually only gets bug fixes, not new features. If this PR is merged, this feature would normally only be in 3.6.

That said, since this PR fixes a performance regression between 3.4 and 3.5, an exception may be made here to cherry-pick it to the 3.5 branch.

[jordo]

I don't think this is really a feature, it's only intended to address a performance regression. The editor was close to unusable for us and extremely laggy with a lot of console output... so I'm surprised that no-one else is experiencing this. But also perhaps it is only us though, as I would imagine there would be more issues reported if it was widely experienced.

[akien-mga]

This looks good, but I'd prefer if the commit had the proper co-author attribution and title like #64314.

We usually do cherry-picks with git cherry-pick -x 9f88300007cd1d418e2c63c663ca660e46d72fa4, then solve merge conflicts, which would keep the author and commit log and add a reference to the original commit, like this:

commit b229fed0c0831d2a6724c56b3bb089160f821420 (HEAD -> fix/control-theme-cache)
Author:     Yuri Sizov <yuris@humnom.net>
AuthorDate: Fri Oct 14 16:52:58 2022 +0200
Commit:     Jordan Schidlowsky <jordanschidlowsky@gmail.com>
CommitDate: Fri Oct 14 16:56:53 2022 +0200

    Add dumb theme item cache to Control
    
    (cherry picked from commit 9f88300007cd1d418e2c63c663ca660e46d72fa4)

You can also add yourself as co-author if you had to do significant work to make the backport with Co-authored-by: Name <email> as last line in the commit message.

This makes it easy to find back the original commit, and the original PR, for a cherry-picked commit.

[jordo]

Just getting back to this here, and I tried to cherry-pick the suggested commit into what's current on 3.x, and at first glance it's just a bunch of more work to resolve all the conflicts in suggested manner (and then test again to make sure i didn't miss anything and have merged into the correct spots). example of the proposed cherry-pick below, (these are just totally wrong and not even close to correct and I would just end up redoing what I have already done):

So is there another method to get original author and commit reference into this PR so it is acceptable without redoing this work manually? ( I do want to credit and capture that, I just don't want to redo and potentially re-break something). Most of the work here isn't necessarily in the code itself, but a lot of my time was spent identifying & isolating the issue, & then verifying, testing, and profiling the proposed backport of the cache impl.

[akien-mga]

Sorry for the delay. You could use the commit I pushed to my branch: https://github.com/akien-mga/godot/commits/fix/control-theme-cache (rebased from what I had done back in October as example)

It can be done manually like this:

GIT_AUTHOR_NAME="Yuri Sizov" GIT_AUTHOR_EMAIL="yuris@humnom.net" GIT_AUTHOR_DATE="Fri Aug 12 14:36:06 2022 +0300" git commit --amend --reset-author

Then set commit message to:

Add dumb theme item cache to Control

(cherry picked from commit 9f88300007cd1d418e2c63c663ca660e46d72fa4)

Co-authored-by: Jordan Schidlowsky <jordanschidlowsky@gmail.com>

[akien-mga]

Thanks!