TaskSources register tasks with MessageLoopTaskQueues dispatcher #25307

iskakaushik · 2021-03-30T18:03:39Z

Task Source

Introduces a notion of a TaskSource. TaskSource provides the task dispatcher (MessageLoopTaskQueues) an abstraction to obtain the next task to run. TaskSources as implemented in the PR have two managed task heaps. kPrimary and kSecondary task heaps. When registering a task with the task dispatcher, we provide a way to specify whether the task is for the kPrimiary or kSecondary task heap. Secondary heaps can be paused and resumed.

Usage

During a frame workload, on vsync, we pause the secondary heap on the UI thread's task source. This is later resumed on the completion of the frame build process.
As a part of this change, all the HandleMessage calls on a dart isolate are also serviced by the secondary heap of the UI thread's task source.

This makes it so that all non-frame build related events on the UI threads are paused on vsync and we re enable them only when the frame gets built. See: go/flutter-engine-task-prioritization.

Benchmarks

lands the framework to address: flutter/flutter#69694

flutter-dashboard · 2021-03-30T18:03:41Z

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.

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.

iskakaushik · 2021-03-30T18:06:50Z

@chinmaygarde this PR is mostly ready, I still need to write tests for this, wanted to get your take on this before I go ahead with it.

chinmaygarde

This is extremely compelling stuff. I did an initial pass over the interface with some suggestions for your consideration. Looking at the implementation in detail now. Thanks!

fml/task_source.h

fml/task_queue_id.h

fml/task_source.h

chinmaygarde · 2021-04-12T18:53:09Z

fml/task_source.h

+
+  void Clear();
+
+  void RegisterTask(TaskSourceGrade grade, DelayedTask&& task);


Can DelayedTask have its copy ctor disabled? We can then remove the && and ensure that it is impossible to have unnecessary copies.

I tried doing this, looks like using a priority queue without a copy constructor is non-trivial. Will attempt doing this in a future PR.

fml/task_source.h

chinmaygarde

This is so awesome.

chinmaygarde · 2021-04-15T22:04:08Z

fml/task_source.cc

