Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@jonahwilliams
Copy link
Contributor

With the current stopwatch design, we request the frame rate multiple times per frame. On Android this calls into JNI which is pretty slow. If we cache the value at construction of the StopwatchVisualizer, then we will only compute it once per frame (because the StopwatchVisualizer is reconstructed each frame).

Fixes flutter/flutter#137797

So the issue isn't that we're checking the fresh rate every frame, its that we were checking N times on each frame.

explicit StopwatchVisualizer(const Stopwatch& stopwatch)
: stopwatch_(stopwatch) {}
: stopwatch_(stopwatch) {
frame_budget_ = stopwatch_.GetFrameBudget();
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's note why this is happening so someone doesn't refactor it back in the future (it won't be clear this is a JNI call).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 13, 2023
@auto-submit auto-submit bot merged commit ed001a5 into flutter:main Nov 13, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 13, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 13, 2023
flutter/engine@74ba6c1...db6da00

2023-11-13 jonahwilliams@google.com [Impeller] Add convex tess benchmark. (flutter/engine#47956)
2023-11-13 jonahwilliams@google.com [engine] request frame rate once per frame. (flutter/engine#47954)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@jonahwilliams jonahwilliams deleted the cache_frame_rate branch November 16, 2023 18:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[engine] Stopwatch Visualizer checks refresh rate every frame.

2 participants