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

Use AHashMap for RBMap nodes and HashMap input_activity (reverted) #101548

Conversation

Nazarwadim
Copy link
Contributor

@Nazarwadim Nazarwadim commented Jan 14, 2025

Forgot to change in #92554

Gives 15% more FPS on MPR from #101494.

Helps with #101494

@Nazarwadim Nazarwadim requested a review from a team as a code owner January 14, 2025 17:42
@akien-mga
Copy link
Member

Gives 15% more FPS on MPR from #101494.

Probably closes #101494

That's amazing! 🎉
I wouldn't say that it closes #101494 though, it seems like we're still far from ideal performance targets. If 3.x was 50% faster than 4.x, even by reclaiming 15% off it we still have a way to go.

@Nazarwadim
Copy link
Contributor Author

Nazarwadim commented Jan 14, 2025

@akien-mga See this comment #101494 (comment)

In 4.3, for some reason, two animations are played due to the combination of AnimationPlayer and AnimationMixer. The first is empty from AnimationPlayer, which does nothing, but only takes resources, and the second is real from AnimationTree.

So if this will be merged in 4.4 we will get 20% faster animation compared to 3.6.

Edit: For some reason, it is only for me.

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

I am not familiar with AHashMap, but doesn't it change the sorting order as the hash seed is changed with each startup? If so, this argument is needed to avoid to change .tres. Resolved as AHashMap doesn't change index order.

scene/animation/animation_blend_tree.h Show resolved Hide resolved
@Repiteo Repiteo merged commit 96a6dc2 into godotengine:master Jan 31, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Jan 31, 2025

Thanks!

@akien-mga akien-mga changed the title Use AHashMap for RBMap nodes and HashMap input_activity Use AHashMap for RBMap nodes and HashMap input_activity (reverted) Feb 4, 2025
@akien-mga
Copy link
Member

Reverting with #102432 to fix #102374, but this is worth looking into again for 4.5 as the gains seemed to be substantial.

@akien-mga akien-mga changed the title Use AHashMap for RBMap nodes and HashMap input_activity (reverted) Use AHashMap for RBMap nodes and HashMap input_activity (reverted) Feb 4, 2025
@eddieataberk
Copy link

Reverting with #102432 to fix #102374, but this is worth looking into again for 4.5 as the gains seemed to be substantial.

#102376
This is pr closed but i highly recommend merging it. I've been using it in a very complex anim tree since yesterday with no issues.

@clayjohn
Copy link
Member

clayjohn commented Feb 5, 2025

@Nazarwadim Can you open a new PR with these changes combined with #102376 ?

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.

6 participants