Skip to content

Fix loss of precision in AnimClock and improve performance - #24

Merged
djeedai merged 1 commit into
djeedai:mainfrom
SUPERCILEX:precision
May 17, 2022
Merged

Fix loss of precision in AnimClock and improve performance#24
djeedai merged 1 commit into
djeedai:mainfrom
SUPERCILEX:precision

Conversation

@SUPERCILEX

Copy link
Copy Markdown
Contributor

Old:

1s 320ms
Left: 140292640
Right: 140292630

New:

759ms
Success

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
@SUPERCILEX

Copy link
Copy Markdown
Contributor Author

Oh, for whatever reason now I'm seeing ~1s time for the old version. The new one is still faster, but not by as much I guess.

@djeedai

djeedai commented May 16, 2022

Copy link
Copy Markdown
Owner

If you want stable results you need to bench it. This test is not warming the codepath before measuring, so will have variations per run. An ideal perf test runs the code first a few times so that all caches are hot and the CPU frequency scaling if any is already in power/perf mode (and ideally disables low-power and frequency scaling, but that's not always possible). That's probably good enough though given the large difference before/after.

@SUPERCILEX

Copy link
Copy Markdown
Contributor Author

Yeah, I'm being very lazy. I mainly just wanted to check that using u128s wasn't slower than the old stuff.

@djeedai

djeedai commented May 17, 2022

Copy link
Copy Markdown
Owner

I'm going to merge as is for now, but I think the removal of times_completed will have to be reverted for #16 to work. Anyway, we'll see.

@djeedai
djeedai merged commit ab1e792 into djeedai:main May 17, 2022
@SUPERCILEX
SUPERCILEX deleted the precision branch May 17, 2022 21:15
@SUPERCILEX

Copy link
Copy Markdown
Contributor Author

Sounds good! I don't have time this week, but I'll start working on your ideas for #19 (unless you get to it first 😄) which will let me see what's still applicable in #21.

azarmadr pushed a commit to azarmadr/bevy_tweening that referenced this pull request Aug 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants