Optimize animation performance

[Ryan-000]

---

Before this patch (2dd26a0).

FPS: high 192, low 185, median 188.5
MSPF: high 5.40, low 5.20, median 5.30

After this patch. (Benchmarks no longer accurate)

FPS: high 234, low 229, median 231
MSPF: high 4.36, low 4.27, median 4.32

---

Here is the test project I used, I ran it from the command line with --headless --print-fps, and the engine was compiled with scons compiledb=yes debug_symbols=yes production=yes
Mrp.zip

Changes:

• Replaced List<> usages with LocalVector<>.
• Updated many variables to use &, to avoid copies.
• Changed many function parameters/returns from by-value to const & (to avoid copies and ref counting overhead)
• Use single-lookup (e.g. has_node()+get_node() changed to get_node_or_null()), same for HashMaps.
• Added SNAME when needed.
• AnimationNodeBlendTreeEditor now queues (and merges) update_graph calls at the end of the frame.
• Large structs like AnimationMixer::PlaybackInfo are now passed by const & instead of copied.
• AnimationNode::_blend_node now uses preallocated StringNames.
• AnimationTree is now safe to use with Node::process_thread_group
• Misc optimizations and improvements

[hsvfan-jan]

What kind of hardware did you use for those performance tests? Removing pretty much exactly 1 ms is very nice! I wonder how it scales on lower or higher end hardware.

[Ryan-000]

What kind of hardware did you use for those performance tests? Removing pretty much exactly 1 ms is very nice! I wonder how it scales on lower or higher end hardware.

I tested it on my Surface Laptop Studio 2 with an i7-13700H

[Ivorforce]

I think the function you're looking for already exists (operator[]). Unless you really do need r_was_added? But I don't see you use this function at all. Did you add it by mistake?

[Ryan-000]

I need r_was_added here

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_mixer.cpp#L139

[Ivorforce]

Please don't resolve comments prematurely.
Thanks for the links, for some reason GitHub didn't show me those on search. Still, i don't like duplicating methods. Figuring this out can wait until after this comes out of draft again though.

[Nazarwadim]

You can use find + insert_new instead

For example
https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

Here you can:

	AHashMap<StringName, Pair<Variant, bool>> &property_map = process_state->tree->property_map;
	auto iter = property_cache.find(p_name);
	if(!iter) {
		iter = property_cache.insert_new(p_name, 0);
	} else {
		return property_map.get_by_index(iter->value).value.first;
	}

[Ryan-000]

You can use find + insert_new instead

For example

https://github.com/godotengine/godot/blob/677d862ce082019c762fe84ebe76ec56b7725d32/scene/animation/animation_tree.cpp#L136

Here you can:

	AHashMap<StringName, Pair<Variant, bool>> &property_map = process_state->tree->property_map;
	auto iter = property_cache.find(p_name);
	if(!iter) {
		iter = property_cache.insert_new(p_name, 0);
	} else {
		return property_map.get_by_index(iter->value).value.first;
	}

Problem is using find then insert_new hashes the key twice, while get_value_ref_or_add_default hashes it once.

[TokageItLab]

Probably there's a lot of overlap with #101285. So I think there is worth for you discuss with @Nazarwadim.

[Ryan-000]

Probably there's a lot of overlap with #101285. So I think there is worth for you discuss with @Nazarwadim.

This PR will then have to wait until #101285 is merged in

[clayjohn]

@Ryan-000 We just merged #101285! As expected it led to some conflicts with your PR.

[TokageItLab]

Changing String->StringName and modifying the importer should generally be fine.

However, this PR's changes to AnimationTree completely break the behavior for nested AnimationNodeBlendTree instances. When I looked at the code, I noticed that the way AnimationNode instances are accessed has changed and since that concerns me, I tested it.

Before:

nested_atree.mp4

After this PR:

nested_atree_after.mp4

nested_tree.zip

Therefore, I recommend removing the AnimationTree's change to how AnimationNode instances are accessed from this PR for now. And if you really want to implement it, it would be better to create a separate PR for that.

[Ryan-000]

I'm going to split this into many smaller PRs to make it more manageable.

[Ryan-000]

I changed my mind