Optimize AnimationMixer blend process #92838

Nazarwadim · 2024-06-06T12:55:39Z

This PR is created to optimize the AnimaionMixer _process_animation.

void AnimationMixer::_process_animation(double p_delta, bool p_update_only) {
	_blend_init();
	if (_blend_pre_process(p_delta, track_count, track_map)) {
		_blend_capture(p_delta);
		_blend_calc_total_weight();             
		_blend_process(p_delta, p_update_only);
		_blend_apply();
		_blend_post_process();
		emit_signal(SNAME("mixer_applied"));
	};
	clear_animation_instances();
}

Benchmarking methods:

I made some benchmarks of how long each of these methods takes for one 3D model, using animation_tree.

Some explanations to understand the results.
Units of measurement: usec.
is_process = _blend_pre_process
weight = _blend_calc_total_weight

Master:

You can see here that the _blend_process, _blend_pre_process and _blend_calc_total_weight methods take the most time.

I will show the results that are in this PR.

You can see that blend_process has improved by 30%, blend calc_total_weight has improved by 15% and _blend_pre_process has improved by 50%.

Real project benchmarks:

Master:
Project FPS: 56 (17.85 mspf)
Project FPS: 56 (17.85 mspf)
Project FPS: 56 (17.85 mspf)
Project FPS: 56 (17.85 mspf)
Project FPS: 55 (18.18 mspf)
Project FPS: 55 (18.18 mspf)

Current PR:
Project FPS: 68 (14.70 mspf)
Project FPS: 69 (14.49 mspf)
Project FPS: 66 (15.15 mspf)
Project FPS: 66 (15.15 mspf)
Project FPS: 68 (14.70 mspf)

Here you can see a pretty good + 22%. Note that #92554 also improves animation performance, which with this PR adds 40% to performance.

How to test:

Here #92554 in the benchmark section is Animation_test.zip project. After opening, fps will be output to the console.

What was done:

Animation:

For enums, the size was reduced from 4 to 1 byte.
Added the get_tracks method.

AnimationTree:

I made track_map a pointer in order not to copy maps.

AnimationMixer:

The first is to use getptr instead of has + operator[].
The second is to use int count = a->get_track_count(); In order to store in a register the number of iterations.
Used the method already created in animation.h to take the array. const Vector<Animation::Track *> tracks = a->get_tracks();
In the method post_process_key_value I cache whether there is GDVIRTUAL_CALL. That is, there will be only 1 check per _blend_process call.

Probably closes: #92693

AThousandShips · 2024-06-06T13:04:36Z

scene/resources/animation.h

