engine

engine copied to clipboard

Original reland was reverted because of skiaperf regressions. The regressions are caused by runner machines running display at 30hz.

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

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

Hold off for a little bit on the reland while we see if we can verify frame timings on the bot manually. We're getting conflicting info on the refresh rate for their bot pool, and need to go through Chrome Ops to verify the machines in the pool the affected test runs on. Will reply as soon as I've got confirmation.

We could maybe add conditional logging (i.e. enabled through Info.plist) to see what ticks we're getting from CVDisplayLink.

Or even just a "test" that always passes but collects ~10 frame intervals and logs min, average, max?

Or even just a "test" that always passes but collects ~10 frame intervals and logs min, average, max?

I like that. Added FlutterDisplayLinkTest.CVDisplayLinkInterval.

Setting as WIP until I figure out what's going on with average_frame_request_pending_latency.

Update: This is currently blocked on Infra. I've provided a test application which should help us determine whether the regression is caused by the PR or runner.

Yep, as mentioned on Discord, I think we should re-land this. I don't trust our benchmarking on this one at all and looking into it is hugely time-consuming given the back and forth between us, our infra team, and Chrome's (who owns the bots). I'm fine with us disabling the benchmark (or possibly re-baselining it though not sure how useful that even is).

I think I'd just let it regress from now, ask folks to try it out and see. Possibly the apps are getting treated as "backgrounded" by the OS and so they are correctly running at 30fps. Or maybe its just cursed.

The clock issue is fixed in this PR. It properly converts CAMediaTime to engine time and back.