From d64f4e997c02e2d267d3fc6220da11cbb348d5dd Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Wed, 6 May 2020 16:03:55 -0700 Subject: [PATCH] Remove the global engine entry timestamp The engine was using a global to store a timestamp representing the launch of the engine. This timestamp is initialized with a JNI call on Android and during shell setup on other platforms. Later the timestamp is added to a FlutterEngineMainEnter timeline event used to measure engine startup time in benchmarks. This PR removes the global and the JNI call and moves the timestamp into the settings object. --- ci/licenses_golden/licenses_flutter | 2 -- common/settings.h | 6 +++++ runtime/BUILD.gn | 2 -- runtime/dart_vm.cc | 18 +++++++------- runtime/start_up.cc | 11 --------- runtime/start_up.h | 24 ------------------- shell/common/shell.cc | 14 ++++------- shell/platform/android/flutter_main.cc | 23 ++++++------------ shell/platform/android/flutter_main.h | 3 ++- .../flutter/embedding/engine/FlutterJNI.java | 6 ++--- .../engine/loader/FlutterLoader.java | 16 ++++--------- 11 files changed, 36 insertions(+), 89 deletions(-) delete mode 100644 runtime/start_up.cc delete mode 100644 runtime/start_up.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index f79af121f3781..7e124a8e2f959 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -559,8 +559,6 @@ FILE: ../../../flutter/runtime/service_protocol.cc FILE: ../../../flutter/runtime/service_protocol.h FILE: ../../../flutter/runtime/skia_concurrent_executor.cc FILE: ../../../flutter/runtime/skia_concurrent_executor.h -FILE: ../../../flutter/runtime/start_up.cc -FILE: ../../../flutter/runtime/start_up.h FILE: ../../../flutter/runtime/test_font_data.cc FILE: ../../../flutter/runtime/test_font_data.h FILE: ../../../flutter/runtime/window_data.cc diff --git a/common/settings.h b/common/settings.h index 45f851d3543a8..78d5693ae31d9 100644 --- a/common/settings.h +++ b/common/settings.h @@ -8,6 +8,7 @@ #include #include +#include #include #include #include @@ -207,6 +208,11 @@ struct Settings { /// https://github.com/dart-lang/sdk/blob/ca64509108b3e7219c50d6c52877c85ab6a35ff2/runtime/vm/flag_list.h#L150 int64_t old_gen_heap_size = -1; + /// A timestamp representing when the engine started. The value is based + /// on the clock used by the Dart timeline APIs. This timestamp is used + /// to log a timeline event that tracks the latency of engine startup. + std::chrono::microseconds engine_start_timestamp = {}; + std::string ToString() const; }; diff --git a/runtime/BUILD.gn b/runtime/BUILD.gn index 59ebfbedc9d92..8dd56e9452568 100644 --- a/runtime/BUILD.gn +++ b/runtime/BUILD.gn @@ -70,8 +70,6 @@ source_set("runtime") { "service_protocol.h", "skia_concurrent_executor.cc", "skia_concurrent_executor.h", - "start_up.cc", - "start_up.h", "window_data.cc", "window_data.h", ] diff --git a/runtime/dart_vm.cc b/runtime/dart_vm.cc index 0a63ed80dbbe7..85e16c6393f9d 100644 --- a/runtime/dart_vm.cc +++ b/runtime/dart_vm.cc @@ -25,7 +25,6 @@ #include "flutter/runtime/dart_isolate.h" #include "flutter/runtime/dart_service_isolate.h" #include "flutter/runtime/ptrace_ios.h" -#include "flutter/runtime/start_up.h" #include "third_party/dart/runtime/include/bin/dart_io_api.h" #include "third_party/skia/include/core/SkExecutor.h" #include "third_party/tonic/converter/dart_converter.h" @@ -426,14 +425,15 @@ DartVM::DartVM(std::shared_ptr vm_data, // the very first frame gives us a good idea about Flutter's startup time. // Use a duration event so about:tracing will consider this event when // deciding the earliest event to use as time 0. - if (engine_main_enter_ts != 0) { - Dart_TimelineEvent("FlutterEngineMainEnter", // label - engine_main_enter_ts, // timestamp0 - Dart_TimelineGetMicros(), // timestamp1_or_async_id - Dart_Timeline_Event_Duration, // event type - 0, // argument_count - nullptr, // argument_names - nullptr // argument_values + if (settings_.engine_start_timestamp.count()) { + Dart_TimelineEvent( + "FlutterEngineMainEnter", // label + settings_.engine_start_timestamp.count(), // timestamp0 + Dart_TimelineGetMicros(), // timestamp1_or_async_id + Dart_Timeline_Event_Duration, // event type + 0, // argument_count + nullptr, // argument_names + nullptr // argument_values ); } } diff --git a/runtime/start_up.cc b/runtime/start_up.cc deleted file mode 100644 index cba501ad23579..0000000000000 --- a/runtime/start_up.cc +++ /dev/null @@ -1,11 +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/runtime/start_up.h" - -namespace flutter { - -int64_t engine_main_enter_ts = 0; - -} // namespace flutter diff --git a/runtime/start_up.h b/runtime/start_up.h deleted file mode 100644 index 83079a3e8a472..0000000000000 --- a/runtime/start_up.h +++ /dev/null @@ -1,24 +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_RUNTIME_START_UP_H_ -#define FLUTTER_RUNTIME_START_UP_H_ - -#include - -namespace flutter { - -// The earliest available timestamp in the application's lifecycle. The -// difference between this timestamp and the time we render the very first -// frame gives us a good idea about Flutter's startup time. -// -// This timestamp only covers Flutter's own startup. In an upside-down model -// it is possible that the first Flutter view is not initialized until some -// time later. In this case the timestamp may not cover the time spent in the -// user code prior to initializing Flutter. -extern int64_t engine_main_enter_ts; - -} // namespace flutter - -#endif // FLUTTER_RUNTIME_START_UP_H_ diff --git a/shell/common/shell.cc b/shell/common/shell.cc index 1a27b009dc906..8d278535c15e9 100644 --- a/shell/common/shell.cc +++ b/shell/common/shell.cc @@ -20,7 +20,6 @@ #include "flutter/fml/trace_event.h" #include "flutter/fml/unique_fd.h" #include "flutter/runtime/dart_vm.h" -#include "flutter/runtime/start_up.h" #include "flutter/shell/common/engine.h" #include "flutter/shell/common/persistent_cache.h" #include "flutter/shell/common/skia_event_tracer_impl.h" @@ -175,12 +174,6 @@ std::unique_ptr Shell::CreateShellOnPlatformThread( return shell; } -static void RecordStartupTimestamp() { - if (engine_main_enter_ts == 0) { - engine_main_enter_ts = Dart_TimelineGetMicros(); - } -} - static void Tokenize(const std::string& input, std::vector* results, char delimiter) { @@ -197,7 +190,7 @@ static void Tokenize(const std::string& input, // TODO(chinmaygarde): The unfortunate side effect of this call is that settings // that cause shell initialization failures will still lead to some of their // settings being applied. -static void PerformInitializationTasks(const Settings& settings) { +static void PerformInitializationTasks(Settings& settings) { { fml::LogSettings log_settings; log_settings.min_log_level = @@ -207,7 +200,10 @@ static void PerformInitializationTasks(const Settings& settings) { static std::once_flag gShellSettingsInitialization = {}; std::call_once(gShellSettingsInitialization, [&settings] { - RecordStartupTimestamp(); + if (settings.engine_start_timestamp.count() == 0) { + settings.engine_start_timestamp = + std::chrono::microseconds(Dart_TimelineGetMicros()); + } tonic::SetLogHandler( [](const char* message) { FML_LOG(ERROR) << message; }); diff --git a/shell/platform/android/flutter_main.cc b/shell/platform/android/flutter_main.cc index 1ddcbf6ae9509..cb2d51649aaeb 100644 --- a/shell/platform/android/flutter_main.cc +++ b/shell/platform/android/flutter_main.cc @@ -18,7 +18,6 @@ #include "flutter/fml/size.h" #include "flutter/lib/ui/plugins/callback_cache.h" #include "flutter/runtime/dart_vm.h" -#include "flutter/runtime/start_up.h" #include "flutter/shell/common/shell.h" #include "flutter/shell/common/switches.h" #include "third_party/dart/runtime/include/dart_tools_api.h" @@ -62,7 +61,8 @@ void FlutterMain::Init(JNIEnv* env, jobjectArray jargs, jstring kernelPath, jstring appStoragePath, - jstring engineCachesPath) { + jstring engineCachesPath, + jlong initTimeMillis) { std::vector args; args.push_back("flutter"); for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) { @@ -72,6 +72,10 @@ void FlutterMain::Init(JNIEnv* env, auto settings = SettingsFromCommandLine(command_line); + int64_t init_time_micros = initTimeMillis * 1000; + settings.engine_start_timestamp = + std::chrono::microseconds(Dart_TimelineGetMicros() - init_time_micros); + // Restore the callback cache. // TODO(chinmaygarde): Route all cache file access through FML and remove this // setter. @@ -151,27 +155,14 @@ void FlutterMain::SetupObservatoryUriCallback(JNIEnv* env) { }); } -static void RecordStartTimestamp(JNIEnv* env, - jclass jcaller, - jlong initTimeMillis) { - int64_t initTimeMicros = - static_cast(initTimeMillis) * static_cast(1000); - flutter::engine_main_enter_ts = Dart_TimelineGetMicros() - initTimeMicros; -} - bool FlutterMain::Register(JNIEnv* env) { static const JNINativeMethod methods[] = { { .name = "nativeInit", .signature = "(Landroid/content/Context;[Ljava/lang/String;Ljava/" - "lang/String;Ljava/lang/String;Ljava/lang/String;)V", + "lang/String;Ljava/lang/String;Ljava/lang/String;J)V", .fnPtr = reinterpret_cast(&Init), }, - { - .name = "nativeRecordStartTimestamp", - .signature = "(J)V", - .fnPtr = reinterpret_cast(&RecordStartTimestamp), - }, }; jclass clazz = env->FindClass("io/flutter/embedding/engine/FlutterJNI"); diff --git a/shell/platform/android/flutter_main.h b/shell/platform/android/flutter_main.h index 11cb842de990a..171800b3ec500 100644 --- a/shell/platform/android/flutter_main.h +++ b/shell/platform/android/flutter_main.h @@ -35,7 +35,8 @@ class FlutterMain { jobjectArray jargs, jstring kernelPath, jstring appStoragePath, - jstring engineCachesPath); + jstring engineCachesPath, + jlong initTimeMillis); void SetupObservatoryUriCallback(JNIEnv* env); diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index e599281f84137..891498280999c 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -103,10 +103,8 @@ public static native void nativeInit( @NonNull String[] args, @Nullable String bundlePath, @NonNull String appStoragePath, - @NonNull String engineCachesPath); - - // TODO(mattcarroll): add javadocs - public static native void nativeRecordStartTimestamp(long initTimeMillis); + @NonNull String engineCachesPath, + long initTimeMillis); // TODO(mattcarroll): add javadocs @UiThread diff --git a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java index effe382634072..bbb1f635d96a4 100644 --- a/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java +++ b/shell/platform/android/io/flutter/embedding/engine/loader/FlutterLoader.java @@ -78,6 +78,7 @@ public static FlutterLoader getInstance() { private boolean initialized = false; @Nullable private ResourceExtractor resourceExtractor; @Nullable private Settings settings; + private long initStartTimestampMillis; /** * Starts initialization of the native system. @@ -113,8 +114,7 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se this.settings = settings; - long initStartTimestampMillis = SystemClock.uptimeMillis(); - initConfig(applicationContext); + initStartTimestampMillis = SystemClock.uptimeMillis(); initResources(applicationContext); System.loadLibrary("flutter"); @@ -122,14 +122,6 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se VsyncWaiter.getInstance( (WindowManager) applicationContext.getSystemService(Context.WINDOW_SERVICE)) .init(); - - // We record the initialization time using SystemClock because at the start of the - // initialization we have not yet loaded the native library to call into dart_tools_api.h. - // To get Timeline timestamp of the start of initialization we simply subtract the delta - // from the Timeline timestamp at the current moment (the assumption is that the overhead - // of the JNI call is negligible). - long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; - FlutterJNI.nativeRecordStartTimestamp(initTimeMillis); } /** @@ -202,12 +194,14 @@ public void ensureInitializationComplete( String appStoragePath = PathUtils.getFilesDir(applicationContext); String engineCachesPath = PathUtils.getCacheDirectory(applicationContext); + long initTimeMillis = SystemClock.uptimeMillis() - initStartTimestampMillis; FlutterJNI.nativeInit( applicationContext, shellArgs.toArray(new String[0]), kernelPath, appStoragePath, - engineCachesPath); + engineCachesPath, + initTimeMillis); initialized = true; } catch (Exception e) {