From 591d149f971b4667bd30f2ebc5aaac646261ccd2 Mon Sep 17 00:00:00 2001 From: George Wright Date: Tue, 22 Oct 2019 14:10:57 -0700 Subject: [PATCH] Make flutter_tester support multithreaded testing, and run all Dart tests in both single and multithreaded configurations (#13273) Make flutter_tester support multithreaded testing, and run all Dart tests in both single and multithreaded configurations This also modifies Shell::GetUIIsolateLastError() and Shell::EngineHasLivePorts() so that they must be called from the UI task runner. --- shell/common/shell.cc | 28 +++----------- shell/common/switches.h | 6 +++ shell/testing/tester_main.cc | 73 +++++++++++++++++++++++++++--------- testing/run_tests.py | 19 +++++++--- 4 files changed, 81 insertions(+), 45 deletions(-) diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 4151ef976..7ead524f9 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -432,14 +432,12 @@ void Shell::RunEngine(RunConfiguration run_configuration, std::optional Shell::GetUIIsolateLastError() const { FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); - // We're using the unique_ptr here because we're sure we're on the Platform - // Thread and callers expect this to be synchronous. - if (!engine_) { + if (!weak_engine_) { return std::nullopt; } - switch (engine_->GetUIIsolateLastError()) { + switch (weak_engine_->GetUIIsolateLastError()) { case tonic::kCompilationErrorType: return DartErrorCode::CompilationError; case tonic::kApiErrorType: @@ -454,27 +452,13 @@ std::optional Shell::GetUIIsolateLastError() const { bool Shell::EngineHasLivePorts() const { FML_DCHECK(is_setup_); - FML_DCHECK(task_runners_.GetPlatformTaskRunner()->RunsTasksOnCurrentThread()); + FML_DCHECK(task_runners_.GetUITaskRunner()->RunsTasksOnCurrentThread()); - // We're using the unique_ptr here because we're sure we're on the Platform - // Thread and callers expect this to be synchronous. - if (!engine_) { + if (!weak_engine_) { return false; } - std::promise ui_isolate_has_live_ports_promise; - auto ui_isolate_has_live_ports_future = - ui_isolate_has_live_ports_promise.get_future(); - auto ui_task_runner = task_runners_.GetUITaskRunner(); - - fml::TaskRunner::RunNowOrPostTask( - ui_task_runner, - [&ui_isolate_has_live_ports_promise, engine = engine_->GetWeakPtr()]() { - ui_isolate_has_live_ports_promise.set_value( - engine->UIIsolateHasLivePorts()); - }); - - return ui_isolate_has_live_ports_future.get(); + return weak_engine_->UIIsolateHasLivePorts(); } bool Shell::IsSetup() const { diff --git a/shell/common/switches.h b/shell/common/switches.h index 29e4c81ea..45c018cac 100644 --- a/shell/common/switches.h +++ b/shell/common/switches.h @@ -165,6 +165,12 @@ DEF_SWITCH(DisableDartAsserts, "disabled. This flag may be specified if the user wishes to run " "with assertions disabled in the debug product mode (i.e. with JIT " "or DBC).") +DEF_SWITCH( + ForceMultithreading, + "force-multithreading", + "Uses separate threads for the platform, UI, GPU and IO task runners. " + "By default, a single thread is used for all task runners. Only available " + "in the flutter_tester.") DEF_SWITCHES_END void PrintUsage(const std::string& executable_name); diff --git a/shell/testing/tester_main.cc b/shell/testing/tester_main.cc index c1560e982..3f31fd6ce 100644 --- a/shell/testing/tester_main.cc +++ b/shell/testing/tester_main.cc @@ -60,8 +60,9 @@ class ScriptCompletionTaskObserver { if (!has_terminated) { // Only try to terminate the loop once. has_terminated = true; - main_task_runner_->PostTask( - []() { fml::MessageLoop::GetCurrent().Terminate(); }); + fml::TaskRunner::RunNowOrPostTask(main_task_runner_, []() { + fml::MessageLoop::GetCurrent().Terminate(); + }); } } @@ -90,8 +91,10 @@ static void UnblockSIGPROF() { #endif // defined(OS_POSIX) } -int RunTester(const flutter::Settings& settings, bool run_forever) { - const auto thread_label = "io.flutter.test"; +int RunTester(const flutter::Settings& settings, + bool run_forever, + bool multithreaded) { + const auto thread_label = "io.flutter.test."; // Necessary if we want to use the CPU profiler on the main isolate's mutator // thread. @@ -104,12 +107,30 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { auto current_task_runner = fml::MessageLoop::GetCurrent().GetTaskRunner(); - // Setup a single threaded test runner configuration. + std::unique_ptr threadhost; + fml::RefPtr platform_task_runner; + fml::RefPtr gpu_task_runner; + fml::RefPtr ui_task_runner; + fml::RefPtr io_task_runner; + + if (multithreaded) { + threadhost = std::make_unique( + thread_label, ThreadHost::Type::Platform | ThreadHost::Type::IO | + ThreadHost::Type::UI | ThreadHost::Type::GPU); + platform_task_runner = current_task_runner; + gpu_task_runner = threadhost->gpu_thread->GetTaskRunner(); + ui_task_runner = threadhost->ui_thread->GetTaskRunner(); + io_task_runner = threadhost->io_thread->GetTaskRunner(); + } else { + platform_task_runner = gpu_task_runner = ui_task_runner = io_task_runner = + current_task_runner; + } + const flutter::TaskRunners task_runners(thread_label, // dart thread label - current_task_runner, // platform - current_task_runner, // gpu - current_task_runner, // ui - current_task_runner // io + platform_task_runner, // platform + gpu_task_runner, // gpu + ui_task_runner, // ui + io_task_runner // io ); Shell::CreateCallback on_create_platform_view = @@ -187,14 +208,28 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { bool engine_did_run = false; - fml::MessageLoop::GetCurrent().AddTaskObserver( - reinterpret_cast(&completion_observer), - [&completion_observer]() { completion_observer.DidProcessTask(); }); + fml::AutoResetWaitableEvent latch; + auto task_observer_add = [&completion_observer]() { + fml::MessageLoop::GetCurrent().AddTaskObserver( + reinterpret_cast(&completion_observer), + [&completion_observer]() { completion_observer.DidProcessTask(); }); + }; + + auto task_observer_remove = [&completion_observer, &latch]() { + fml::MessageLoop::GetCurrent().RemoveTaskObserver( + reinterpret_cast(&completion_observer)); + latch.Signal(); + }; shell->RunEngine(std::move(run_configuration), - [&engine_did_run](Engine::RunStatus run_status) mutable { + [&engine_did_run, &ui_task_runner, + &task_observer_add](Engine::RunStatus run_status) mutable { if (run_status != flutter::Engine::RunStatus::Failure) { engine_did_run = true; + // Now that our engine is initialized we can install the + // ScriptCompletionTaskObserver + fml::TaskRunner::RunNowOrPostTask(ui_task_runner, + task_observer_add); } }); @@ -209,8 +244,8 @@ int RunTester(const flutter::Settings& settings, bool run_forever) { // Cleanup the completion observer synchronously as it is living on the // stack. - fml::MessageLoop::GetCurrent().RemoveTaskObserver( - reinterpret_cast(&completion_observer)); + fml::TaskRunner::RunNowOrPostTask(ui_task_runner, task_observer_remove); + latch.Wait(); if (!engine_did_run) { // If the engine itself didn't have a chance to run, there is no point in @@ -270,7 +305,9 @@ int main(int argc, char* argv[]) { return true; }; - return flutter::RunTester( - settings, command_line.HasOption( - flutter::FlagForSwitch(flutter::Switch::RunForever))); + return flutter::RunTester(settings, + command_line.HasOption(flutter::FlagForSwitch( + flutter::Switch::RunForever)), + command_line.HasOption(flutter::FlagForSwitch( + flutter::Switch::ForceMultithreading))); } diff --git a/testing/run_tests.py b/testing/run_tests.py index 3596c84f5..29639d29c 100755 --- a/testing/run_tests.py +++ b/testing/run_tests.py @@ -162,18 +162,26 @@ def SnapshotTest(build_dir, dart_file, kernel_file_output, verbose_dart_snapshot assert os.path.exists(kernel_file_output) -def RunDartTest(build_dir, dart_file, verbose_dart_snapshot): +def RunDartTest(build_dir, dart_file, verbose_dart_snapshot, multithreaded): kernel_file_name = os.path.basename(dart_file) + '.kernel.dill' kernel_file_output = os.path.join(out_dir, kernel_file_name) SnapshotTest(build_dir, dart_file, kernel_file_output, verbose_dart_snapshot) - print "Running test '%s' using 'flutter_tester'" % kernel_file_name - RunEngineExecutable(build_dir, 'flutter_tester', None, [ + command_args = [ '--disable-observatory', '--use-test-fonts', kernel_file_output - ]) + ] + + if multithreaded: + threading = 'multithreaded' + command_args.insert(0, '--force-multithreading') + else: + threading = 'single-threaded' + + print "Running test '%s' using 'flutter_tester' (%s)" % (kernel_file_name, threading) + RunEngineExecutable(build_dir, 'flutter_tester', None, command_args) def RunPubGet(build_dir, directory): print "Running 'pub get' in the tests directory %s" % dart_tests_dir @@ -287,7 +295,8 @@ def RunDartTests(build_dir, filter, verbose_dart_snapshot): print "Skipping %s due to filter." % dart_test_file else: print "Testing dart file %s" % dart_test_file - RunDartTest(build_dir, dart_test_file, verbose_dart_snapshot) + RunDartTest(build_dir, dart_test_file, verbose_dart_snapshot, True) + RunDartTest(build_dir, dart_test_file, verbose_dart_snapshot, False) def main(): parser = argparse.ArgumentParser();