Closed Dev-Salem closed 1 month ago
Just to add to this: there should be no reason to ever use a zero duration periodic timer. It's basically a non-blocking while (true)
loop, which will always cause significant CPU usage. At most, this timer needs to run once per frame, and you don't need a Timer to do that - Flutter exposes methods to register frame callbacks.
However, even in this case such callbacks would cause unnecessary overhead. One of the tasks this timer performs is updating a ValueNotifier<Duration>
with the current animation time. The reason this is problematic is that ValueNotifier
is only meant for discrete state changes, but elapsed time is a continually changing value. Trying to represent that in a ValueNotifier results in a very wasteful way of representing the information. A better way to handle this would be a DateTime startTime
field, and maybe a Duration get elapsedTime
that can compute the elapsed time based on the start time - it is then up to users to poll that getter only when they want access to the start time, instead of Rive always updating a ValueNotifier that may not be used.
The other task this timer does is disposing of finished animations. This can instead be done by creating a timer for an animation's length when it starts which will dispose the animation.
Basically: don't poll for events, react to them instead.
Hi @Dev-Salem and @abitofevrything, the underlying timer will be stopped and set to null when the owning state machine controller is disposed of.
What you're missing in your above code is to call controller.dispose()
, as you're managing the StateMachineController
in this instance. You can do this in the widget's dispose()
method, or wherever it makes sense in your application.
Could you add that change and confirm if that resolves it for you, thanks.
This will not resolve the issue. There should never be a 0 duration periodic timer, whether it only runs during the animation or not.
Ah thank you for highlighting that! Let me sync with the team to look into resolving this.
But you should still be disposing the state machine controller to resolve the issue of still seeing this high CPU usage after the page is disposed of.
We should clean this up, but there's some nuance to this and reason as to why we're doing this odd bit of polling that has to do with Flutter's threading model with FFI.
Fixing it requires a little bit of FFI (and WASM for web) refactoring of the internal sound API to let native call up when sounds complete (which also requires some extra mapping and storage for the lookup between C++ and Dart of these sound refs). We do this in rive_native for render paths @HayesGordon and it helps remove some of the polling nature.
We may still need to poll (although way less frequently) to clean the refs as Flutter's Native Finalizers callback on a separate thread and at least as of Flutter 3.24.0 even using a NativeCallable.listener isn't viable as it will cause a deadlock in the Flutter engine.
All that to say, we need to spend some time re-working the polling nature of this on our side, but in the meantime I'd try what Gordon is suggesting to unblock yourselves.
Another potentially easy win is to increase the timeout to only let it process the sound cleanup. I don't think we need the ValueNotifiers at runtime. We can refactor that pretty quickly (and we can implement a TrackingAudioPlayer for where we do need it @HayesGordon).
The issue with a 0 duration timer is that whenever it is running, even if that's just during the animation, the CPU will be pinned to 100% usage.
I agree a proper fix is to rework the code so that the native side can call up when an animation completes. That is the "correct" solution.
However I also believe that not causing incredibly high CPU usage whenever an animation runs should be a high priority issue, especially when the temporary fix is just to increase a polling interval from 0 to a few ms...
This should be fixed in 0.13.14
I wasn't able to replicate the 100% CPU usage, but let us know if you're still encountering issues. Maybe there is something else worth investigating.
Description
In our app, we use Rive for the splash screen (the animation also plays audio), but we noticed that even after the animation is finished and the page is disposed of, the CPU usage is still high (this problem only occurs on iOS). After further investigation, we found that Rive creates a periodic timer with a duration of 0. This timer isn't disposed of, and it runs in the background, causing 200% CPU usage and device overheating (this could also be relevant to #396).
Stack trace:
The function that creates the periodic timer with duration 0:
We managed to temporarily fix the problem by overriding the behavior of creating a periodic timer with a duration of 0 using zones:
Steps To Reproduce
minimal reproducible example:
Expected behavior
We expect Rive to be 1) performant with minimal CPU usage on all platforms, and 2) any Rive functions to be disposed of after the animation finishes and the widget is no longer a part of the widget tree.
Screenshots
CPU usage as it appears on Xcode's Instruments, running in profile mode with a blank page
CPU processes on the dev tools
CPU usage after implementing the temporary fix
Device & Versions
Additional context
Relevant discussion on discord (CC: @abitofevrything)