Expose NotifyIdle from RuntimeController to Dart, allowing flutter_smooth to get 60FPS, even if GC needs to run for 14ms per 16.67ms

[fzyzcjy]

1. I will finish code details, refine code, add tests, make tests pass, etc, after a code review that thinks the rough idea is acceptable. It is because, from my past experience, reviews may request changing a lot. If the general idea is to be changed, all detailed implementation efforts are wasted :)
2. The PR has an already-working counterpart, and it produces ~60FPS smooth experimental results. The benchmark results and detailed analysis is in chapter https://cjycode.com/flutter_smooth/benchmark/. All the source code is in https://github.com/fzyzcjy/engine/tree/flutter-smooth and https://github.com/fzyzcjy/flutter/tree/flutter-smooth.
3. Possibly useful as a context to this PR, there is a whole chapter discussing the internals - how flutter_smooth is implemented. (Link: https://cjycode.com/flutter_smooth/design/)

---

The PR simply exposes RuntimeController::NotifyIdle to the dart layer.

It is necessary for https://github.com/fzyzcjy/flutter_smooth because of the following commonly seen scenario: Suppose we are in a janky frame (say it takes 200ms). Then, NotifyIdle is never called at all, because it is usually called at the end of DrawFrame (indeed, more exactly, in the Animator::AwaitVSync). Then, during the 200ms, garbage accumulates, and at one time the young generation is full, then Dart VM must stop the world and make a GC. From my experiments, such GC can even take 20ms on my testing device. Stop the world for 20ms - then we must miss one frames, causing non-60FPS. Even if the stop-the-world GC is fast, say, 5ms, it can still cause a jank. For example, when it happens at 97.5ms-102.5ms, then the preemptRender which should originally be done near 98-100ms can only be done at 105ms, so it calls window.render too late, thus the rasterizer may fail to rasterize the frame before the 116.67ms vsync, so there is a jank. (If needed, I can draw a figure).

However, with this PR, there is no such problem at all. The flutter_smooth will call NotifyIdle immediately after each and every preemptRender, with a deadline of roughly 14ms (16.67ms minus a few ms). By doing this, there are two benefits. Firstly, since the heap is not that full, GC can finish its work sooner instead of the 20ms bad case when the heap is really full. This avoids the 20ms-long-GC problem above. Secondly, since we actively tell Dart VM that it can start a GC at this time, GC can run for a time duration as long as ~14ms without causing any jank. This is contrary to the discussion above, where even a 5ms GC can cause a frame jank. As for why it can run 14ms without causing trouble, it is because, suppose we start it at 100ms and it runs 14ms, then we are now at 114ms, and we start preemptRender. Since preemptRender is really fast (e.g. 2ms), we will submit window.render at 116ms. In other words, we submit window.render with sufficient time left for rasterizer to finish its job - as long as rasterizer finishes its job before 133.33ms, no jank will happen.

Therefore, the title is explained well: It allows flutter_smooth to get 60FPS, even if GC needs to run for 14ms per 16.67ms. (That extreme GC will not happen in real world, I just want to say this proposal works even for that.)

I have already done that for my engine branch and ran experiments on flutter_smooth. It works pretty well - originally I observe GC-caused janks and then they disappear after this fix. If you are interested I can present some data.

As for tests: Have not found a way to add tests for this very simple calling, may need a test exempt.

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 and the C++, Objective-C, Java style guides.
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
  writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the CLA.
• All existing and new tests are passing.

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

[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]

We don't want to expose this kind of internal of the VM to users.

Perhaps it would be more appropriate to expose a different API around whether an animation is occurring currently, although I think @iskakaushik has already done some work on that.

I'm going to close this to get it off of review queues for now, since in its current state it would not land.

[fzyzcjy]

Thanks for the reply, I will ask on discord later

[fzyzcjy]

Hi @iskakaushik may I know what previous work you have done? Thanks

[fzyzcjy]

We don't want to expose this kind of internal of the VM to users.

Hmm NotifyIdle seems very generic - there seems no VM internals, but only "hey I am idle for a period of time". It is not something like NotifyDartToDoYoungGC which is internals :) But instead, it is a well-defined interface so dev can tell flutter it is free.

Perhaps it would be more appropriate to expose a different API around whether an animation is occurring currently, although I think @iskakaushik has already done some work on that.

Sorry but I do not quite get it. In flutter_smooth (detailed design: https://cjycode.com/flutter_smooth/design/), a janky frame can last for a long time (for convenience of description, suppose 100ms). Then, during this whole period, the Dart in UI thread will be busy, so it is impossible to execute any C++ code unless Dart code explicitly do so. With this PR, flutter_smooth will explictly call notifyIdle with an explicit deadline of ~14ms (much longer than classical Flutter). This is a bit like the pull vs push model problem.