[win32] Replace threadpool timer with custom background thread timer #179249

knopp · 2025-11-30T16:31:18Z

It seems that for some reason the win32 thread pool timer is not precise enough for our use case which causes major frame pacing issues on Windows. This PR replaces it with a simple custom built background thread timer.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

knopp · 2025-11-30T16:33:10Z

With this PR I'm getting a constant 120fps on Windows.

gemini-code-assist

Code Review

This pull request replaces the Win32 thread pool timer with a custom background thread timer to address frame pacing issues. The new TimerThread class provides a more precise timing mechanism. The implementation is a good approach, but I've found a few issues: a critical race condition in the TimerThread constructor due to member initialization order, and a high-severity bug in the timer scheduling logic that can lead to an overflow and prevents timer cancellation. My review comments provide details and suggestions for fixing these issues.

engine/src/flutter/shell/platform/windows/task_runner_window.cc

gemini-code-assist · 2025-11-30T16:33:40Z

engine/src/flutter/shell/platform/windows/task_runner_window.cc

+  // Schedules the callback to be called at specified time point. If there is
+  // already a callback scheduled earlier than the specified time point, does
+  // nothing.
+  void ScheduleAt(
+      std::chrono::time_point<std::chrono::high_resolution_clock> time_point) {
+    {
+      std::lock_guard<std::mutex> lock(mutex_);
+      if (time_point < next_fire_time_) {
+        next_fire_time_ = time_point;
+      }


The current implementation of ScheduleAt only updates the fire time if the new time is earlier. This prevents the cancellation or rescheduling of a pending timer to a later time, which is necessary when the task queue becomes empty or the next task is further in the future. For a task runner that schedules based on the next deadline, the timer should always be set to the newly calculated time.

The comment for ScheduleAt should also be updated to reflect this change, for example: "Schedules the callback to be called at the specified time point, replacing any previously scheduled time."

// Schedules the callback to be called at specified time point, replacing any // previously scheduled time. void ScheduleAt( std::chrono::time_point<std::chrono::high_resolution_clock> time_point) { { std::lock_guard<std::mutex> lock(mutex_); next_fire_time_ = time_point; ++schedule_counter_; cv_.notify_all(); }

No. The behavior is on purpose. If anything the name of the function may need to be changed to reflect that.

Added test to ensure this works as expected.

engine/src/flutter/shell/platform/windows/task_runner_window.cc

filiph · 2025-12-01T18:06:59Z

Just curious: is this issue (#178916) something that might be afflicting Linux as well?

engine/src/flutter/shell/platform/windows/task_runner_unittests.cc

loic-sharma · 2025-12-01T19:41:45Z

engine/src/flutter/shell/platform/windows/task_runner_window.cc

+  cv_.notify_all();
+}
+
+void TimerThread::TimerThreadMain() {


Should we be setting a thread priority here?

I'm not sure really. I don't know much about windows scheduler, even with the default values I'm getting sub-ms accuracy, but maybe the situation is worse with less CPU cores.

I'm leaning towards yes...

We could also use Multimedia Class Scheduler Service for the thread scheduling. This might need some experimenting. But maybe we can do that as a follow-up PR.

We use "above normal" for the UI and raster threads: flutter/engine#31778

I'd be OK with skipping thread priority for now and adding this in later if/when we find cases where this improves things.

knopp · 2025-12-01T19:42:01Z

Just curious: is this issue (#178916) something that might be afflicting Linux as well?

Not really. Linux has it's own set of issues, mostly related to Gtk3 gl contexts not being shared and the disconnect between how Gtk wants to drive the rendering versus what Flutter expects (vsync waiter). Especially on higher refresh rates we're not in a good place. I have a prototype somewhere that completely bypasses Gtk rendering using wayland subsurfaces, but it needs a lot more time than I can afford right now :)

engine/src/flutter/shell/platform/windows/task_runner_window.cc

…s.cc Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

loic-sharma

Thanks for the excellent fix!

damywise · 2025-12-02T09:17:31Z

Can confirm this works.
Before:

multiple_windows_WUSfOBCR05.mp4

After:

multiple_windows_UbFcYDJVQy.mp4

…ead timer (flutter/flutter#179249)

…d timer (flutter/flutter#179249)

flutter/flutter@05d6005...5545bb3 2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 4371ed0ce49e to 45337c4e919d (1 revision) (flutter/flutter#179342) 2025-12-02 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from sTk6OB7a4yudbfdZg... to l0DvmZrMHlF12frrX... (flutter/flutter#179338) 2025-12-02 huy@nevercode.io Unfocus search anchor bar when the view is closed (flutter/flutter#178910) 2025-12-02 sstrickl@google.com Directly generate a Mach-O dynamic library using gen_snapshot. [reland] (flutter/flutter#174870) 2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 1fc59bf5cbb1 to 4371ed0ce49e (3 revisions) (flutter/flutter#179326) 2025-12-02 matej.knopp@gmail.com [win32] Replace threadpool timer with custom background thread timer (flutter/flutter#179249) 2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 61257a1036fb to 1fc59bf5cbb1 (1 revision) (flutter/flutter#179321) 2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from ef52cf952211 to 61257a1036fb (2 revisions) (flutter/flutter#179319) 2025-12-02 engine-flutter-autoroll@skia.org Roll Skia from 8887653a773e to ef52cf952211 (1 revision) (flutter/flutter#179316) 2025-12-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#179313) 2025-12-02 katelovett@google.com Update customer tests (flutter/flutter#179309) 2025-12-01 fluttergithubbot@gmail.com Marks Linux_pixel_7pro new_gallery__transition_perf to be unflaky (flutter/flutter#176339) 2025-12-01 feinstein@users.noreply.github.com Fix typo (flutter/flutter#179200) 2025-12-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 5.0.1 to 6.0.0 in the all-github-actions group (flutter/flutter#179308) 2025-12-01 engine-flutter-autoroll@skia.org Roll Dart SDK from c54108eeb2c1 to eb743a1d4ade (1 revision) (flutter/flutter#179304) 2025-12-01 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#179280) 2025-12-01 engine-flutter-autoroll@skia.org Roll Skia from 68cc3257e734 to 8887653a773e (4 revisions) (flutter/flutter#179302) 2025-12-01 97480502+b-luk@users.noreply.github.com Support round caps for the fast arc stroke generator (flutter/flutter#178269) 2025-12-01 31510811+jwlilly@users.noreply.github.com Fix for PR #174374 - Fix - TalkBack does not announce list information (flutter/flutter#177622) 2025-12-01 116356835+AbdeMohlbi@users.noreply.github.com Small cleanup in `‎AccessibilityBridge.java‎` (flutter/flutter#179226) 2025-12-01 engine-flutter-autoroll@skia.org Roll Skia from 925c311f4b37 to 68cc3257e734 (44 revisions) (flutter/flutter#179294) 2025-12-01 bkonyi@google.com [ Widget Preview ] Ignore changes under `ios/.symlinks` (flutter/flutter#179290) 2025-12-01 jesswon@google.com Delete unecessary lockfile (flutter/flutter#179052) 2025-12-01 matt.kosarek@canonical.com Resolving and piping the view ID through the WidgetController and the TestPointer so that clicks wind up on the right view (flutter/flutter#178941) 2025-12-01 116356835+AbdeMohlbi@users.noreply.github.com Fix link specified as plain text `FlutterApplication.java‎` (flutter/flutter#178573) 2025-12-01 bruno.leroux@gmail.com Update some comments to reflect theme normalization (flutter/flutter#179013) 2025-12-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 51fe8cd01fbe to c54108eeb2c1 (1 revision) (flutter/flutter#179267) 2025-12-01 bungeman@chromium.org Explicitly use FreeType font scanner with Fuchsia (flutter/flutter#179055) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

krll-kov · 2025-12-02T17:56:43Z

Can confirm this works. Before:

multiple_windows_WUSfOBCR05.mp4
After:

multiple_windows_UbFcYDJVQy.mp4

What was your fps before 3.38 release? also 70-90?

…lutter#179249) Fixes flutter#178916 It seems that for some reason the win32 thread pool timer is not precise enough for our use case which causes major frame pacing issues on Windows. This PR replaces it with a simple custom built background thread timer. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

Predidit · 2025-12-03T08:29:03Z

Excuse me, could this fix be cherry-picked to the stable branch?

knopp · 2025-12-03T19:29:35Z

The fix here might have regressed #173843. With the new timer implementation now actually working as expected sometimes the one millisecond delay is not enough to for other windows messages to be processed :-/

krll-kov · 2025-12-03T20:34:46Z

The fix here might have regressed #173843. With the new timer implementation now actually working as expected sometimes the one millisecond delay is not enough to for other windows messages to be processed :-/

So there's no sense in this fix because it does not do anything anymore?

knopp · 2025-12-03T20:48:17Z

No. This PR fixes the framerate. But in some cases

Future<void> _startEvilLoop() async {
    // Spam the Dart event loop with tasks.
    // This isn't ideal, but the app should continue to work.
    while (true) {
      await Future.delayed(Duration.zero);
    }
  }

Might lock up the application. Not that is a good idea to do this.

The initial workaround for the bug worked because the timer delay was rather big, which causes the framerate issue but at the same time avoided draining dart run loop too fast. Now that the framerate issue is fixed, we drain the dart event loop too fast taking precedence over win32 messages. To fix this with new timer we need better solution.

krll-kov · 2025-12-03T20:52:43Z

No. This PR fixes the framerate. But in some cases
Future<void> _startEvilLoop() async {
    // Spam the Dart event loop with tasks.
    // This isn't ideal, but the app should continue to work.
    while (true) {
      await Future.delayed(Duration.zero);
    }
  }
Might lock up the application. Not that is a good idea to do this.

The initial workaround for the bug worked because the timer delay was rather big, which causes the framerate issue but at the same time avoided draining dart run loop too fast. Now that the framerate issue is fixed, we drain the dart event loop too fast taking precedence over win32 messages. To fix this with new timer we need better solution.

That's what I mean, without timer and with it, the outcome is the same , that original issue with app freeze during loops with await of 0ms is still present, so it's presence seems pointless now

…lutter#179249) Fixes flutter#178916 It seems that for some reason the win32 thread pool timer is not precise enough for our use case which causes major frame pacing issues on Windows. This PR replaces it with a simple custom built background thread timer. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

This comment was marked as outdated.

Sign in to view

github-actions bot added engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop labels Nov 30, 2025

gemini-code-assist bot reviewed Nov 30, 2025

View reviewed changes

knopp added 3 commits November 30, 2025 20:47

[win32] Replace threadpool timer with custom background thread timer

63ab373

Gemini feedback

77c1004

Refactor and add test

92327ba

knopp force-pushed the win_fix_framerate branch from 2502074 to 92327ba Compare November 30, 2025 19:48

knopp requested a review from loic-sharma November 30, 2025 20:58

krll-kov mentioned this pull request Dec 1, 2025

Frame Request Pending takes 16ms despite CPU being underutilized #179265

Closed

1 task

Rmove uncecessary braces

f17d7c7

loic-sharma reviewed Dec 1, 2025

View reviewed changes

engine/src/flutter/shell/platform/windows/task_runner_unittests.cc Outdated Show resolved Hide resolved

loic-sharma reviewed Dec 1, 2025

View reviewed changes

engine/src/flutter/shell/platform/windows/task_runner_window.cc Show resolved Hide resolved

knopp and others added 2 commits December 1, 2025 20:56

Remove timeEndPeriod

a0157f4

Update engine/src/flutter/shell/platform/windows/task_runner_unittest…

b1a378a

…s.cc Co-authored-by: Loïc Sharma <737941+loic-sharma@users.noreply.github.com>

loic-sharma approved these changes Dec 1, 2025

View reviewed changes

knopp added this pull request to the merge queue Dec 2, 2025

Merged via the queue into flutter:master with commit 8eab409 Dec 2, 2025
180 checks passed

knopp deleted the win_fix_framerate branch December 2, 2025 09:05

engine-flutter-autoroll mentioned this pull request Dec 2, 2025

Roll Flutter from 05d6005dd85c to 14063cfd0bc1 (26 revisions) flutter/packages#10548

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025

8eab409 [win32] Replace threadpool timer with custom background threa…

8b467e0

…ead timer (flutter/flutter#179249)

engine-flutter-autoroll mentioned this pull request Dec 2, 2025

Roll Flutter from 05d6005dd85c to 271eb460369e (27 revisions) flutter/packages#10550

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025

8eab409 [win32] Replace threadpool timer with custom background threa…

61e7d22

…d timer (flutter/flutter#179249)

engine-flutter-autoroll mentioned this pull request Dec 2, 2025

Roll Flutter from 05d6005dd85c to 5545bb397e7b (28 revisions) flutter/packages#10551

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025

8eab409 [win32] Replace threadpool timer with custom background threa…

0ef1c11

…d timer (flutter/flutter#179249)

loic-sharma mentioned this pull request Dec 2, 2025

Windows significant performance regression after update from 3.35.7 to 3.38.2 #178916

Open

knopp mentioned this pull request Dec 9, 2025

[win32] Do not flood message loop with wake up messages #179538

Merged

9 tasks

Ferry-200 mentioned this pull request Dec 16, 2025

[coriander] 修复Windows全屏冻结和窗口尺寸记忆问题 Ferry-200/coriander_player#235

Merged

6 tasks

[win32] Replace threadpool timer with custom background thread timer #179249

[win32] Replace threadpool timer with custom background thread timer #179249

Uh oh!

Conversation

knopp commented Nov 30, 2025

Pre-launch Checklist

Uh oh!

This comment was marked as outdated.

knopp commented Nov 30, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

knopp Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

knopp Nov 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

filiph commented Dec 1, 2025

Uh oh!

Uh oh!

loic-sharma Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

knopp Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

knopp Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

knopp Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

loic-sharma Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

knopp commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

damywise commented Dec 2, 2025

Uh oh!

krll-kov commented Dec 2, 2025

Uh oh!

Predidit commented Dec 3, 2025

Uh oh!

knopp commented Dec 3, 2025

Uh oh!

krll-kov commented Dec 3, 2025

Uh oh!

knopp commented Dec 3, 2025

Uh oh!

krll-kov commented Dec 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

knopp commented Dec 1, 2025 •

edited

Loading