@@ -45,7 +45,7 @@ class Animation : public Resource {

 	static inline String PARAMETERS_BASE_PATH = "parameters/";

-	enum TrackType {
+	enum TrackType : uint8_t {


I'm not sure how much this improves, the original data type would have been int but the alignment and other factors prevent significant optimization generally, but it might cause issues down the line

This was done in order to reduce the size of the Track from 40 to 32 bytes. You can check using sizeof.

Then that's useful, but needs to be evaluated for other aspects and for data safety with memory management, unsure if some data here is used in more direct ways that might break if you change it

JekSun97 · 2024-06-06T14:47:24Z

Your test project is not very suitable for such a test, due to the dynamic camera, and a large number of meshes, I used the project from a recent discussion (#92724), which uses 301 skeletons, and it really showed excellent results!

4.3 beta1 (master): FPS 47-48
This PR: FPS 56-58

I'll attach a project that is great for testing this PR:
anm.zip

Let's see what @TokageItLab says about the code

scene/animation/animation_mixer.cpp

Saul2022 · 2024-06-06T17:03:01Z

I would say that by itself it won’t be enough but with the pr you mentioned it might close it as it reduces it cost a lot more(40% as you stated when combining it )

Either way this is great job and hopefully it gets merged quick in 4.4 along with your other great optimizations pr’s.

JekSun97 · 2024-06-06T18:04:49Z

Either way this is great job and hopefully it gets merged quick in 4.4 along with your other great optimizations pr’s.

There are no compatibility problems or any major innovations here, this is just an optimization of what already exists.
So I think this would work well for 4.3 as well

Calinou

Tested locally (rebased on top of master 292e50e), it works as expected. Great work 🙂

Benchmark

Using an optimized editor build (optimize=speed lto=full) for all tests. Project is kept at its default window size and the camera isn't moved after startup. The editor is not running in the background.

PC specifications

CPU: Intel Core i9-13900K
GPU: NVIDIA GeForce RTX 4090
RAM: 64 GB (2×32 GB DDR5-5800 C30)
SSD: Solidigm P44 Pro 2 TB
OS: Linux (Fedora 39)

Using Animation_test.zip:

`master`	This PR	#92554	This PR + #92554 ¹
90 FPS (11.11 mspf)	116 FPS (8.62 mspf)	113 FPS (8.84 mspf)	123 FPS (8.13 mspf)

This required trivial code changes to get it to compile, since there's a small merge conflict when rebasing https://github.com/godotengine/godot/pull/92554 on top of this PR. Visual results still looked correct after doing so. ↩

TokageItLab

I recall our earlier discussion that we should use GDREGISTER_VIRTUAL_CLASS for some Animation related classes.

TokageItLab · 2024-06-12T00:09:44Z

scene/animation/animation_mixer.cpp

+Variant AnimationMixer::post_process_key_value(const Ref<Animation> &p_anim, int p_track, Variant p_value, ObjectID p_object_id, int p_object_sub_idx) {
+	if (is_GDVIRTUAL_CALL_post_process_key_value) {
+		Variant res;
+		if (GDVIRTUAL_CALL(_post_process_key_value, p_anim, p_track, p_value, p_object_id, p_object_sub_idx, res)) {
+			return res;
+		}
+		is_GDVIRTUAL_CALL_post_process_key_value = false;
+	}
+	return _post_process_key_value(p_anim, p_track, p_value, p_object_id, p_object_sub_idx);
+}
+


Maybe this change would not be necessary if you make AnimationMixer be registered as GDREGISTER_VIRTUAL_CLASS instead of GDREGISTER_ABSTRACT_CLASS in register_core_types.cpp? cc @reduz

This is also applicable to the AnimationNode.

It is still necessary. I don't know what is happening in the GDVIRTUAL_CALL method, but I know that it is slow no matter how this class was registered. And so this variable caches the return result of this method if it returned false.

Also, for some reason, the performance decreased when the registration was changed to GDREGISTER_VIRTUAL_CLASS

TokageItLab · 2024-06-12T00:22:05Z

scene/animation/animation_mixer.cpp

@@ -1073,15 +1076,18 @@ void AnimationMixer::_blend_calc_total_weight() {
 		real_t weight = ai.playback_info.weight;
 		Vector<real_t> track_weights = ai.playback_info.track_weights;
 		Vector<int> processed_indices;
-		for (int i = 0; i < a->get_track_count(); i++) {
-			if (!a->track_is_enabled(i)) {
+		const Vector<Animation::Track *> tracks = a->get_tracks();


Suggested change

const Vector<Animation::Track *> tracks = a->get_tracks();

const Vector<Animation::Track *> tracks = a->get_tracks();

I remember we always saying that we should use LocalVector whenever possible. There are several other places in the animation code where Vector is used besides here, but as long as it doesn't use has(), sort(), etc., I think it can be migrated to LocalVector.

Using LocalVector can result in many copies. Also, if it is changed, it will be necessary to make many changes in the already existing code. For example, with int in for loop, replace it with uint32_t so that there are no warnings from the compiler.
In order to remove the performance overhead, I used ptr.

JekSun97 · 2024-08-26T21:12:29Z

@reduz I think it's time to merge it with 4.4, no conflicts related to this PR have been found

TokageItLab

Looking at the code, there should be no change in the essential behavior. I guess there are places where we can be optimized more, but I think this PR is fine for now.

akien-mga · 2024-09-02T10:31:31Z

Thanks!

Nazarwadim requested a review from a team as a code owner June 6, 2024 12:55

AThousandShips reviewed Jun 6, 2024

View reviewed changes

AThousandShips added enhancement needs testing topic:animation performance labels Jun 6, 2024

AThousandShips added this to the 4.x milestone Jun 6, 2024

fire reviewed Jun 6, 2024

View reviewed changes

scene/animation/animation_mixer.cpp Outdated Show resolved Hide resolved

Calinou approved these changes Jun 11, 2024

View reviewed changes

Calinou mentioned this pull request Jun 11, 2024

Implement array based hash map AHashMap #92554

Merged

TokageItLab reviewed Jun 12, 2024

View reviewed changes

Calinou mentioned this pull request Jun 12, 2024

[Core] Optimize HashMap #90082

Open

akien-mga changed the title ~~Optimize animations~~ Optimize AnimationMixer blend process Jun 12, 2024

TokageItLab requested a review from reduz June 15, 2024 12:03

jitspoe mentioned this pull request Jun 21, 2024

AnimationPlayer/AnimationTree high process large framedrops #92693

Closed

This comment was marked as off-topic.

Sign in to view

fire requested a review from a team August 27, 2024 00:03

Optimize animations

3580ced

Nazarwadim force-pushed the small_animation_optimization branch from 6ada396 to 4fcae61 Compare August 30, 2024 16:37

Use static LocalVector for processed_hashes

660e28f

Nazarwadim force-pushed the small_animation_optimization branch from 4fcae61 to 660e28f Compare August 30, 2024 16:41

akien-mga requested a review from TokageItLab August 30, 2024 21:34

TokageItLab approved these changes Aug 31, 2024

View reviewed changes

TokageItLab modified the milestones: 4.x, 4.4 Aug 31, 2024

akien-mga merged commit 25fc316 into godotengine:master Sep 2, 2024
19 checks passed

Nazarwadim deleted the small_animation_optimization branch September 3, 2024 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize AnimationMixer blend process #92838

Optimize AnimationMixer blend process #92838

Nazarwadim commented Jun 6, 2024

AThousandShips Jun 6, 2024 •

edited

Loading

Nazarwadim Jun 6, 2024

AThousandShips Jun 6, 2024

JekSun97 commented Jun 6, 2024

Saul2022 commented Jun 6, 2024

JekSun97 commented Jun 6, 2024

Calinou left a comment •

edited

Loading

TokageItLab left a comment •

edited

Loading

TokageItLab Jun 12, 2024 •

edited

Loading

Nazarwadim Jun 15, 2024

Nazarwadim Jun 15, 2024

TokageItLab Jun 12, 2024

Nazarwadim Jun 15, 2024

This comment was marked as off-topic.

This comment was marked as off-topic.

JekSun97 commented Aug 26, 2024 •

edited

Loading

TokageItLab left a comment •

edited

Loading

akien-mga commented Sep 2, 2024

	const Vector<Animation::Track *> tracks = a->get_tracks();
	const Vector<Animation::Track *> tracks = a->get_tracks();

Optimize AnimationMixer blend process #92838

Optimize AnimationMixer blend process #92838

Conversation

Nazarwadim commented Jun 6, 2024

Benchmarking methods:

Real project benchmarks:

How to test:

What was done:

AThousandShips Jun 6, 2024 • edited Loading

Choose a reason for hiding this comment

Nazarwadim Jun 6, 2024

Choose a reason for hiding this comment

AThousandShips Jun 6, 2024

Choose a reason for hiding this comment

JekSun97 commented Jun 6, 2024

Saul2022 commented Jun 6, 2024

JekSun97 commented Jun 6, 2024

Calinou left a comment • edited Loading

Choose a reason for hiding this comment

Benchmark

Footnotes

TokageItLab left a comment • edited Loading

Choose a reason for hiding this comment

TokageItLab Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

Nazarwadim Jun 15, 2024

Choose a reason for hiding this comment

Nazarwadim Jun 15, 2024

Choose a reason for hiding this comment

TokageItLab Jun 12, 2024

Choose a reason for hiding this comment

Nazarwadim Jun 15, 2024

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

JekSun97 commented Aug 26, 2024 • edited Loading

TokageItLab left a comment • edited Loading

Choose a reason for hiding this comment

akien-mga commented Sep 2, 2024

AThousandShips Jun 6, 2024 •

edited

Loading

Calinou left a comment •

edited

Loading

TokageItLab left a comment •

edited

Loading

TokageItLab Jun 12, 2024 •

edited

Loading

JekSun97 commented Aug 26, 2024 •

edited

Loading

TokageItLab left a comment •

edited

Loading