From fd51492d89ca401f08c305fb3c6c1d792bed56ee Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 23 Sep 2024 10:02:55 -0700 Subject: [PATCH 1/5] [engine] set platform thread name to ui. --- shell/platform/android/android_shell_holder.cc | 13 +++++++++---- .../darwin/ios/framework/Source/FlutterEngine.mm | 11 +++++++---- .../ios/framework/Source/FlutterEngineTest.mm | 3 +++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 49a969627d476..d30a69b5edcef 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -94,12 +94,17 @@ AndroidShellHolder::AndroidShellHolder( flutter::ThreadHost::ThreadHostConfig host_config( thread_label, mask, AndroidPlatformThreadConfigSetter); + + auto ui_config = fml::Thread::ThreadConfig( + flutter::ThreadHost::ThreadHostConfig::MakeThreadName( + flutter::ThreadHost::Type::kUi, thread_label), + fml::Thread::ThreadPriority::kDisplay); if (!settings.merged_platform_ui_thread) { - host_config.ui_config = fml::Thread::ThreadConfig( - flutter::ThreadHost::ThreadHostConfig::MakeThreadName( - flutter::ThreadHost::Type::kUi, thread_label), - fml::Thread::ThreadPriority::kDisplay); + host_config.ui_config = ui_config; + } else { + fml::Thread::SetCurrentThreadName(ui_config); } + host_config.raster_config = fml::Thread::ThreadConfig( flutter::ThreadHost::ThreadHostConfig::MakeThreadName( flutter::ThreadHost::Type::kRaster, thread_label), diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index b1ca5e66ae6d1..3b72fe9a93dd2 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -802,11 +802,14 @@ + (NSString*)generateThreadLabel:(NSString*)labelPrefix { flutter::ThreadHost::ThreadHostConfig host_config(thread_label.UTF8String, threadHostType, IOSPlatformThreadConfigSetter); + auto ui_config = + fml::Thread::ThreadConfig(flutter::ThreadHost::ThreadHostConfig::MakeThreadName( + flutter::ThreadHost::Type::kUi, thread_label.UTF8String), + fml::Thread::ThreadPriority::kDisplay); if (!settings.enable_impeller) { - host_config.ui_config = - fml::Thread::ThreadConfig(flutter::ThreadHost::ThreadHostConfig::MakeThreadName( - flutter::ThreadHost::Type::kUi, thread_label.UTF8String), - fml::Thread::ThreadPriority::kDisplay); + host_config.ui_config = ui_config; + } else { + fml::Thread::SetCurrentThreadName(ui_config); } host_config.raster_config = diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 26ad1e1c52f55..56f6604297de0 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -490,6 +490,9 @@ - (void)testCanNotUnMergePlatformAndUIThread { XCTAssertEqual(engine.shell.GetTaskRunners().GetUITaskRunner(), engine.shell.GetTaskRunners().GetPlatformTaskRunner()); + + NSString* s = [NSString stringWithFormat:@"%@", [NSThread currentThread]]; + XCTAssertEqual(s, @"io.flutter.ui"); } @end From 0d9ee3ec07a8cc297d600dcead00db048fe015c5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 24 Sep 2024 11:23:38 -0700 Subject: [PATCH 2/5] ++ --- shell/platform/android/android_shell_holder.cc | 6 ++++-- .../darwin/ios/framework/Source/FlutterEngine.mm | 9 ++++++--- .../darwin/ios/framework/Source/FlutterEngineTest.mm | 3 --- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index d30a69b5edcef..538412f717f01 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -89,8 +89,10 @@ AndroidShellHolder::AndroidShellHolder( static size_t thread_host_count = 1; auto thread_label = std::to_string(thread_host_count++); - auto mask = - ThreadHost::Type::kUi | ThreadHost::Type::kRaster | ThreadHost::Type::kIo; + auto mask = ThreadHost::Type::kRaster | ThreadHost::Type::kIo; + if (!settings.merged_platform_ui_thread) { + mask |= ThreadHost::Type::kUi; + } flutter::ThreadHost::ThreadHostConfig host_config( thread_label, mask, AndroidPlatformThreadConfigSetter); diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 3b72fe9a93dd2..60129bc926e21 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -45,6 +45,7 @@ /// Inheriting ThreadConfigurer and use iOS platform thread API to configure the thread priorities /// Using iOS platform thread API to configure thread priority static void IOSPlatformThreadConfigSetter(const fml::Thread::ThreadConfig& config) { + FML_LOG(ERROR) << "IOSPlatformThreadConfigSetter: " << config.name; // set thread name fml::Thread::SetCurrentThreadName(config); @@ -792,8 +793,10 @@ + (NSString*)generateThreadLabel:(NSString*)labelPrefix { // initialized. fml::MessageLoop::EnsureInitializedForCurrentThread(); - uint32_t threadHostType = flutter::ThreadHost::Type::kUi | flutter::ThreadHost::Type::kRaster | - flutter::ThreadHost::Type::kIo; + uint32_t threadHostType = flutter::ThreadHost::Type::kRaster | flutter::ThreadHost::Type::kIo; + if (!settings.enable_impeller) { + threadHostType |= flutter::ThreadHost::Type::kUi; + } if ([FlutterEngine isProfilerEnabled]) { threadHostType = threadHostType | flutter::ThreadHost::Type::kProfiler; @@ -809,7 +812,7 @@ + (NSString*)generateThreadLabel:(NSString*)labelPrefix { if (!settings.enable_impeller) { host_config.ui_config = ui_config; } else { - fml::Thread::SetCurrentThreadName(ui_config); + // fml::Thread::SetCurrentThreadName(ui_config); } host_config.raster_config = diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm index 56f6604297de0..26ad1e1c52f55 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngineTest.mm @@ -490,9 +490,6 @@ - (void)testCanNotUnMergePlatformAndUIThread { XCTAssertEqual(engine.shell.GetTaskRunners().GetUITaskRunner(), engine.shell.GetTaskRunners().GetPlatformTaskRunner()); - - NSString* s = [NSString stringWithFormat:@"%@", [NSThread currentThread]]; - XCTAssertEqual(s, @"io.flutter.ui"); } @end From 9d2491b5cacde522b7bb72bc166b9b49b4d714a1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 24 Sep 2024 17:46:56 -0700 Subject: [PATCH 3/5] testing --- runtime/dart_isolate.cc | 12 +++- runtime/dart_isolate_unittests.cc | 61 +++++++++++++++++++ .../platform/android/android_shell_holder.cc | 9 +-- .../ios/framework/Source/FlutterEngine.mm | 8 +-- 4 files changed, 72 insertions(+), 18 deletions(-) diff --git a/runtime/dart_isolate.cc b/runtime/dart_isolate.cc index 3dc8403506f7f..4af4758d6154a 100644 --- a/runtime/dart_isolate.cc +++ b/runtime/dart_isolate.cc @@ -633,10 +633,16 @@ bool DartIsolate::UpdateThreadPoolNames() const { } if (auto task_runner = task_runners.GetPlatformTaskRunner()) { + bool is_merged_platform_ui_thread = + task_runner == task_runners.GetUITaskRunner(); + std::string label; + if (is_merged_platform_ui_thread) { + label = task_runners.GetLabel() + std::string{".ui"}; + } else { + label = task_runners.GetLabel() + std::string{".platform"}; + } task_runner->PostTask( - [label = task_runners.GetLabel() + std::string{".platform"}]() { - Dart_SetThreadName(label.c_str()); - }); + [label = std::move(label)]() { Dart_SetThreadName(label.c_str()); }); } return true; diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index a030d99ddda5c..20bb823e905c2 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -17,6 +17,7 @@ #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/scopes/dart_isolate_scope.h" +#include "vm/os_thread.h" // CREATE_NATIVE_ENTRY is leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -1185,6 +1186,66 @@ TEST_F(DartIsolateTest, PlatformIsolateMainThrowsError) { // root isolate will be auto-shutdown } +TEST_F(DartIsolateTest, DartPlatformThreadNameIsSetWithMergedThreads) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + + const auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + auto platform_thread = CreateNewThread(); + auto other_thread = CreateNewThread(); + // Set platform and UI thread runner to the same thread. + TaskRunners task_runners("io.flutter", // + platform_thread, // + other_thread, // + platform_thread, // + other_thread // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", + {}, GetDefaultKernelFilePath()); + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + + fml::AutoResetWaitableEvent latch; + std::string thread_name; + platform_thread->PostTask([&latch, &thread_name]() { + dart::OSThread* thread = dart::OSThread::Current(); + thread_name = std::string(thread->name()); + latch.Signal(); + }); + latch.Wait(); + + EXPECT_EQ(thread_name, "io.flutter.ui"); +} + +TEST_F(DartIsolateTest, DartPlatformThreadNameIsSet) { + ASSERT_FALSE(DartVMRef::IsInstanceRunning()); + + const auto settings = CreateSettingsForFixture(); + auto vm_ref = DartVMRef::Create(settings); + // Set platform and UI thread runner to the same thread. + TaskRunners task_runners("io.flutter", // + CreateNewThread(), // + CreateNewThread(), // + CreateNewThread(), // + CreateNewThread() // + ); + auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", + {}, GetDefaultKernelFilePath()); + ASSERT_TRUE(isolate); + ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); + + fml::AutoResetWaitableEvent latch; + std::string thread_name; + task_runners.GetPlatformTaskRunner()->PostTask([&latch, &thread_name]() { + dart::OSThread* thread = dart::OSThread::Current(); + thread_name = std::string(thread->name()); + latch.Signal(); + }); + latch.Wait(); + + EXPECT_EQ(thread_name, "io.flutter.platform"); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/android/android_shell_holder.cc b/shell/platform/android/android_shell_holder.cc index 538412f717f01..e86effe09bd92 100644 --- a/shell/platform/android/android_shell_holder.cc +++ b/shell/platform/android/android_shell_holder.cc @@ -96,17 +96,10 @@ AndroidShellHolder::AndroidShellHolder( flutter::ThreadHost::ThreadHostConfig host_config( thread_label, mask, AndroidPlatformThreadConfigSetter); - - auto ui_config = fml::Thread::ThreadConfig( + host_config.ui_config = fml::Thread::ThreadConfig( flutter::ThreadHost::ThreadHostConfig::MakeThreadName( flutter::ThreadHost::Type::kUi, thread_label), fml::Thread::ThreadPriority::kDisplay); - if (!settings.merged_platform_ui_thread) { - host_config.ui_config = ui_config; - } else { - fml::Thread::SetCurrentThreadName(ui_config); - } - host_config.raster_config = fml::Thread::ThreadConfig( flutter::ThreadHost::ThreadHostConfig::MakeThreadName( flutter::ThreadHost::Type::kRaster, thread_label), diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 60129bc926e21..996b1ae977883 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -805,16 +805,10 @@ + (NSString*)generateThreadLabel:(NSString*)labelPrefix { flutter::ThreadHost::ThreadHostConfig host_config(thread_label.UTF8String, threadHostType, IOSPlatformThreadConfigSetter); - auto ui_config = + host_config.ui_config = fml::Thread::ThreadConfig(flutter::ThreadHost::ThreadHostConfig::MakeThreadName( flutter::ThreadHost::Type::kUi, thread_label.UTF8String), fml::Thread::ThreadPriority::kDisplay); - if (!settings.enable_impeller) { - host_config.ui_config = ui_config; - } else { - // fml::Thread::SetCurrentThreadName(ui_config); - } - host_config.raster_config = fml::Thread::ThreadConfig(flutter::ThreadHost::ThreadHostConfig::MakeThreadName( flutter::ThreadHost::Type::kRaster, thread_label.UTF8String), From 017bf6947478496c5159b02837ffe06bc26a36b9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 24 Sep 2024 17:51:37 -0700 Subject: [PATCH 4/5] ++ --- shell/platform/darwin/ios/framework/Source/FlutterEngine.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm index 996b1ae977883..713d4fd0e288f 100644 --- a/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm +++ b/shell/platform/darwin/ios/framework/Source/FlutterEngine.mm @@ -45,7 +45,6 @@ /// Inheriting ThreadConfigurer and use iOS platform thread API to configure the thread priorities /// Using iOS platform thread API to configure thread priority static void IOSPlatformThreadConfigSetter(const fml::Thread::ThreadConfig& config) { - FML_LOG(ERROR) << "IOSPlatformThreadConfigSetter: " << config.name; // set thread name fml::Thread::SetCurrentThreadName(config); From 6c55442d87af7317a9c07c838879d2902591f75d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 25 Sep 2024 09:53:33 -0700 Subject: [PATCH 5/5] ++ --- runtime/dart_isolate_unittests.cc | 61 ------------------------------- 1 file changed, 61 deletions(-) diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index 20bb823e905c2..a030d99ddda5c 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -17,7 +17,6 @@ #include "third_party/dart/runtime/include/dart_api.h" #include "third_party/tonic/converter/dart_converter.h" #include "third_party/tonic/scopes/dart_isolate_scope.h" -#include "vm/os_thread.h" // CREATE_NATIVE_ENTRY is leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -1186,66 +1185,6 @@ TEST_F(DartIsolateTest, PlatformIsolateMainThrowsError) { // root isolate will be auto-shutdown } -TEST_F(DartIsolateTest, DartPlatformThreadNameIsSetWithMergedThreads) { - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - - const auto settings = CreateSettingsForFixture(); - auto vm_ref = DartVMRef::Create(settings); - auto platform_thread = CreateNewThread(); - auto other_thread = CreateNewThread(); - // Set platform and UI thread runner to the same thread. - TaskRunners task_runners("io.flutter", // - platform_thread, // - other_thread, // - platform_thread, // - other_thread // - ); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, GetDefaultKernelFilePath()); - ASSERT_TRUE(isolate); - ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); - - fml::AutoResetWaitableEvent latch; - std::string thread_name; - platform_thread->PostTask([&latch, &thread_name]() { - dart::OSThread* thread = dart::OSThread::Current(); - thread_name = std::string(thread->name()); - latch.Signal(); - }); - latch.Wait(); - - EXPECT_EQ(thread_name, "io.flutter.ui"); -} - -TEST_F(DartIsolateTest, DartPlatformThreadNameIsSet) { - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - - const auto settings = CreateSettingsForFixture(); - auto vm_ref = DartVMRef::Create(settings); - // Set platform and UI thread runner to the same thread. - TaskRunners task_runners("io.flutter", // - CreateNewThread(), // - CreateNewThread(), // - CreateNewThread(), // - CreateNewThread() // - ); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, "main", - {}, GetDefaultKernelFilePath()); - ASSERT_TRUE(isolate); - ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); - - fml::AutoResetWaitableEvent latch; - std::string thread_name; - task_runners.GetPlatformTaskRunner()->PostTask([&latch, &thread_name]() { - dart::OSThread* thread = dart::OSThread::Current(); - thread_name = std::string(thread->name()); - latch.Signal(); - }); - latch.Wait(); - - EXPECT_EQ(thread_name, "io.flutter.platform"); -} - } // namespace testing } // namespace flutter