Fix janks caused by await vsync in classical Flutter (V2)

[fzyzcjy]

Previous: #36911

(Since the previous one is already closed and I have no permission to reopen, I guess I should create a new issue such that it can be put in the review queue)

As @dnfield points out in #36911 (comment):

we need some motivating benchmarks that clearly show what benefits would be gained here, and we need to see that changes to this won't negatively impact existing benchmarks/users.

Thus, reproduction is shown in the section below. As for existing benchmarks, could you please give some hints? I guess I have no permission to execute skia perf as it seems only run on master or other flutter branches. Thus, maybe I can do nothing except for creating a PR and see skia perf after it is merged...

Reproduction

Setup and analysis

In the benchmark, I deliberately make a dead loop to mimic the case when ui thread runs for smaller than but very close to 16.67ms. This is quite hacky and may not work on your device, so please modify the hardcoded time in the code if you cannot reproduce it. But hopefully this is ok as a "motivating" benchmark you mentioned :)

I also realize this seems hard to reproduce if the await vsync is called per 16ms. Instead, when I make ui thread deliberately janky (e.g. ~100ms), so that await vsync is not called for ~100ms, this is reproduced. Not sure why, but anyway regardless of why this happens, the PR can solve it.

In the screenshots below, I add red vertical lines manually by mimicking the VSYNC interval (this is the correct interval that I have fixed previously, so can use it). As we can see, the AwaitVsync is called before the vsync (which is imaginary and not shown in figure), but we miss it for a whole 16.67ms.

Data

Code: https://github.com/fzyzcjy/flutter/tree/feat/await-vsync-directly-call + standard engine build (because this is reproduction not bugfix)
Run it: /path/to/flutter drive --profile -t test_driver/run_app.dart --driver test_driver/near_full_tim_perf_test.dart

Sample result timeline json:
near_full_time_perf.timeline.json.zip

Sample screenshot:

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[dnfield]

Part of the work to make this benchmark would be coming up with an at least somewhat realistic case where UI thread work goes to just over vsync budget. Yes, you can construct a benchmark that does exactly that, but what's an example of a real application that actually behaves that way on a real device? IME, it's more common to see an application that far exceeds budget on some frames and then is under budget on many other frames, rather than one that is consistently just slightly over budget. In fact, if you had one that was consistently over budget, you should instead look to optimize your consistent workload to be less - it's an application problem rather than an engine one. But right now, we don't have good ways to fix the problem where some frames over budget despite the fact that you're not doing anything all that unreasonble - for example, oyu're showing a screen full of text that can't all do its initial layout within frame budget on your target device.

You should be able to run the devicelab benchmarks locally and do experiments before and after -see https://github.com/flutter/flutter/blob/master/dev/devicelab/README.md

I'm closing this PR for the same reasons as the previous PR right now - it's not in a state that is ready for review, and much of this content might be more appropriate for an issue right now.

[fzyzcjy]

@dnfield

Yes, you can construct a benchmark that does exactly that, but what's an example of a real application that actually behaves that way on a real device? IME, it's more common to see an application that far exceeds budget on some frames and then is under budget on many other frames, rather than one that is consistently just slightly over budget.

IMHO it is more like a math/statistics problem than a programming problem. Using my observation from previous issue (this problem happens as long as 2ms before deadline), and a vsync interval is 16.67ms. Then, for a very rough estimation, suppose the real time used is uniform from shortest time to longest time possible - this is very rough, but it somehow makes sense because an app targets from highest-end to lowest-end devices. Then, we have 2/16.67 = 12% probability. Surely that does not sound huge, but it does happen and affect user feeling. Indeed, my flutter_smooth's goal is 60FPS (on 60FPS machine) and this single problem drops 7.2FPS if using the above very rough analysis - and in reality I roughly see more drops.

So, answering "it's more common to see..." - surely yes, but Flutter is a high-performance framework and wants to be 60FPS, instead of a framework that allows jank from time to time ;)

In fact, if you had one that was consistently over budget, you should instead look to optimize your consistent workload to be less - it's an application problem rather than an engine one

By the way, this bug is not only about "over budget" - if ui thread runs over 16.67ms (on 60hz machine) one can never get 60FPS (but can still be 60FPS with flutter_smooth - that's another story). In addition, a frame that is computed quicker than 16.67ms in one phone may be slower than 16.67ms on another lower end phone.

But right now, we don't have good ways to fix the problem where some frames over budget despite the fact that you're not doing anything all that unreasonble - for example, oyu're showing a screen full of text that can't all do its initial layout within frame budget on your target device.

Indeed we have - flutter_smooth :) That example is indeed the first example that I have solved when prototyping (btw thanks for providing that example as a canonical test base!).
(As long as all PRs are merged after discussions and modifications)

You should be able to run the devicelab benchmarks locally and do experiments before and after -see flutter/flutter@master/dev/devicelab/README.md
I'm closing this PR for the same reasons as the previous PR right now - it's not in a state that is ready for review, and much of this content might be more appropriate for an issue right now.

I see, thanks (originally I used macrobenchmark folder directly)