+  switch (task.GetTaskSourceGrade()) {
+    case TaskSourceGrade::kUserInteraction:
+      primary_task_queue_.push(task);
+      break;


Nit: Here and later, you can use [[fallthrough]] if the compiler troubles you about the missing break. You call though.

DartHandle messages will be held

1. Add tests for TaskSource 2. Add docs for TaskSource 3. Get existing failing tests to pass

iskakaushik · 2021-04-19T14:19:32Z

@chinmaygarde I've noticed a couple of assumptions in the profiler initialization code that prevents us from pausing the micro task queue on vsync start. We have to do it after the first frame renders. I'm landing the changes to the task management as done in this PR. I will address these issues (in a much smaller patch) in a follow-up PR.

…her (flutter/engine#25307)

Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

…her (flutter/engine#25307)

chinmaygarde · 2021-04-19T20:22:06Z

I'm landing the changes to the task management as done in this PR.

SGTM.

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

zanderso · 2021-04-20T16:31:12Z

Does this also close flutter/flutter#69694?

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik · 2021-04-20T16:32:07Z

@zanderso not yet, this lands the framework to prioritize tasks. I did not prioritize the tasks just yet. That's WIP at #25653.

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

…her (flutter#25307)" This reverts commit f2f09b6.

…tter#25307)

iskakaushik requested a review from chinmaygarde March 30, 2021 18:03

googlebot added the cla: yes label Mar 30, 2021

iskakaushik force-pushed the event_loop_scheduling branch from ed21997 to c74d60b Compare March 30, 2021 18:05

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Apr 1, 2021

iskakaushik force-pushed the event_loop_scheduling branch from c74d60b to 934a678 Compare April 12, 2021 16:58

iskakaushik changed the title ~~[WIP] Improve the scheduling of events on the UI thread~~ Improve the scheduling of events on the UI thread Apr 12, 2021

flutter-dashboard bot added the embedder Related to the embedder API label Apr 12, 2021

iskakaushik removed the Work in progress (WIP) Not ready (yet) for review! label Apr 12, 2021

iskakaushik force-pushed the event_loop_scheduling branch from 237f4a9 to 6e07478 Compare April 12, 2021 17:50

chinmaygarde reviewed Apr 12, 2021

View reviewed changes

chinmaygarde approved these changes Apr 15, 2021

View reviewed changes

iskakaushik added 5 commits April 16, 2021 09:43

[tasks] Add a concept of primary and secondary queues in task source

10d5bcc

DartHandle messages will be held

Add a test and fix licenses

ce2efd2

Address comments modulo:

46746af

1. Add tests for TaskSource 2. Add docs for TaskSource 3. Get existing failing tests to pass

docs for task source

d9f8da7

add tests for task_sourcd

88bed7b

iskakaushik force-pushed the event_loop_scheduling branch from 2dd11ca to f49ec77 Compare April 16, 2021 16:43

rebase

7795d8d

iskakaushik force-pushed the event_loop_scheduling branch from f49ec77 to 7795d8d Compare April 16, 2021 18:41

fix thread local

35cc7b2

iskakaushik changed the title ~~Improve the scheduling of events on the UI thread~~ TaskSources register tasks with MessageLoopTaskQueues dispatcher Apr 19, 2021

iskakaushik merged commit f2f09b6 into flutter:master Apr 19, 2021

iskakaushik deleted the event_loop_scheduling branch April 19, 2021 14:20

engine-flutter-autoroll mentioned this pull request Apr 19, 2021

Roll Engine from 60f12daf8502 to f2f09b696914 (3 revisions) flutter/flutter#80730

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2021

f2f09b6 TaskSources register tasks with MessageLoopTaskQueues dispatc…

6f77f8e

…her (flutter/engine#25307)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 19, 2021

Start microtasks after first frame is scheduled

fe0e1e4

Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik mentioned this pull request Apr 19, 2021

Start microtasks only in non-test envs #25653

Merged

engine-flutter-autoroll mentioned this pull request Apr 19, 2021

Roll Engine from 60f12daf8502 to 7f4d7c67a5a1 (5 revisions) flutter/flutter#80745

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 19, 2021

f2f09b6 TaskSources register tasks with MessageLoopTaskQueues dispatc…

b2f9d5b

…her (flutter/engine#25307)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

0594cc1

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

da400da

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik mentioned this pull request Apr 20, 2021

Improve engine event loop task prioritization flutter/flutter#76570

Closed

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

3bbab63

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

ebb6ab7

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

a0ce6e0

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 20, 2021

Start microtasks only for non-tests

20d6b1c

This is because tests don't really wait for vsync to complete Fixes: flutter/flutter#69694 See: flutter#25307 (comment)

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 21, 2021

Revert "TaskSources register tasks with MessageLoopTaskQueues dispatc…

d764314

…her (flutter#25307)" This reverts commit f2f09b6.

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 21, 2021

Revert "TaskSources register tasks with MessageLoopTaskQueues dispatc…

c6680f8

…her (flutter#25307)" This reverts commit f2f09b6.

iskakaushik added a commit to iskakaushik/engine that referenced this pull request Apr 21, 2021

TaskSources register tasks with MessageLoopTaskQueues dispatcher (flu…

6af5c77

…tter#25307)

GuoXi pushed a commit to GuoXi/engine that referenced this pull request Apr 25, 2021

TaskSources register tasks with MessageLoopTaskQueues dispatcher (flu…

2413590

…tter#25307)


		void Clear();

		void RegisterTask(TaskSourceGrade grade, DelayedTask&& task);

TaskSources register tasks with MessageLoopTaskQueues dispatcher #25307

TaskSources register tasks with MessageLoopTaskQueues dispatcher #25307

Uh oh!

Conversation

iskakaushik commented Mar 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Task Source

Usage

Benchmarks

Uh oh!

flutter-dashboard bot commented Mar 30, 2021

Uh oh!

iskakaushik commented Mar 30, 2021

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chinmaygarde Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

iskakaushik Apr 13, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Apr 15, 2021

Choose a reason for hiding this comment

Uh oh!

iskakaushik commented Apr 19, 2021

Uh oh!

chinmaygarde commented Apr 19, 2021

Uh oh!

zanderso commented Apr 20, 2021

Uh oh!

iskakaushik commented Apr 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iskakaushik commented Mar 30, 2021 •

edited

Loading

iskakaushik commented Apr 20, 2021 •

edited

Loading