From 10d5bcc73316c37bab49307d33241d1244fa4321 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 29 Mar 2021 15:11:44 -0700 Subject: [PATCH 1/7] [tasks] Add a concept of primary and secondary queues in task source DartHandle messages will be held --- fml/BUILD.gn | 3 + fml/message_loop_task_queues.cc | 92 ++++++++++++++++++------------ fml/message_loop_task_queues.h | 35 ++++++------ fml/task_queue_id.h | 26 +++++++++ fml/task_source.cc | 99 +++++++++++++++++++++++++++++++++ fml/task_source.h | 61 ++++++++++++++++++++ runtime/dart_isolate.cc | 15 +++-- shell/common/vsync_waiter.cc | 8 ++- 8 files changed, 280 insertions(+), 59 deletions(-) create mode 100644 fml/task_queue_id.h create mode 100644 fml/task_source.cc create mode 100644 fml/task_source.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 9eaccd086207a..a11defe924c9b 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -70,8 +70,11 @@ source_set("fml") { "synchronization/sync_switch.h", "synchronization/waitable_event.cc", "synchronization/waitable_event.h", + "task_queue_id.h", "task_runner.cc", "task_runner.h", + "task_source.cc", + "task_source.h", "thread.cc", "thread.h", "thread_local.cc", diff --git a/fml/message_loop_task_queues.cc b/fml/message_loop_task_queues.cc index e1c59c9190909..4fb0cec8367e5 100644 --- a/fml/message_loop_task_queues.cc +++ b/fml/message_loop_task_queues.cc @@ -7,6 +7,7 @@ #include "flutter/fml/message_loop_task_queues.h" #include +#include #include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop_impl.h" @@ -19,11 +20,13 @@ const size_t TaskQueueId::kUnmerged = ULONG_MAX; fml::RefPtr MessageLoopTaskQueues::instance_; -TaskQueueEntry::TaskQueueEntry() - : owner_of(_kUnmerged), subsumed_by(_kUnmerged) { +TaskQueueEntry::TaskQueueEntry(TaskQueueId created_for_arg) + : owner_of(_kUnmerged), + subsumed_by(_kUnmerged), + created_for(created_for_arg) { wakeable = NULL; task_observers = TaskObservers(); - delayed_tasks = DelayedTaskQueue(); + task_source = std::make_unique(created_for); } fml::RefPtr MessageLoopTaskQueues::GetInstance() { @@ -38,7 +41,7 @@ TaskQueueId MessageLoopTaskQueues::CreateTaskQueue() { std::lock_guard guard(queue_mutex_); TaskQueueId loop_id = TaskQueueId(task_queue_id_counter_); ++task_queue_id_counter_; - queue_entries_[loop_id] = std::make_unique(); + queue_entries_[loop_id] = std::make_unique(loop_id); return loop_id; } @@ -63,24 +66,31 @@ void MessageLoopTaskQueues::DisposeTasks(TaskQueueId queue_id) { const auto& queue_entry = queue_entries_.at(queue_id); FML_DCHECK(queue_entry->subsumed_by == _kUnmerged); TaskQueueId subsumed = queue_entry->owner_of; - queue_entry->delayed_tasks = {}; + queue_entry->task_source->Clear(); if (subsumed != _kUnmerged) { - queue_entries_.at(subsumed)->delayed_tasks = {}; + queue_entries_.at(subsumed)->task_source->Clear(); } } -void MessageLoopTaskQueues::RegisterTask(TaskQueueId queue_id, - const fml::closure& task, - fml::TimePoint target_time) { +void MessageLoopTaskQueues::RegisterTask( + TaskQueueId queue_id, + const fml::closure& task, + fml::TimePoint target_time, + fml::TaskSourceGrade task_source_grade) { std::lock_guard guard(queue_mutex_); size_t order = order_++; const auto& queue_entry = queue_entries_.at(queue_id); - queue_entry->delayed_tasks.push({order, task, target_time}); + queue_entry->task_source->RegisterTask(task_source_grade, + {order, task, target_time}); TaskQueueId loop_to_wake = queue_id; if (queue_entry->subsumed_by != _kUnmerged) { loop_to_wake = queue_entry->subsumed_by; } - WakeUpUnlocked(loop_to_wake, GetNextWakeTimeUnlocked(loop_to_wake)); + + // This can happen when the secondary tasks are paused. + if (HasPendingTasksUnlocked(loop_to_wake)) { + WakeUpUnlocked(loop_to_wake, GetNextWakeTimeUnlocked(loop_to_wake)); + } } bool MessageLoopTaskQueues::HasPendingTasks(TaskQueueId queue_id) const { @@ -94,8 +104,7 @@ fml::closure MessageLoopTaskQueues::GetNextTaskToRun(TaskQueueId queue_id, if (!HasPendingTasksUnlocked(queue_id)) { return nullptr; } - TaskQueueId top_queue = _kUnmerged; - const auto& top = PeekNextTaskUnlocked(queue_id, top_queue); + TaskSource::TopTask top = PeekNextTaskUnlocked(queue_id); if (!HasPendingTasksUnlocked(queue_id)) { WakeUpUnlocked(queue_id, fml::TimePoint::Max()); @@ -103,11 +112,12 @@ fml::closure MessageLoopTaskQueues::GetNextTaskToRun(TaskQueueId queue_id, WakeUpUnlocked(queue_id, GetNextWakeTimeUnlocked(queue_id)); } - if (top.GetTargetTime() > from_time) { + if (top.task.GetTargetTime() > from_time) { return nullptr; } - fml::closure invocation = top.GetTask(); - queue_entries_.at(top_queue)->delayed_tasks.pop(); + fml::closure invocation = top.task.GetTask(); + queue_entries_.at(top.task_queue_id) + ->task_source->PopTask(top.task_source_grade); return invocation; } @@ -126,12 +136,12 @@ size_t MessageLoopTaskQueues::GetNumPendingTasks(TaskQueueId queue_id) const { } size_t total_tasks = 0; - total_tasks += queue_entry->delayed_tasks.size(); + total_tasks += queue_entry->task_source->Size(); TaskQueueId subsumed = queue_entry->owner_of; if (subsumed != _kUnmerged) { const auto& subsumed_entry = queue_entries_.at(subsumed); - total_tasks += subsumed_entry->delayed_tasks.size(); + total_tasks += subsumed_entry->task_source->Size(); } return total_tasks; } @@ -248,6 +258,20 @@ TaskQueueId MessageLoopTaskQueues::GetSubsumedTaskQueueId( return queue_entries_.at(owner)->owner_of; } +void MessageLoopTaskQueues::PauseSecondarySource(TaskQueueId queue_id) { + std::lock_guard guard(queue_mutex_); + queue_entries_.at(queue_id)->task_source->PauseSecondary(); +} + +void MessageLoopTaskQueues::ResumeSecondarySource(TaskQueueId queue_id) { + std::lock_guard guard(queue_mutex_); + queue_entries_.at(queue_id)->task_source->ResumeSecondary(); + // Schedule a wake as needed. + if (HasPendingTasksUnlocked(queue_id)) { + WakeUpUnlocked(queue_id, GetNextWakeTimeUnlocked(queue_id)); + } +} + // Subsumed queues will never have pending tasks. // Owning queues will consider both their and their subsumed tasks. bool MessageLoopTaskQueues::HasPendingTasksUnlocked( @@ -258,7 +282,7 @@ bool MessageLoopTaskQueues::HasPendingTasksUnlocked( return false; } - if (!entry->delayed_tasks.empty()) { + if (!entry->task_source->Empty()) { return true; } @@ -267,37 +291,35 @@ bool MessageLoopTaskQueues::HasPendingTasksUnlocked( // this is not an owner and queue is empty. return false; } else { - return !queue_entries_.at(subsumed)->delayed_tasks.empty(); + return !queue_entries_.at(subsumed)->task_source->Empty(); } } fml::TimePoint MessageLoopTaskQueues::GetNextWakeTimeUnlocked( TaskQueueId queue_id) const { - TaskQueueId tmp = _kUnmerged; - return PeekNextTaskUnlocked(queue_id, tmp).GetTargetTime(); + return PeekNextTaskUnlocked(queue_id).task.GetTargetTime(); } -const DelayedTask& MessageLoopTaskQueues::PeekNextTaskUnlocked( - TaskQueueId owner, - TaskQueueId& top_queue_id) const { +TaskSource::TopTask MessageLoopTaskQueues::PeekNextTaskUnlocked( + TaskQueueId owner) const { FML_DCHECK(HasPendingTasksUnlocked(owner)); const auto& entry = queue_entries_.at(owner); const TaskQueueId subsumed = entry->owner_of; if (subsumed == _kUnmerged) { - top_queue_id = owner; - return entry->delayed_tasks.top(); + return entry->task_source->Top(); } - const auto& owner_tasks = entry->delayed_tasks; - const auto& subsumed_tasks = queue_entries_.at(subsumed)->delayed_tasks; + TaskSource* owner_tasks = entry->task_source.get(); + TaskSource* subsumed_tasks = queue_entries_.at(subsumed)->task_source.get(); // we are owning another task queue - const bool subsumed_has_task = !subsumed_tasks.empty(); - const bool owner_has_task = !owner_tasks.empty(); + const bool subsumed_has_task = !subsumed_tasks->Empty(); + const bool owner_has_task = !owner_tasks->Empty(); + fml::TaskQueueId top_queue_id = owner; if (owner_has_task && subsumed_has_task) { - const auto owner_task = owner_tasks.top(); - const auto subsumed_task = subsumed_tasks.top(); - if (owner_task > subsumed_task) { + const auto owner_task = owner_tasks->Top(); + const auto subsumed_task = subsumed_tasks->Top(); + if (owner_task.task > subsumed_task.task) { top_queue_id = subsumed; } else { top_queue_id = owner; @@ -307,7 +329,7 @@ const DelayedTask& MessageLoopTaskQueues::PeekNextTaskUnlocked( } else { top_queue_id = subsumed; } - return queue_entries_.at(top_queue_id)->delayed_tasks.top(); + return queue_entries_.at(top_queue_id)->task_source->Top(); } } // namespace fml diff --git a/fml/message_loop_task_queues.h b/fml/message_loop_task_queues.h index a555faf60e699..031a718f7f816 100644 --- a/fml/message_loop_task_queues.h +++ b/fml/message_loop_task_queues.h @@ -14,22 +14,12 @@ #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/synchronization/shared_mutex.h" +#include "flutter/fml/task_queue_id.h" +#include "flutter/fml/task_source.h" #include "flutter/fml/wakeable.h" namespace fml { -class TaskQueueId { - public: - static const size_t kUnmerged; - - explicit TaskQueueId(size_t value) : value_(value) {} - - operator int() const { return value_; } - - private: - size_t value_ = kUnmerged; -}; - static const TaskQueueId _kUnmerged = TaskQueueId(TaskQueueId::kUnmerged); // This is keyed by the |TaskQueueId| and contains all the queue @@ -39,7 +29,7 @@ class TaskQueueEntry { using TaskObservers = std::map; Wakeable* wakeable; TaskObservers task_observers; - DelayedTaskQueue delayed_tasks; + std::unique_ptr task_source; // Note: Both of these can be _kUnmerged, which indicates that // this queue has not been merged or subsumed. OR exactly one @@ -48,7 +38,9 @@ class TaskQueueEntry { TaskQueueId owner_of; TaskQueueId subsumed_by; - TaskQueueEntry(); + TaskQueueId created_for; + + explicit TaskQueueEntry(TaskQueueId created_for); private: FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TaskQueueEntry); @@ -77,9 +69,11 @@ class MessageLoopTaskQueues // Tasks methods. - void RegisterTask(TaskQueueId queue_id, - const fml::closure& task, - fml::TimePoint target_time); + void RegisterTask( + TaskQueueId queue_id, + const fml::closure& task, + fml::TimePoint target_time, + fml::TaskSourceGrade task_source_grade = fml::TaskSourceGrade::kPrimary); bool HasPendingTasks(TaskQueueId queue_id) const; @@ -126,6 +120,10 @@ class MessageLoopTaskQueues // otherwise. TaskQueueId GetSubsumedTaskQueueId(TaskQueueId owner) const; + void PauseSecondarySource(TaskQueueId queue_id); + + void ResumeSecondarySource(TaskQueueId queue_id); + private: class MergedQueuesRunner; @@ -137,8 +135,7 @@ class MessageLoopTaskQueues bool HasPendingTasksUnlocked(TaskQueueId queue_id) const; - const DelayedTask& PeekNextTaskUnlocked(TaskQueueId owner, - TaskQueueId& top_queue_id) const; + TaskSource::TopTask PeekNextTaskUnlocked(TaskQueueId owner) const; fml::TimePoint GetNextWakeTimeUnlocked(TaskQueueId queue_id) const; diff --git a/fml/task_queue_id.h b/fml/task_queue_id.h new file mode 100644 index 0000000000000..abf0e51e3f975 --- /dev/null +++ b/fml/task_queue_id.h @@ -0,0 +1,26 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_FML_TASK_QUEUE_ID_H_ +#define FLUTTER_FML_TASK_QUEUE_ID_H_ + +#include "flutter/fml/logging.h" + +namespace fml { + +class TaskQueueId { + public: + static const size_t kUnmerged; + + explicit TaskQueueId(size_t value) : value_(value) {} + + operator int() const { return value_; } + + private: + size_t value_ = kUnmerged; +}; + +} // namespace fml + +#endif // FLUTTER_FML_TASK_QUEUE_ID_H_ diff --git a/fml/task_source.cc b/fml/task_source.cc new file mode 100644 index 0000000000000..77dabcdbff3d2 --- /dev/null +++ b/fml/task_source.cc @@ -0,0 +1,99 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#define FML_USED_ON_EMBEDDER + +#include "flutter/fml/task_source.h" + +namespace fml { + +TaskSource::TaskSource(TaskQueueId task_queue_id) + : task_queue_id_(task_queue_id) {} + +TaskSource::~TaskSource() = default; + +void TaskSource::Clear() { + primary_task_queue_ = {}; + secondary_task_queue_ = {}; +} + +void TaskSource::RegisterTask(TaskSourceGrade grade, DelayedTask&& task) { + switch (grade) { + case TaskSourceGrade::kPrimary: + primary_task_queue_.push(task); + break; + case TaskSourceGrade::kSecondary: + secondary_task_queue_.push(task); + break; + } +} + +void TaskSource::PopTask(TaskSourceGrade grade) { + switch (grade) { + case TaskSourceGrade::kPrimary: + primary_task_queue_.pop(); + break; + case TaskSourceGrade::kSecondary: + secondary_task_queue_.pop(); + break; + } +} + +size_t TaskSource::Size() const { + size_t size = primary_task_queue_.size(); + if (secondary_pause_requests_ == 0) { + size += secondary_task_queue_.size(); + } + return size; +} + +bool TaskSource::Empty() const { + return Size() == 0; +} + +TaskSource::TopTask TaskSource::Top() const { + FML_CHECK(!Empty()); + if (secondary_pause_requests_ > 0 || secondary_task_queue_.empty()) { + const auto& primary_top = primary_task_queue_.top(); + return { + .task_queue_id = task_queue_id_, + .task_source_grade = TaskSourceGrade::kPrimary, + .task = primary_top, + }; + } else if (primary_task_queue_.empty()) { + const auto& secondary_top = secondary_task_queue_.top(); + return { + .task_queue_id = task_queue_id_, + .task_source_grade = TaskSourceGrade::kSecondary, + .task = secondary_top, + }; + } else { + const auto& primary_top = primary_task_queue_.top(); + const auto& secondary_top = secondary_task_queue_.top(); + if (primary_top > secondary_top) { + return { + .task_queue_id = task_queue_id_, + .task_source_grade = TaskSourceGrade::kSecondary, + .task = secondary_top, + }; + } else { + return { + .task_queue_id = task_queue_id_, + .task_source_grade = TaskSourceGrade::kPrimary, + .task = primary_top, + }; + } + } +} + +void TaskSource::PauseSecondary() { + secondary_pause_requests_++; +} + +void TaskSource::ResumeSecondary() { + secondary_pause_requests_--; + FML_DCHECK(secondary_pause_requests_ >= 0); +} + +} // namespace fml diff --git a/fml/task_source.h b/fml/task_source.h new file mode 100644 index 0000000000000..14c1e22d1a232 --- /dev/null +++ b/fml/task_source.h @@ -0,0 +1,61 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_FML_TASK_SOURCE_H_ +#define FLUTTER_FML_TASK_SOURCE_H_ + +#include "flutter/fml/delayed_task.h" +#include "flutter/fml/task_queue_id.h" + +namespace fml { + +class MessageLoopTaskQueues; + +enum TaskSourceGrade { + kPrimary, + kSecondary, +}; + +class TaskSource { + public: + struct TopTask { + TaskQueueId task_queue_id; + TaskSourceGrade task_source_grade; + DelayedTask task; + }; + + explicit TaskSource(TaskQueueId task_queue_id); + + ~TaskSource(); + + void Clear(); + + void RegisterTask(TaskSourceGrade grade, DelayedTask&& task); + + void PopTask(TaskSourceGrade grade); + + size_t Size() const; + + bool Empty() const; + + TopTask Top() const; + + private: + friend class MessageLoopTaskQueues; + + void PauseSecondary(); + + void ResumeSecondary(); + + const fml::TaskQueueId task_queue_id_; + fml::DelayedTaskQueue primary_task_queue_; + fml::DelayedTaskQueue secondary_task_queue_; + int secondary_pause_requests_ = 0; + + FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TaskSource); +}; + +} // namespace fml + +#endif // FLUTTER_FML_TASK_SOURCE_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 1938f64532dd4..a07833bd3055a 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -18,6 +18,9 @@ #include "flutter/runtime/dart_vm.h" #include "flutter/runtime/dart_vm_lifecycle.h" #include "flutter/runtime/isolate_configuration.h" +#include "fml/message_loop_task_queues.h" +#include "fml/task_source.h" +#include "fml/time/time_point.h" #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/dart/runtime/include/dart_tools_api.h" #include "third_party/tonic/converter/dart_converter.h" @@ -455,10 +458,14 @@ void DartIsolate::SetMessageHandlingTaskRunner( message_handling_task_runner_ = runner; message_handler().Initialize([runner](std::function task) { - runner->PostTask([task = std::move(task)]() { - TRACE_EVENT0("flutter", "DartIsolate::HandleMessage"); - task(); - }); + auto task_queues = fml::MessageLoopTaskQueues::GetInstance(); + task_queues->RegisterTask( + runner->GetTaskQueueId(), + [task = std::move(task)]() { + TRACE_EVENT0("flutter", "DartIsolate::HandleMessage"); + task(); + }, + fml::TimePoint::Now(), fml::TaskSourceGrade::kSecondary); }); } diff --git a/shell/common/vsync_waiter.cc b/shell/common/vsync_waiter.cc index f1cb89342874c..14ca394d563e1 100644 --- a/shell/common/vsync_waiter.cc +++ b/shell/common/vsync_waiter.cc @@ -6,6 +6,7 @@ #include "flutter/fml/task_runner.h" #include "flutter/fml/trace_event.h" +#include "fml/message_loop_task_queues.h" namespace flutter { @@ -113,6 +114,9 @@ void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, if (callback) { auto flow_identifier = fml::tracing::TraceNonce(); + auto ui_task_queue_id = task_runners_.GetUITaskRunner()->GetTaskQueueId(); + auto task_queues = fml::MessageLoopTaskQueues::GetInstance(); + task_queues->PauseSecondarySource(ui_task_queue_id); // The base trace ensures that flows have a root to begin from if one does // not exist. The trace viewer will ignore traces that have no base event @@ -123,11 +127,13 @@ void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, TRACE_FLOW_BEGIN("flutter", kVsyncFlowName, flow_identifier); task_runners_.GetUITaskRunner()->PostTaskForTime( - [callback, flow_identifier, frame_start_time, frame_target_time]() { + [callback, flow_identifier, frame_start_time, frame_target_time, + task_queues, ui_task_queue_id]() { FML_TRACE_EVENT("flutter", kVsyncTraceName, "StartTime", frame_start_time, "TargetTime", frame_target_time); callback(frame_start_time, frame_target_time); TRACE_FLOW_END("flutter", kVsyncFlowName, flow_identifier); + task_queues->ResumeSecondarySource(ui_task_queue_id); }, frame_start_time); } From ce2efd252d5b5bc159a06197ead5a1ae89c9b690 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Mon, 12 Apr 2021 10:37:40 -0700 Subject: [PATCH 2/7] Add a test and fix licenses --- ci/licenses_golden/licenses_flutter | 3 ++ fml/message_loop_task_queues.cc | 15 ++++++ fml/message_loop_task_queues.h | 2 + .../embedder/tests/embedder_unittests_gl.cc | 52 +++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index d5e25a91ec72c..1f2470bec9e5f 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -269,8 +269,11 @@ FILE: ../../../flutter/fml/synchronization/sync_switch_unittest.cc FILE: ../../../flutter/fml/synchronization/waitable_event.cc FILE: ../../../flutter/fml/synchronization/waitable_event.h FILE: ../../../flutter/fml/synchronization/waitable_event_unittest.cc +FILE: ../../../flutter/fml/task_queue_id.h FILE: ../../../flutter/fml/task_runner.cc FILE: ../../../flutter/fml/task_runner.h +FILE: ../../../flutter/fml/task_source.cc +FILE: ../../../flutter/fml/task_source.h FILE: ../../../flutter/fml/thread.cc FILE: ../../../flutter/fml/thread.h FILE: ../../../flutter/fml/thread_local.cc diff --git a/fml/message_loop_task_queues.cc b/fml/message_loop_task_queues.cc index 4fb0cec8367e5..0cef93f577e11 100644 --- a/fml/message_loop_task_queues.cc +++ b/fml/message_loop_task_queues.cc @@ -11,6 +11,7 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop_impl.h" +#include "flutter/fml/task_source.h" namespace fml { @@ -18,8 +19,13 @@ std::mutex MessageLoopTaskQueues::creation_mutex_; const size_t TaskQueueId::kUnmerged = ULONG_MAX; +// Guarded by creation_mutex_. fml::RefPtr MessageLoopTaskQueues::instance_; +// Guarded by creation_mutex_. +static thread_local TaskSourceGrade tls_task_source_grade = + TaskSourceGrade::kPrimary; + TaskQueueEntry::TaskQueueEntry(TaskQueueId created_for_arg) : owner_of(_kUnmerged), subsumed_by(_kUnmerged), @@ -72,6 +78,11 @@ void MessageLoopTaskQueues::DisposeTasks(TaskQueueId queue_id) { } } +TaskSourceGrade MessageLoopTaskQueues::GetCurrentTaskSourceGrade() { + std::scoped_lock creation(creation_mutex_); + return tls_task_source_grade; +} + void MessageLoopTaskQueues::RegisterTask( TaskQueueId queue_id, const fml::closure& task, @@ -118,6 +129,10 @@ fml::closure MessageLoopTaskQueues::GetNextTaskToRun(TaskQueueId queue_id, fml::closure invocation = top.task.GetTask(); queue_entries_.at(top.task_queue_id) ->task_source->PopTask(top.task_source_grade); + { + std::scoped_lock creation(creation_mutex_); + tls_task_source_grade = top.task_source_grade; + } return invocation; } diff --git a/fml/message_loop_task_queues.h b/fml/message_loop_task_queues.h index 031a718f7f816..d25e54249d1a3 100644 --- a/fml/message_loop_task_queues.h +++ b/fml/message_loop_task_queues.h @@ -81,6 +81,8 @@ class MessageLoopTaskQueues size_t GetNumPendingTasks(TaskQueueId queue_id) const; + static TaskSourceGrade GetCurrentTaskSourceGrade(); + // Observers methods. void AddTaskObserver(TaskQueueId queue_id, diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index b6744999753eb..d9fccdf855e2b 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -14,9 +14,11 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/mapping.h" #include "flutter/fml/message_loop.h" +#include "flutter/fml/message_loop_task_queues.h" #include "flutter/fml/paths.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/fml/task_source.h" #include "flutter/fml/thread.h" #include "flutter/lib/ui/painting/image.h" #include "flutter/runtime/dart_vm.h" @@ -3443,5 +3445,55 @@ TEST_F(EmbedderTest, SnapshotRenderTargetScalesDownToDriverMax) { latch.Wait(); } +TEST_F(EmbedderTest, ObjectsPostedViaPortsServicedOnSecondaryTaskHeap) { + auto& context = GetEmbedderContext(EmbedderTestContextType::kOpenGLContext); + EmbedderConfigBuilder builder(context); + builder.SetOpenGLRendererConfig(SkISize::Make(800, 1024)); + builder.SetDartEntrypoint("objects_can_be_posted"); + + // Synchronously acquire the send port from the Dart end. We will be using + // this to send message. The Dart end will just echo those messages back to us + // for inspection. + FlutterEngineDartPort port = 0; + fml::AutoResetWaitableEvent event; + context.AddNativeCallback("SignalNativeCount", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + port = tonic::DartConverter::FromDart( + Dart_GetNativeArgument(args, 0)); + event.Signal(); + })); + auto engine = builder.LaunchEngine(); + ASSERT_TRUE(engine.is_valid()); + event.Wait(); + ASSERT_NE(port, 0); + + using Trampoline = std::function; + Trampoline trampoline; + + context.AddNativeCallback("SendObjectToNativeCode", + CREATE_NATIVE_ENTRY([&](Dart_NativeArguments args) { + FML_CHECK(trampoline); + auto trampoline_copy = trampoline; + trampoline = nullptr; + trampoline_copy(Dart_GetNativeArgument(args, 0)); + })); + + // Send a boolean value and assert that it's received by the right heap. + { + FlutterEngineDartObject object = {}; + object.type = kFlutterEngineDartObjectTypeBool; + object.bool_value = true; + trampoline = [&](Dart_Handle handle) { + ASSERT_TRUE(tonic::DartConverter::FromDart(handle)); + auto task_grade = fml::MessageLoopTaskQueues::GetCurrentTaskSourceGrade(); + EXPECT_EQ(task_grade, fml::TaskSourceGrade::kSecondary); + event.Signal(); + }; + ASSERT_EQ(FlutterEnginePostDartObject(engine.get(), port, &object), + kSuccess); + event.Wait(); + } +} + } // namespace testing } // namespace flutter From 46746af56aa31bf61824116ecc58b89b0fe92c83 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Tue, 13 Apr 2021 11:28:07 -0700 Subject: [PATCH 3/7] Address comments modulo: 1. Add tests for TaskSource 2. Add docs for TaskSource 3. Get existing failing tests to pass --- fml/delayed_task.cc | 16 ++++++--- fml/delayed_task.h | 7 +++- fml/message_loop_task_queues.cc | 26 +++++++------- fml/message_loop_task_queues.h | 10 +++--- fml/task_queue_id.h | 8 ++++- fml/task_source.cc | 36 ++++++++++--------- fml/task_source.h | 17 ++++----- fml/task_source_grade.h | 28 +++++++++++++++ runtime/dart_isolate.cc | 2 +- .../embedder/tests/embedder_unittests_gl.cc | 2 +- 10 files changed, 99 insertions(+), 53 deletions(-) create mode 100644 fml/task_source_grade.h diff --git a/fml/delayed_task.cc b/fml/delayed_task.cc index 997176d7694e0..bb72daa7cb78e 100644 --- a/fml/delayed_task.cc +++ b/fml/delayed_task.cc @@ -10,13 +10,17 @@ namespace fml { DelayedTask::DelayedTask(size_t order, const fml::closure& task, - fml::TimePoint target_time) - : order_(order), task_(task), target_time_(target_time) {} - -DelayedTask::DelayedTask(const DelayedTask& other) = default; + fml::TimePoint target_time, + fml::TaskSourceGrade task_source_grade) + : order_(order), + task_(task), + target_time_(target_time), + task_source_grade_(task_source_grade) {} DelayedTask::~DelayedTask() = default; +DelayedTask::DelayedTask(const DelayedTask& other) = default; + const fml::closure& DelayedTask::GetTask() const { return task_; } @@ -25,6 +29,10 @@ fml::TimePoint DelayedTask::GetTargetTime() const { return target_time_; } +fml::TaskSourceGrade DelayedTask::GetTaskSourceGrade() const { + return task_source_grade_; +} + bool DelayedTask::operator>(const DelayedTask& other) const { if (target_time_ == other.target_time_) { return order_ > other.order_; diff --git a/fml/delayed_task.h b/fml/delayed_task.h index ba6fc8cfe4c9d..6552a68652918 100644 --- a/fml/delayed_task.h +++ b/fml/delayed_task.h @@ -8,6 +8,7 @@ #include #include "flutter/fml/closure.h" +#include "flutter/fml/task_source_grade.h" #include "flutter/fml/time/time_point.h" namespace fml { @@ -16,7 +17,8 @@ class DelayedTask { public: DelayedTask(size_t order, const fml::closure& task, - fml::TimePoint target_time); + fml::TimePoint target_time, + fml::TaskSourceGrade task_source_grade); DelayedTask(const DelayedTask& other); @@ -26,12 +28,15 @@ class DelayedTask { fml::TimePoint GetTargetTime() const; + fml::TaskSourceGrade GetTaskSourceGrade() const; + bool operator>(const DelayedTask& other) const; private: size_t order_; fml::closure task_; fml::TimePoint target_time_; + fml::TaskSourceGrade task_source_grade_; }; using DelayedTaskQueue = std::priority_queue MessageLoopTaskQueues::instance_; // Guarded by creation_mutex_. static thread_local TaskSourceGrade tls_task_source_grade = - TaskSourceGrade::kPrimary; + TaskSourceGrade::kUnspecified; TaskQueueEntry::TaskQueueEntry(TaskQueueId created_for_arg) : owner_of(_kUnmerged), @@ -72,9 +72,9 @@ void MessageLoopTaskQueues::DisposeTasks(TaskQueueId queue_id) { const auto& queue_entry = queue_entries_.at(queue_id); FML_DCHECK(queue_entry->subsumed_by == _kUnmerged); TaskQueueId subsumed = queue_entry->owner_of; - queue_entry->task_source->Clear(); + queue_entry->task_source->ShutDown(); if (subsumed != _kUnmerged) { - queue_entries_.at(subsumed)->task_source->Clear(); + queue_entries_.at(subsumed)->task_source->ShutDown(); } } @@ -91,8 +91,8 @@ void MessageLoopTaskQueues::RegisterTask( std::lock_guard guard(queue_mutex_); size_t order = order_++; const auto& queue_entry = queue_entries_.at(queue_id); - queue_entry->task_source->RegisterTask(task_source_grade, - {order, task, target_time}); + queue_entry->task_source->RegisterTask( + {order, task, target_time, task_source_grade}); TaskQueueId loop_to_wake = queue_id; if (queue_entry->subsumed_by != _kUnmerged) { loop_to_wake = queue_entry->subsumed_by; @@ -128,10 +128,10 @@ fml::closure MessageLoopTaskQueues::GetNextTaskToRun(TaskQueueId queue_id, } fml::closure invocation = top.task.GetTask(); queue_entries_.at(top.task_queue_id) - ->task_source->PopTask(top.task_source_grade); + ->task_source->PopTask(top.task.GetTaskSourceGrade()); { std::scoped_lock creation(creation_mutex_); - tls_task_source_grade = top.task_source_grade; + tls_task_source_grade = top.task.GetTaskSourceGrade(); } return invocation; } @@ -151,12 +151,12 @@ size_t MessageLoopTaskQueues::GetNumPendingTasks(TaskQueueId queue_id) const { } size_t total_tasks = 0; - total_tasks += queue_entry->task_source->Size(); + total_tasks += queue_entry->task_source->GetNumPendingTasks(); TaskQueueId subsumed = queue_entry->owner_of; if (subsumed != _kUnmerged) { const auto& subsumed_entry = queue_entries_.at(subsumed); - total_tasks += subsumed_entry->task_source->Size(); + total_tasks += subsumed_entry->task_source->GetNumPendingTasks(); } return total_tasks; } @@ -297,7 +297,7 @@ bool MessageLoopTaskQueues::HasPendingTasksUnlocked( return false; } - if (!entry->task_source->Empty()) { + if (!entry->task_source->IsEmpty()) { return true; } @@ -306,7 +306,7 @@ bool MessageLoopTaskQueues::HasPendingTasksUnlocked( // this is not an owner and queue is empty. return false; } else { - return !queue_entries_.at(subsumed)->task_source->Empty(); + return !queue_entries_.at(subsumed)->task_source->IsEmpty(); } } @@ -328,8 +328,8 @@ TaskSource::TopTask MessageLoopTaskQueues::PeekNextTaskUnlocked( TaskSource* subsumed_tasks = queue_entries_.at(subsumed)->task_source.get(); // we are owning another task queue - const bool subsumed_has_task = !subsumed_tasks->Empty(); - const bool owner_has_task = !owner_tasks->Empty(); + const bool subsumed_has_task = !subsumed_tasks->IsEmpty(); + const bool owner_has_task = !owner_tasks->IsEmpty(); fml::TaskQueueId top_queue_id = owner; if (owner_has_task && subsumed_has_task) { const auto owner_task = owner_tasks->Top(); diff --git a/fml/message_loop_task_queues.h b/fml/message_loop_task_queues.h index d25e54249d1a3..3d7f2bcb87227 100644 --- a/fml/message_loop_task_queues.h +++ b/fml/message_loop_task_queues.h @@ -69,11 +69,11 @@ class MessageLoopTaskQueues // Tasks methods. - void RegisterTask( - TaskQueueId queue_id, - const fml::closure& task, - fml::TimePoint target_time, - fml::TaskSourceGrade task_source_grade = fml::TaskSourceGrade::kPrimary); + void RegisterTask(TaskQueueId queue_id, + const fml::closure& task, + fml::TimePoint target_time, + fml::TaskSourceGrade task_source_grade = + fml::TaskSourceGrade::kUnspecified); bool HasPendingTasks(TaskQueueId queue_id) const; diff --git a/fml/task_queue_id.h b/fml/task_queue_id.h index abf0e51e3f975..2395f9db92be6 100644 --- a/fml/task_queue_id.h +++ b/fml/task_queue_id.h @@ -9,13 +9,19 @@ namespace fml { +/** + * `MessageLoopTaskQueues` task dispatcher's internal task queue identifier. + */ class TaskQueueId { public: + /// This constant indicates whether a task queue has been subsumed by a task + /// runner. static const size_t kUnmerged; + /// Intializes a task queue with the given value as it's ID. explicit TaskQueueId(size_t value) : value_(value) {} - operator int() const { return value_; } + operator size_t() const { return value_; } private: size_t value_ = kUnmerged; diff --git a/fml/task_source.cc b/fml/task_source.cc index 77dabcdbff3d2..e62e1952e65a5 100644 --- a/fml/task_source.cc +++ b/fml/task_source.cc @@ -11,19 +11,24 @@ namespace fml { TaskSource::TaskSource(TaskQueueId task_queue_id) : task_queue_id_(task_queue_id) {} -TaskSource::~TaskSource() = default; +TaskSource::~TaskSource() { + ShutDown(); +} -void TaskSource::Clear() { +void TaskSource::ShutDown() { primary_task_queue_ = {}; secondary_task_queue_ = {}; } -void TaskSource::RegisterTask(TaskSourceGrade grade, DelayedTask&& task) { - switch (grade) { - case TaskSourceGrade::kPrimary: +void TaskSource::RegisterTask(const DelayedTask& task) { + switch (task.GetTaskSourceGrade()) { + case TaskSourceGrade::kUserInteraction: + primary_task_queue_.push(task); + break; + case TaskSourceGrade::kUnspecified: primary_task_queue_.push(task); break; - case TaskSourceGrade::kSecondary: + case TaskSourceGrade::kDartMicroTasks: secondary_task_queue_.push(task); break; } @@ -31,16 +36,19 @@ void TaskSource::RegisterTask(TaskSourceGrade grade, DelayedTask&& task) { void TaskSource::PopTask(TaskSourceGrade grade) { switch (grade) { - case TaskSourceGrade::kPrimary: + case TaskSourceGrade::kUserInteraction: + primary_task_queue_.pop(); + break; + case TaskSourceGrade::kUnspecified: primary_task_queue_.pop(); break; - case TaskSourceGrade::kSecondary: + case TaskSourceGrade::kDartMicroTasks: secondary_task_queue_.pop(); break; } } -size_t TaskSource::Size() const { +size_t TaskSource::GetNumPendingTasks() const { size_t size = primary_task_queue_.size(); if (secondary_pause_requests_ == 0) { size += secondary_task_queue_.size(); @@ -48,24 +56,22 @@ size_t TaskSource::Size() const { return size; } -bool TaskSource::Empty() const { - return Size() == 0; +bool TaskSource::IsEmpty() const { + return GetNumPendingTasks() == 0; } TaskSource::TopTask TaskSource::Top() const { - FML_CHECK(!Empty()); + FML_CHECK(!IsEmpty()); if (secondary_pause_requests_ > 0 || secondary_task_queue_.empty()) { const auto& primary_top = primary_task_queue_.top(); return { .task_queue_id = task_queue_id_, - .task_source_grade = TaskSourceGrade::kPrimary, .task = primary_top, }; } else if (primary_task_queue_.empty()) { const auto& secondary_top = secondary_task_queue_.top(); return { .task_queue_id = task_queue_id_, - .task_source_grade = TaskSourceGrade::kSecondary, .task = secondary_top, }; } else { @@ -74,13 +80,11 @@ TaskSource::TopTask TaskSource::Top() const { if (primary_top > secondary_top) { return { .task_queue_id = task_queue_id_, - .task_source_grade = TaskSourceGrade::kSecondary, .task = secondary_top, }; } else { return { .task_queue_id = task_queue_id_, - .task_source_grade = TaskSourceGrade::kPrimary, .task = primary_top, }; } diff --git a/fml/task_source.h b/fml/task_source.h index 14c1e22d1a232..88c0ca92b99c2 100644 --- a/fml/task_source.h +++ b/fml/task_source.h @@ -7,37 +7,32 @@ #include "flutter/fml/delayed_task.h" #include "flutter/fml/task_queue_id.h" +#include "flutter/fml/task_source_grade.h" namespace fml { class MessageLoopTaskQueues; -enum TaskSourceGrade { - kPrimary, - kSecondary, -}; - class TaskSource { public: struct TopTask { TaskQueueId task_queue_id; - TaskSourceGrade task_source_grade; - DelayedTask task; + const DelayedTask& task; }; explicit TaskSource(TaskQueueId task_queue_id); ~TaskSource(); - void Clear(); + void ShutDown(); - void RegisterTask(TaskSourceGrade grade, DelayedTask&& task); + void RegisterTask(const DelayedTask& task); void PopTask(TaskSourceGrade grade); - size_t Size() const; + size_t GetNumPendingTasks() const; - bool Empty() const; + bool IsEmpty() const; TopTask Top() const; diff --git a/fml/task_source_grade.h b/fml/task_source_grade.h new file mode 100644 index 0000000000000..d7447f511f1e5 --- /dev/null +++ b/fml/task_source_grade.h @@ -0,0 +1,28 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_FML_TASK_SOURCE_GRADE_H_ +#define FLUTTER_FML_TASK_SOURCE_GRADE_H_ + +namespace fml { + +/** + * Categories of work dispatched to `MessageLoopTaskQueues` dispatcher. By + * specifying the `TaskSourceGrade`, you indicate the task's importance to the + * dispatcher. + */ +enum class TaskSourceGrade { + /// This `TaskSourceGrade` indicates that a task is critical to user + /// interaction. + kUserInteraction, + /// This `TaskSourceGrade` indicates that a task corresponds to servicing a + /// dart micro task. These aren't critical to user interaction. + kDartMicroTasks, + /// The absence of a specialized `TaskSourceGrade`. + kUnspecified, +}; + +} // namespace fml + +#endif // FLUTTER_FML_TASK_SOURCE_GRADE_H_ diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index a07833bd3055a..8371e3f225c88 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -465,7 +465,7 @@ void DartIsolate::SetMessageHandlingTaskRunner( TRACE_EVENT0("flutter", "DartIsolate::HandleMessage"); task(); }, - fml::TimePoint::Now(), fml::TaskSourceGrade::kSecondary); + fml::TimePoint::Now(), fml::TaskSourceGrade::kDartMicroTasks); }); } diff --git a/shell/platform/embedder/tests/embedder_unittests_gl.cc b/shell/platform/embedder/tests/embedder_unittests_gl.cc index d9fccdf855e2b..7a5f4750b16cb 100644 --- a/shell/platform/embedder/tests/embedder_unittests_gl.cc +++ b/shell/platform/embedder/tests/embedder_unittests_gl.cc @@ -3486,7 +3486,7 @@ TEST_F(EmbedderTest, ObjectsPostedViaPortsServicedOnSecondaryTaskHeap) { trampoline = [&](Dart_Handle handle) { ASSERT_TRUE(tonic::DartConverter::FromDart(handle)); auto task_grade = fml::MessageLoopTaskQueues::GetCurrentTaskSourceGrade(); - EXPECT_EQ(task_grade, fml::TaskSourceGrade::kSecondary); + EXPECT_EQ(task_grade, fml::TaskSourceGrade::kDartMicroTasks); event.Signal(); }; ASSERT_EQ(FlutterEnginePostDartObject(engine.get(), port, &object), From d9f8da707f157e725031b075b0af2610fc215bf1 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Tue, 13 Apr 2021 11:50:46 -0700 Subject: [PATCH 4/7] docs for task source --- fml/task_source.h | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/fml/task_source.h b/fml/task_source.h index 88c0ca92b99c2..c9531ed42d22d 100644 --- a/fml/task_source.h +++ b/fml/task_source.h @@ -13,6 +13,25 @@ namespace fml { class MessageLoopTaskQueues; +/** + * A Source of tasks for the `MessageLoopTaskQueues` task dispatcher. This is a + * wrapper around a primary and secondary task heap with the difference between + * them being that the secondary task heap can be paused and resumed by the task + * dispatcher. `TaskSourceGrade` determines what task heap the task is assigned + * to. + * + * Registering Tasks + * ----------------- + * The task dispatcher associates a task source with each `TaskQueueID`. When + * the user of the task dispatcher registers a task, the task is in-turn + * registered with the `TaskSource` corresponding to the `TaskQueueID`. + * + * Processing Tasks + * ---------------- + * Task dispatcher provides the event loop a way to acquire tasks to run via + * `GetNextTaskToRun`. Task dispatcher asks the underlying `TaskSource` for the + * next task. + */ class TaskSource { public: struct TopTask { @@ -20,20 +39,30 @@ class TaskSource { const DelayedTask& task; }; + /// Construts a TaskSource with the given `task_queue_id`. explicit TaskSource(TaskQueueId task_queue_id); ~TaskSource(); + /// Drops the pending tasks from both primary and secondary task heaps. void ShutDown(); + /// Adds a task to the corresponding task heap as dictated by the + /// `TaskSourceGrade` of the `DelayedTask`. void RegisterTask(const DelayedTask& task); + /// Pops the task heap corresponding to the `TaskSourceGrade`. void PopTask(TaskSourceGrade grade); + /// Returns the number of pending tasks. Excludes the tasks from the secondary + /// heap if it's paused. size_t GetNumPendingTasks() const; + /// Returns true if `GetNumPendingTasks` is zero. bool IsEmpty() const; + /// Returns the top task based on scheduled time, taking into account whether + /// the secondary heap has been paused or not. TopTask Top() const; private: From 88bed7baf19464cf16f4e82c72afc35124b0a28f Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Tue, 13 Apr 2021 12:12:06 -0700 Subject: [PATCH 5/7] add tests for task_sourcd --- fml/BUILD.gn | 1 + fml/task_source.h | 6 +-- fml/task_source_unittests.cc | 100 +++++++++++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 3 deletions(-) create mode 100644 fml/task_source_unittests.cc diff --git a/fml/BUILD.gn b/fml/BUILD.gn index a11defe924c9b..3a720fae2f9fb 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -264,6 +264,7 @@ if (enable_unittests) { "synchronization/semaphore_unittest.cc", "synchronization/sync_switch_unittest.cc", "synchronization/waitable_event_unittest.cc", + "task_source_unittests.cc", "thread_local_unittests.cc", "thread_unittests.cc", "time/time_delta_unittest.cc", diff --git a/fml/task_source.h b/fml/task_source.h index c9531ed42d22d..64e151d9482ec 100644 --- a/fml/task_source.h +++ b/fml/task_source.h @@ -65,13 +65,13 @@ class TaskSource { /// the secondary heap has been paused or not. TopTask Top() const; - private: - friend class MessageLoopTaskQueues; - + /// Pause providing tasks from secondary task heap. void PauseSecondary(); + /// Resume providing tasks from secondary task heap. void ResumeSecondary(); + private: const fml::TaskQueueId task_queue_id_; fml::DelayedTaskQueue primary_task_queue_; fml::DelayedTaskQueue secondary_task_queue_; diff --git a/fml/task_source_unittests.cc b/fml/task_source_unittests.cc new file mode 100644 index 0000000000000..e44e9e3b87fd3 --- /dev/null +++ b/fml/task_source_unittests.cc @@ -0,0 +1,100 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include +#include + +#include "flutter/fml/macros.h" +#include "flutter/fml/task_source.h" +#include "flutter/fml/time/time_delta.h" +#include "flutter/fml/time/time_point.h" +#include "gtest/gtest.h" + +namespace fml { +namespace testing { + +TEST(TaskSourceTests, SimpleInitialization) { + TaskSource task_source = TaskSource(TaskQueueId(1)); + task_source.RegisterTask( + {1, [] {}, fml::TimePoint::Now(), TaskSourceGrade::kUnspecified}); + ASSERT_EQ(task_source.GetNumPendingTasks(), 1u); +} + +TEST(TaskSourceTests, MultipleTaskGrades) { + TaskSource task_source = TaskSource(TaskQueueId(1)); + task_source.RegisterTask( + {1, [] {}, fml::TimePoint::Now(), TaskSourceGrade::kUnspecified}); + task_source.RegisterTask( + {2, [] {}, fml::TimePoint::Now(), TaskSourceGrade::kUserInteraction}); + task_source.RegisterTask( + {3, [] {}, fml::TimePoint::Now(), TaskSourceGrade::kDartMicroTasks}); + ASSERT_EQ(task_source.GetNumPendingTasks(), 3u); +} + +TEST(TaskSourceTests, SimpleOrdering) { + TaskSource task_source = TaskSource(TaskQueueId(1)); + auto time_stamp = fml::TimePoint::Now(); + int value = 0; + task_source.RegisterTask( + {1, [&] { value = 1; }, time_stamp, TaskSourceGrade::kUnspecified}); + task_source.RegisterTask({2, [&] { value = 7; }, + time_stamp + fml::TimeDelta::FromMilliseconds(1), + TaskSourceGrade::kUnspecified}); + task_source.Top().task.GetTask()(); + task_source.PopTask(TaskSourceGrade::kUnspecified); + ASSERT_EQ(value, 1); + task_source.Top().task.GetTask()(); + task_source.PopTask(TaskSourceGrade::kUnspecified); + ASSERT_EQ(value, 7); +} + +TEST(TaskSourceTests, SimpleOrderingMultiTaskHeaps) { + TaskSource task_source = TaskSource(TaskQueueId(1)); + auto time_stamp = fml::TimePoint::Now(); + int value = 0; + task_source.RegisterTask( + {1, [&] { value = 1; }, time_stamp, TaskSourceGrade::kDartMicroTasks}); + task_source.RegisterTask({2, [&] { value = 7; }, + time_stamp + fml::TimeDelta::FromMilliseconds(1), + TaskSourceGrade::kUserInteraction}); + auto top_task = task_source.Top(); + top_task.task.GetTask()(); + task_source.PopTask(top_task.task.GetTaskSourceGrade()); + ASSERT_EQ(value, 1); + + auto second_task = task_source.Top(); + second_task.task.GetTask()(); + task_source.PopTask(second_task.task.GetTaskSourceGrade()); + ASSERT_EQ(value, 7); +} + +TEST(TaskSourceTests, OrderingMultiTaskHeapsSecondaryPaused) { + TaskSource task_source = TaskSource(TaskQueueId(1)); + auto time_stamp = fml::TimePoint::Now(); + int value = 0; + task_source.RegisterTask( + {1, [&] { value = 1; }, time_stamp, TaskSourceGrade::kDartMicroTasks}); + task_source.RegisterTask({2, [&] { value = 7; }, + time_stamp + fml::TimeDelta::FromMilliseconds(1), + TaskSourceGrade::kUserInteraction}); + + task_source.PauseSecondary(); + + auto top_task = task_source.Top(); + top_task.task.GetTask()(); + task_source.PopTask(top_task.task.GetTaskSourceGrade()); + ASSERT_EQ(value, 7); + + ASSERT_TRUE(task_source.IsEmpty()); + + task_source.ResumeSecondary(); + + auto second_task = task_source.Top(); + second_task.task.GetTask()(); + task_source.PopTask(second_task.task.GetTaskSourceGrade()); + ASSERT_EQ(value, 1); +} + +} // namespace testing +} // namespace fml From 7795d8db359bcd30689ba401c42330fb7e75ada3 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Fri, 16 Apr 2021 09:43:38 -0700 Subject: [PATCH 6/7] rebase --- ci/licenses_golden/licenses_flutter | 2 ++ shell/common/vsync_waiter.cc | 7 +------ 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 1f2470bec9e5f..8617793825fb3 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -274,6 +274,8 @@ FILE: ../../../flutter/fml/task_runner.cc FILE: ../../../flutter/fml/task_runner.h FILE: ../../../flutter/fml/task_source.cc FILE: ../../../flutter/fml/task_source.h +FILE: ../../../flutter/fml/task_source_grade.h +FILE: ../../../flutter/fml/task_source_unittests.cc FILE: ../../../flutter/fml/thread.cc FILE: ../../../flutter/fml/thread.h FILE: ../../../flutter/fml/thread_local.cc diff --git a/shell/common/vsync_waiter.cc b/shell/common/vsync_waiter.cc index 14ca394d563e1..886f6724db61c 100644 --- a/shell/common/vsync_waiter.cc +++ b/shell/common/vsync_waiter.cc @@ -114,9 +114,6 @@ void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, if (callback) { auto flow_identifier = fml::tracing::TraceNonce(); - auto ui_task_queue_id = task_runners_.GetUITaskRunner()->GetTaskQueueId(); - auto task_queues = fml::MessageLoopTaskQueues::GetInstance(); - task_queues->PauseSecondarySource(ui_task_queue_id); // The base trace ensures that flows have a root to begin from if one does // not exist. The trace viewer will ignore traces that have no base event @@ -127,13 +124,11 @@ void VsyncWaiter::FireCallback(fml::TimePoint frame_start_time, TRACE_FLOW_BEGIN("flutter", kVsyncFlowName, flow_identifier); task_runners_.GetUITaskRunner()->PostTaskForTime( - [callback, flow_identifier, frame_start_time, frame_target_time, - task_queues, ui_task_queue_id]() { + [callback, flow_identifier, frame_start_time, frame_target_time]() { FML_TRACE_EVENT("flutter", kVsyncTraceName, "StartTime", frame_start_time, "TargetTime", frame_target_time); callback(frame_start_time, frame_target_time); TRACE_FLOW_END("flutter", kVsyncFlowName, flow_identifier); - task_queues->ResumeSecondarySource(ui_task_queue_id); }, frame_start_time); } From 35cc7b215a4816c564bbfd123ce926a30c8180bb Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Fri, 16 Apr 2021 13:43:33 -0700 Subject: [PATCH 7/7] fix thread local --- fml/message_loop_task_queues.cc | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/fml/message_loop_task_queues.cc b/fml/message_loop_task_queues.cc index 5aa5ce8bc7254..a2b4b600d26eb 100644 --- a/fml/message_loop_task_queues.cc +++ b/fml/message_loop_task_queues.cc @@ -12,6 +12,7 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/message_loop_impl.h" #include "flutter/fml/task_source.h" +#include "flutter/fml/thread_local.h" namespace fml { @@ -22,9 +23,22 @@ const size_t TaskQueueId::kUnmerged = ULONG_MAX; // Guarded by creation_mutex_. fml::RefPtr MessageLoopTaskQueues::instance_; +namespace { + +// iOS prior to version 9 prevents c++11 thread_local and __thread specefier, +// having us resort to boxed enum containers. +class TaskSourceGradeHolder { + public: + TaskSourceGrade task_source_grade; + + explicit TaskSourceGradeHolder(TaskSourceGrade task_source_grade_arg) + : task_source_grade(task_source_grade_arg) {} +}; +} // namespace + // Guarded by creation_mutex_. -static thread_local TaskSourceGrade tls_task_source_grade = - TaskSourceGrade::kUnspecified; +FML_THREAD_LOCAL ThreadLocalUniquePtr + tls_task_source_grade; TaskQueueEntry::TaskQueueEntry(TaskQueueId created_for_arg) : owner_of(_kUnmerged), @@ -39,6 +53,8 @@ fml::RefPtr MessageLoopTaskQueues::GetInstance() { std::scoped_lock creation(creation_mutex_); if (!instance_) { instance_ = fml::MakeRefCounted(); + tls_task_source_grade.reset( + new TaskSourceGradeHolder{TaskSourceGrade::kUnspecified}); } return instance_; } @@ -80,7 +96,7 @@ void MessageLoopTaskQueues::DisposeTasks(TaskQueueId queue_id) { TaskSourceGrade MessageLoopTaskQueues::GetCurrentTaskSourceGrade() { std::scoped_lock creation(creation_mutex_); - return tls_task_source_grade; + return tls_task_source_grade.get()->task_source_grade; } void MessageLoopTaskQueues::RegisterTask( @@ -131,7 +147,8 @@ fml::closure MessageLoopTaskQueues::GetNextTaskToRun(TaskQueueId queue_id, ->task_source->PopTask(top.task.GetTaskSourceGrade()); { std::scoped_lock creation(creation_mutex_); - tls_task_source_grade = top.task.GetTaskSourceGrade(); + const auto task_source_grade = top.task.GetTaskSourceGrade(); + tls_task_source_grade.reset(new TaskSourceGradeHolder{task_source_grade}); } return invocation; }