From 9e7f27781c740a9b2b9cddd13fa6739335976e77 Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Fri, 8 May 2020 13:18:29 -0700 Subject: [PATCH 1/2] refactor --- fml/memory/task_runner_checker.cc | 13 +++++---- fml/memory/task_runner_checker.h | 2 ++ fml/memory/task_runner_checker_unittest.cc | 33 +++++++++++++++++++--- fml/task_runner.cc | 16 ++--------- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/fml/memory/task_runner_checker.cc b/fml/memory/task_runner_checker.cc index 590b17eee16d2..b86d443849936 100644 --- a/fml/memory/task_runner_checker.cc +++ b/fml/memory/task_runner_checker.cc @@ -13,22 +13,25 @@ TaskRunnerChecker::~TaskRunnerChecker() = default; bool TaskRunnerChecker::RunsOnCreationTaskRunner() const { FML_CHECK(fml::MessageLoop::IsInitializedForCurrentThread()); - const auto current_queue_id = MessageLoop::GetCurrentTaskQueueId(); + return RunsOnTheSameThread(current_queue_id, initialized_queue_id_); +}; - if (current_queue_id == initialized_queue_id_) { +bool TaskRunnerChecker::RunsOnTheSameThread(TaskQueueId queue_a, + TaskQueueId queue_b) { + if (queue_a == queue_b) { return true; } auto queues = MessageLoopTaskQueues::GetInstance(); - if (queues->Owns(current_queue_id, initialized_queue_id_)) { + if (queues->Owns(queue_a, queue_b)) { return true; } - if (queues->Owns(initialized_queue_id_, current_queue_id)) { + if (queues->Owns(queue_b, queue_a)) { return true; } return false; -}; +} TaskQueueId TaskRunnerChecker::InitTaskQueueId() { MessageLoop::EnsureInitializedForCurrentThread(); diff --git a/fml/memory/task_runner_checker.h b/fml/memory/task_runner_checker.h index 9c2ca8f46696c..cb36dadcff201 100644 --- a/fml/memory/task_runner_checker.h +++ b/fml/memory/task_runner_checker.h @@ -18,6 +18,8 @@ class TaskRunnerChecker final { bool RunsOnCreationTaskRunner() const; + static bool RunsOnTheSameThread(TaskQueueId queue_a, TaskQueueId queue_b); + private: TaskQueueId initialized_queue_id_; diff --git a/fml/memory/task_runner_checker_unittest.cc b/fml/memory/task_runner_checker_unittest.cc index 927c9a248695a..79808acde49d8 100644 --- a/fml/memory/task_runner_checker_unittest.cc +++ b/fml/memory/task_runner_checker_unittest.cc @@ -4,15 +4,17 @@ #define FML_USED_ON_EMBEDDER -#include +#include + +#include #include "flutter/fml/memory/task_runner_checker.h" + #include "flutter/fml/message_loop.h" +#include "flutter/fml/raster_thread_merger.h" #include "flutter/fml/synchronization/count_down_latch.h" #include "flutter/fml/synchronization/waitable_event.h" -#include - namespace fml { namespace testing { @@ -24,7 +26,7 @@ TEST(TaskRunnerCheckerTests, RunsOnCurrentTaskRunner) { TEST(TaskRunnerCheckerTests, FailsTheCheckIfOnDifferentTaskRunner) { TaskRunnerChecker checker; EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true); - fml::MessageLoop* loop = nullptr; + fml::MessageLoop* loop; fml::AutoResetWaitableEvent latch; std::thread anotherThread([&]() { fml::MessageLoop::EnsureInitializedForCurrentThread(); @@ -41,5 +43,28 @@ TEST(TaskRunnerCheckerTests, FailsTheCheckIfOnDifferentTaskRunner) { EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true); } +TEST(TaskRunnerCheckerTests, SameTaskRunnerRunsOnTheSameThread) { + fml::MessageLoop& loop1 = fml::MessageLoop::GetCurrent(); + fml::MessageLoop& loop2 = fml::MessageLoop::GetCurrent(); + TaskQueueId a = loop1.GetTaskRunner()->GetTaskQueueId(); + TaskQueueId b = loop2.GetTaskRunner()->GetTaskQueueId(); + EXPECT_EQ(TaskRunnerChecker::RunsOnTheSameThread(a, b), true); +} + +TEST(TaskRunnerCheckerTests, RunsOnDifferentThreadsReturnsFalse) { + fml::MessageLoop& loop1 = fml::MessageLoop::GetCurrent(); + TaskQueueId a = loop1.GetTaskRunner()->GetTaskQueueId(); + fml::AutoResetWaitableEvent latch; + std::thread anotherThread([&]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + fml::MessageLoop& loop2 = fml::MessageLoop::GetCurrent(); + TaskQueueId b = loop2.GetTaskRunner()->GetTaskQueueId(); + EXPECT_EQ(TaskRunnerChecker::RunsOnTheSameThread(a, b), false); + latch.Signal(); + }); + latch.Wait(); + anotherThread.join(); +} + } // namespace testing } // namespace fml diff --git a/fml/task_runner.cc b/fml/task_runner.cc index c50e14a3ca4b9..66d3bbdbd6ec2 100644 --- a/fml/task_runner.cc +++ b/fml/task_runner.cc @@ -5,6 +5,7 @@ #define FML_USED_ON_EMBEDDER #include "flutter/fml/task_runner.h" +#include "flutter/fml/memory/task_runner_checker.h" #include @@ -47,19 +48,8 @@ bool TaskRunner::RunsTasksOnCurrentThread() { const auto current_queue_id = MessageLoop::GetCurrentTaskQueueId(); const auto loop_queue_id = loop_->GetTaskQueueId(); - if (current_queue_id == loop_queue_id) { - return true; - } - - auto queues = MessageLoopTaskQueues::GetInstance(); - if (queues->Owns(current_queue_id, loop_queue_id)) { - return true; - } - if (queues->Owns(loop_queue_id, current_queue_id)) { - return true; - } - - return false; + return TaskRunnerChecker::RunsOnTheSameThread(current_queue_id, + loop_queue_id); } void TaskRunner::RunNowOrPostTask(fml::RefPtr runner, From e0f4e6829c9664b47fa2b91361a13e57c56377c3 Mon Sep 17 00:00:00 2001 From: cyanglaz Date: Fri, 8 May 2020 13:29:26 -0700 Subject: [PATCH 2/2] add thread merging test --- fml/memory/task_runner_checker.cc | 2 +- fml/memory/task_runner_checker_unittest.cc | 53 +++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/fml/memory/task_runner_checker.cc b/fml/memory/task_runner_checker.cc index b86d443849936..dae642dfea8d5 100644 --- a/fml/memory/task_runner_checker.cc +++ b/fml/memory/task_runner_checker.cc @@ -31,7 +31,7 @@ bool TaskRunnerChecker::RunsOnTheSameThread(TaskQueueId queue_a, return true; } return false; -} +}; TaskQueueId TaskRunnerChecker::InitTaskQueueId() { MessageLoop::EnsureInitializedForCurrentThread(); diff --git a/fml/memory/task_runner_checker_unittest.cc b/fml/memory/task_runner_checker_unittest.cc index 79808acde49d8..5ff93873be9cd 100644 --- a/fml/memory/task_runner_checker_unittest.cc +++ b/fml/memory/task_runner_checker_unittest.cc @@ -4,7 +4,6 @@ #define FML_USED_ON_EMBEDDER - #include #include @@ -26,7 +25,7 @@ TEST(TaskRunnerCheckerTests, RunsOnCurrentTaskRunner) { TEST(TaskRunnerCheckerTests, FailsTheCheckIfOnDifferentTaskRunner) { TaskRunnerChecker checker; EXPECT_EQ(checker.RunsOnCreationTaskRunner(), true); - fml::MessageLoop* loop; + fml::MessageLoop* loop = nullptr; fml::AutoResetWaitableEvent latch; std::thread anotherThread([&]() { fml::MessageLoop::EnsureInitializedForCurrentThread(); @@ -66,5 +65,55 @@ TEST(TaskRunnerCheckerTests, RunsOnDifferentThreadsReturnsFalse) { anotherThread.join(); } +TEST(TaskRunnerCheckerTests, MergedTaskRunnersRunsOnTheSameThread) { + fml::MessageLoop* loop1 = nullptr; + fml::AutoResetWaitableEvent latch1; + fml::AutoResetWaitableEvent term1; + std::thread thread1([&loop1, &latch1, &term1]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop1 = &fml::MessageLoop::GetCurrent(); + latch1.Signal(); + term1.Wait(); + }); + + fml::MessageLoop* loop2 = nullptr; + fml::AutoResetWaitableEvent latch2; + fml::AutoResetWaitableEvent term2; + std::thread thread2([&loop2, &latch2, &term2]() { + fml::MessageLoop::EnsureInitializedForCurrentThread(); + loop2 = &fml::MessageLoop::GetCurrent(); + latch2.Signal(); + term2.Wait(); + }); + + latch1.Wait(); + latch2.Wait(); + fml::TaskQueueId qid1 = loop1->GetTaskRunner()->GetTaskQueueId(); + fml::TaskQueueId qid2 = loop2->GetTaskRunner()->GetTaskQueueId(); + const auto raster_thread_merger_ = + fml::MakeRefCounted(qid1, qid2); + const int kNumFramesMerged = 5; + + raster_thread_merger_->MergeWithLease(kNumFramesMerged); + + // merged, running on the same thread + EXPECT_EQ(TaskRunnerChecker::RunsOnTheSameThread(qid1, qid2), true); + + for (int i = 0; i < kNumFramesMerged; i++) { + ASSERT_TRUE(raster_thread_merger_->IsMerged()); + raster_thread_merger_->DecrementLease(); + } + + ASSERT_FALSE(raster_thread_merger_->IsMerged()); + + // un-merged, not running on the same thread + EXPECT_EQ(TaskRunnerChecker::RunsOnTheSameThread(qid1, qid2), false); + + term1.Signal(); + term2.Signal(); + thread1.join(); + thread2.join(); +} + } // namespace testing } // namespace fml