From ff7a195248ef35804e2d8903abb30fc3c4c90666 Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Wed, 8 Sep 2021 10:28:13 -0700 Subject: [PATCH] Revert "Add benchmarks to measure dart -> native time (#28492)" This reverts commit 3155b000496ca7608c5c324f4501a3f80c9a1a87. --- ci/licenses_golden/licenses_flutter | 1 - runtime/dart_isolate_unittests.cc | 16 ++-- shell/common/BUILD.gn | 6 +- shell/common/dart_native_benchmarks.cc | 103 ------------------------- shell/common/fixtures/shell_test.dart | 6 -- testing/BUILD.gn | 2 - testing/dart_fixture.cc | 72 ----------------- testing/dart_fixture.h | 49 ------------ testing/fixture_test.cc | 62 ++++++++++++++- testing/fixture_test.h | 27 ++++++- 10 files changed, 94 insertions(+), 250 deletions(-) delete mode 100644 shell/common/dart_native_benchmarks.cc delete mode 100644 testing/dart_fixture.cc delete mode 100644 testing/dart_fixture.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index e561b1cadafbb..1d5cee942105c 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -691,7 +691,6 @@ FILE: ../../../flutter/shell/common/canvas_spy.h FILE: ../../../flutter/shell/common/canvas_spy_unittests.cc FILE: ../../../flutter/shell/common/context_options.cc FILE: ../../../flutter/shell/common/context_options.h -FILE: ../../../flutter/shell/common/dart_native_benchmarks.cc FILE: ../../../flutter/shell/common/display.h FILE: ../../../flutter/shell/common/display_manager.cc FILE: ../../../flutter/shell/common/display_manager.h diff --git a/runtime/dart_isolate_unittests.cc b/runtime/dart_isolate_unittests.cc index bd66b1d195fbf..e23408b332b97 100644 --- a/runtime/dart_isolate_unittests.cc +++ b/runtime/dart_isolate_unittests.cc @@ -247,9 +247,11 @@ TEST_F(DartIsolateTest, CanRunDartCodeCodeSynchronously) { TEST_F(DartIsolateTest, CanRegisterNativeCallback) { ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - AddNativeCallback( - "NotifyNative", - CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { Signal(); }))); + AddNativeCallback("NotifyNative", + CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { + FML_LOG(ERROR) << "Hello from Dart!"; + Signal(); + }))); const auto settings = CreateSettingsForFixture(); auto vm_ref = DartVMRef::Create(settings); auto thread = CreateNewThread(); @@ -522,9 +524,11 @@ TEST_F(DartIsolateTest, DISABLED_ValidLoadingUnitSucceeds) { } ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - AddNativeCallback( - "NotifyNative", - CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { Signal(); }))); + AddNativeCallback("NotifyNative", + CREATE_NATIVE_ENTRY(([this](Dart_NativeArguments args) { + FML_LOG(ERROR) << "Hello from Dart!"; + Signal(); + }))); AddNativeCallback( "NotifySuccess", CREATE_NATIVE_ENTRY([this](Dart_NativeArguments args) { auto bool_handle = Dart_GetNativeArgument(args, 0); diff --git a/shell/common/BUILD.gn b/shell/common/BUILD.gn index b8d00101df091..071a9ed29e1fa 100644 --- a/shell/common/BUILD.gn +++ b/shell/common/BUILD.gn @@ -158,17 +158,13 @@ if (enable_unittests) { } shell_host_executable("shell_benchmarks") { - sources = [ - "dart_native_benchmarks.cc", - "shell_benchmarks.cc", - ] + sources = [ "shell_benchmarks.cc" ] deps = [ ":shell_unittests_fixtures", "//flutter/benchmarking", "//flutter/flow", "//flutter/testing:dart", - "//flutter/testing:fixture_test", "//flutter/testing:testing_lib", ] } diff --git a/shell/common/dart_native_benchmarks.cc b/shell/common/dart_native_benchmarks.cc deleted file mode 100644 index c9875a0d8b0d5..0000000000000 --- a/shell/common/dart_native_benchmarks.cc +++ /dev/null @@ -1,103 +0,0 @@ -// 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 "flutter/shell/common/shell.h" - -#include "flutter/benchmarking/benchmarking.h" -#include "flutter/shell/common/thread_host.h" -#include "flutter/testing/dart_fixture.h" -#include "flutter/testing/dart_isolate_runner.h" -#include "flutter/testing/testing.h" -#include "fml/synchronization/count_down_latch.h" -#include "runtime/dart_vm_lifecycle.h" - -namespace flutter::testing { - -class DartNativeBenchmarks : public DartFixture, public benchmark::Fixture { - public: - DartNativeBenchmarks() : DartFixture() {} - - void SetUp(const ::benchmark::State& state) {} - - void TearDown(const ::benchmark::State& state) {} - - private: - FML_DISALLOW_COPY_AND_ASSIGN(DartNativeBenchmarks); -}; - -BENCHMARK_F(DartNativeBenchmarks, TimeToFirstNativeMessageFromIsolateInNewVM) -(benchmark::State& st) { - while (st.KeepRunning()) { - fml::AutoResetWaitableEvent latch; - st.PauseTiming(); - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - AddNativeCallback("NotifyNative", - CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) { - latch.Signal(); - }))); - - const auto settings = CreateSettingsForFixture(); - DartVMRef vm_ref = DartVMRef::Create(settings); - - ThreadHost thread_host("io.flutter.test.DartNativeBenchmarks.", - ThreadHost::Type::Platform | ThreadHost::Type::IO | - ThreadHost::Type::UI); - TaskRunners task_runners( - "test", - thread_host.platform_thread->GetTaskRunner(), // platform - thread_host.platform_thread->GetTaskRunner(), // raster - thread_host.ui_thread->GetTaskRunner(), // ui - thread_host.io_thread->GetTaskRunner() // io - ); - - { - st.ResumeTiming(); - auto isolate = - RunDartCodeInIsolate(vm_ref, settings, task_runners, "notifyNative", - {}, GetDefaultKernelFilePath()); - ASSERT_TRUE(isolate); - ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); - latch.Wait(); - } - } -} - -BENCHMARK_F(DartNativeBenchmarks, MultipleDartToNativeMessages) -(benchmark::State& st) { - while (st.KeepRunning()) { - fml::CountDownLatch latch(1000); - st.PauseTiming(); - ASSERT_FALSE(DartVMRef::IsInstanceRunning()); - AddNativeCallback("NotifyNative", - CREATE_NATIVE_ENTRY(([&latch](Dart_NativeArguments args) { - latch.CountDown(); - }))); - - const auto settings = CreateSettingsForFixture(); - DartVMRef vm_ref = DartVMRef::Create(settings); - - ThreadHost thread_host("io.flutter.test.DartNativeBenchmarks.", - ThreadHost::Type::Platform | ThreadHost::Type::IO | - ThreadHost::Type::UI); - TaskRunners task_runners( - "test", - thread_host.platform_thread->GetTaskRunner(), // platform - thread_host.platform_thread->GetTaskRunner(), // raster - thread_host.ui_thread->GetTaskRunner(), // ui - thread_host.io_thread->GetTaskRunner() // io - ); - - { - st.ResumeTiming(); - auto isolate = RunDartCodeInIsolate(vm_ref, settings, task_runners, - "thousandCallsToNative", {}, - GetDefaultKernelFilePath()); - ASSERT_TRUE(isolate); - ASSERT_EQ(isolate->get()->GetPhase(), DartIsolate::Phase::Running); - latch.Wait(); - } - } -} - -} // namespace flutter::testing diff --git a/shell/common/fixtures/shell_test.dart b/shell/common/fixtures/shell_test.dart index f294575229c4c..dd3586fc33176 100644 --- a/shell/common/fixtures/shell_test.dart +++ b/shell/common/fixtures/shell_test.dart @@ -75,12 +75,6 @@ void sayHiFromFixturesAreFunctionalMain() native 'SayHiFromFixturesAreFunctional void notifyNative() native 'NotifyNative'; -void thousandCallsToNative() { - for (int i = 0; i < 1000; i++) { - notifyNative(); - } -} - void secondaryIsolateMain(String message) { print('Secondary isolate got message: ' + message); notifyNative(); diff --git a/testing/BUILD.gn b/testing/BUILD.gn index e21ef73bc9eba..00bfc0ff16b0c 100644 --- a/testing/BUILD.gn +++ b/testing/BUILD.gn @@ -90,8 +90,6 @@ source_set("fixture_test") { testonly = true sources = [ - "dart_fixture.cc", - "dart_fixture.h", "fixture_test.cc", "fixture_test.h", ] diff --git a/testing/dart_fixture.cc b/testing/dart_fixture.cc deleted file mode 100644 index 4e86a33e33c7f..0000000000000 --- a/testing/dart_fixture.cc +++ /dev/null @@ -1,72 +0,0 @@ -// 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 "flutter/testing/dart_fixture.h" - -namespace flutter::testing { - -DartFixture::DartFixture() - : DartFixture("kernel_blob.bin", - kDefaultAOTAppELFFileName, - kDefaultAOTAppELFSplitFileName) {} - -DartFixture::DartFixture(std::string kernel_filename, - std::string elf_filename, - std::string elf_split_filename) - : native_resolver_(std::make_shared()), - split_aot_symbols_( - LoadELFSplitSymbolFromFixturesIfNeccessary(elf_split_filename)), - kernel_filename_(kernel_filename), - assets_dir_(fml::OpenDirectory(GetFixturesPath(), - false, - fml::FilePermission::kRead)), - aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary(elf_filename)) {} - -Settings DartFixture::CreateSettingsForFixture() { - Settings settings; - settings.leak_vm = false; - settings.task_observer_add = [](intptr_t, fml::closure) {}; - settings.task_observer_remove = [](intptr_t) {}; - settings.isolate_create_callback = [this]() { - native_resolver_->SetNativeResolverForIsolate(); - }; - settings.enable_observatory = false; - SetSnapshotsAndAssets(settings); - return settings; -} - -void DartFixture::SetSnapshotsAndAssets(Settings& settings) { - if (!assets_dir_.is_valid()) { - return; - } - - settings.assets_dir = assets_dir_.get(); - - // In JIT execution, all snapshots are present within the binary itself and - // don't need to be explicitly supplied by the embedder. In AOT, these - // snapshots will be present in the application AOT dylib. - if (DartVM::IsRunningPrecompiledCode()) { - FML_CHECK(PrepareSettingsForAOTWithSymbols(settings, aot_symbols_)); - } else { - settings.application_kernels = [this]() -> Mappings { - std::vector> kernel_mappings; - auto kernel_mapping = - fml::FileMapping::CreateReadOnly(assets_dir_, kernel_filename_); - if (!kernel_mapping || !kernel_mapping->IsValid()) { - FML_LOG(ERROR) << "Could not find kernel blob for test fixture not " - "running in precompiled mode."; - return kernel_mappings; - } - kernel_mappings.emplace_back(std::move(kernel_mapping)); - return kernel_mappings; - }; - } -} - -void DartFixture::AddNativeCallback(std::string name, - Dart_NativeFunction callback) { - native_resolver_->AddNativeCallback(std::move(name), callback); -} - -} // namespace flutter::testing diff --git a/testing/dart_fixture.h b/testing/dart_fixture.h deleted file mode 100644 index 06c00e86e97d0..0000000000000 --- a/testing/dart_fixture.h +++ /dev/null @@ -1,49 +0,0 @@ -// 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_TESTING_DART_FIXTURE_H_ -#define FLUTTER_TESTING_DART_FIXTURE_H_ - -#include - -#include "flutter/common/settings.h" -#include "flutter/runtime/dart_vm.h" -#include "flutter/testing/elf_loader.h" -#include "flutter/testing/test_dart_native_resolver.h" -#include "flutter/testing/testing.h" -#include "flutter/testing/thread_test.h" - -namespace flutter::testing { - -class DartFixture { - public: - // Uses the default filenames from the fixtures generator. - DartFixture(); - - // Allows to customize the kernel, ELF and split ELF filenames. - DartFixture(std::string kernel_filename, - std::string elf_filename, - std::string elf_split_filename); - - virtual Settings CreateSettingsForFixture(); - - void AddNativeCallback(std::string name, Dart_NativeFunction callback); - - protected: - void SetSnapshotsAndAssets(Settings& settings); - - std::shared_ptr native_resolver_; - ELFAOTSymbols split_aot_symbols_; - std::string kernel_filename_; - std::string elf_filename_; - fml::UniqueFD assets_dir_; - ELFAOTSymbols aot_symbols_; - - private: - FML_DISALLOW_COPY_AND_ASSIGN(DartFixture); -}; - -} // namespace flutter::testing - -#endif // FLUTTER_TESTING_DART_FIXTURE_H_ diff --git a/testing/fixture_test.cc b/testing/fixture_test.cc index ed451a50decff..490d70e3a4ad4 100644 --- a/testing/fixture_test.cc +++ b/testing/fixture_test.cc @@ -4,17 +4,71 @@ #include "flutter/testing/fixture_test.h" -#include "flutter/testing/dart_fixture.h" - namespace flutter { namespace testing { -FixtureTest::FixtureTest() : DartFixture() {} +FixtureTest::FixtureTest() + : FixtureTest("kernel_blob.bin", + kDefaultAOTAppELFFileName, + kDefaultAOTAppELFSplitFileName) {} FixtureTest::FixtureTest(std::string kernel_filename, std::string elf_filename, std::string elf_split_filename) - : DartFixture(kernel_filename, elf_filename, elf_split_filename) {} + : native_resolver_(std::make_shared()), + split_aot_symbols_( + LoadELFSplitSymbolFromFixturesIfNeccessary(elf_split_filename)), + kernel_filename_(kernel_filename), + assets_dir_(fml::OpenDirectory(GetFixturesPath(), + false, + fml::FilePermission::kRead)), + aot_symbols_(LoadELFSymbolFromFixturesIfNeccessary(elf_filename)) {} + +Settings FixtureTest::CreateSettingsForFixture() { + Settings settings; + settings.leak_vm = false; + settings.task_observer_add = [](intptr_t, fml::closure) {}; + settings.task_observer_remove = [](intptr_t) {}; + settings.isolate_create_callback = [this]() { + native_resolver_->SetNativeResolverForIsolate(); + }; + settings.enable_observatory = false; + SetSnapshotsAndAssets(settings); + return settings; +} + +void FixtureTest::SetSnapshotsAndAssets(Settings& settings) { + if (!assets_dir_.is_valid()) { + return; + } + + settings.assets_dir = assets_dir_.get(); + + // In JIT execution, all snapshots are present within the binary itself and + // don't need to be explicitly supplied by the embedder. In AOT, these + // snapshots will be present in the application AOT dylib. + if (DartVM::IsRunningPrecompiledCode()) { + FML_CHECK(PrepareSettingsForAOTWithSymbols(settings, aot_symbols_)); + } else { + settings.application_kernels = [this]() -> Mappings { + std::vector> kernel_mappings; + auto kernel_mapping = + fml::FileMapping::CreateReadOnly(assets_dir_, kernel_filename_); + if (!kernel_mapping || !kernel_mapping->IsValid()) { + FML_LOG(ERROR) << "Could not find kernel blob for test fixture not " + "running in precompiled mode."; + return kernel_mappings; + } + kernel_mappings.emplace_back(std::move(kernel_mapping)); + return kernel_mappings; + }; + } +} + +void FixtureTest::AddNativeCallback(std::string name, + Dart_NativeFunction callback) { + native_resolver_->AddNativeCallback(std::move(name), callback); +} } // namespace testing } // namespace flutter diff --git a/testing/fixture_test.h b/testing/fixture_test.h index f54bc80d28a2f..4b95a8b8289a3 100644 --- a/testing/fixture_test.h +++ b/testing/fixture_test.h @@ -5,12 +5,19 @@ #ifndef FLUTTER_TESTING_FIXTURE_TEST_H_ #define FLUTTER_TESTING_FIXTURE_TEST_H_ -#include "flutter/testing/dart_fixture.h" +#include + +#include "flutter/common/settings.h" +#include "flutter/runtime/dart_vm.h" +#include "flutter/testing/elf_loader.h" +#include "flutter/testing/test_dart_native_resolver.h" +#include "flutter/testing/testing.h" +#include "flutter/testing/thread_test.h" namespace flutter { namespace testing { -class FixtureTest : public DartFixture, public ThreadTest { +class FixtureTest : public ThreadTest { public: // Uses the default filenames from the fixtures generator. FixtureTest(); @@ -20,7 +27,23 @@ class FixtureTest : public DartFixture, public ThreadTest { std::string elf_filename, std::string elf_split_filename); + virtual Settings CreateSettingsForFixture(); + + void AddNativeCallback(std::string name, Dart_NativeFunction callback); + + protected: + void SetSnapshotsAndAssets(Settings& settings); + + std::shared_ptr native_resolver_; + + ELFAOTSymbols split_aot_symbols_; + private: + std::string kernel_filename_; + std::string elf_filename_; + fml::UniqueFD assets_dir_; + ELFAOTSymbols aot_symbols_; + FML_DISALLOW_COPY_AND_ASSIGN(FixtureTest); };