Revert "Remove the global engine entry timestamp (#18182)"

mehmetf · web-flow · commit 9ec68bd485e4 · 2020-05-09T10:28:48.000-07:00
This reverts commit 88b9d42.
diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter
@@ -559,6 +559,8 @@ 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
@@ -8,7 +8,6 @@
 #include <fcntl.h>
 #include <stdint.h>
 
-#include <chrono>
 #include <memory>
 #include <string>
 #include <vector>
@@ -208,11 +207,6 @@ 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
@@ -70,6 +70,8 @@ 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
@@ -25,6 +25,7 @@
 #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"
@@ -425,15 +426,14 @@ DartVM::DartVM(std::shared_ptr<const DartVMData> 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 (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
+    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
       );
     }
   }
diff --git a/runtime/start_up.cc b/runtime/start_up.cc
@@ -0,0 +1,11 @@
+// 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
@@ -0,0 +1,24 @@
+// 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 <stdint.h>
+
+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
@@ -20,6 +20,7 @@
 #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"
@@ -174,6 +175,12 @@ std::unique_ptr<Shell> 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<std::string>* results,
                      char delimiter) {
@@ -190,7 +197,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(Settings& settings) {
+static void PerformInitializationTasks(const Settings& settings) {
   {
     fml::LogSettings log_settings;
     log_settings.min_log_level =
@@ -200,10 +207,7 @@ static void PerformInitializationTasks(Settings& settings) {
 
   static std::once_flag gShellSettingsInitialization = {};
   std::call_once(gShellSettingsInitialization, [&settings] {
-    if (settings.engine_start_timestamp.count() == 0) {
-      settings.engine_start_timestamp =
-          std::chrono::microseconds(Dart_TimelineGetMicros());
-    }
+    RecordStartupTimestamp();
 
     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
@@ -18,6 +18,7 @@
 #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"
@@ -61,8 +62,7 @@ void FlutterMain::Init(JNIEnv* env,
                        jobjectArray jargs,
                        jstring kernelPath,
                        jstring appStoragePath,
-                       jstring engineCachesPath,
-                       jlong initTimeMillis) {
+                       jstring engineCachesPath) {
   std::vector<std::string> args;
   args.push_back("flutter");
   for (auto& arg : fml::jni::StringArrayToVector(env, jargs)) {
@@ -72,10 +72,6 @@ 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.
@@ -155,14 +151,27 @@ void FlutterMain::SetupObservatoryUriCallback(JNIEnv* env) {
       });
 }
 
+static void RecordStartTimestamp(JNIEnv* env,
+                                 jclass jcaller,
+                                 jlong initTimeMillis) {
+  int64_t initTimeMicros =
+      static_cast<int64_t>(initTimeMillis) * static_cast<int64_t>(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;J)V",
+                       "lang/String;Ljava/lang/String;Ljava/lang/String;)V",
           .fnPtr = reinterpret_cast<void*>(&Init),
       },
+      {
+          .name = "nativeRecordStartTimestamp",
+          .signature = "(J)V",
+          .fnPtr = reinterpret_cast<void*>(&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
@@ -35,8 +35,7 @@ class FlutterMain {
                    jobjectArray jargs,
                    jstring kernelPath,
                    jstring appStoragePath,
-                   jstring engineCachesPath,
-                   jlong initTimeMillis);
+                   jstring engineCachesPath);
 
   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
@@ -103,8 +103,10 @@ public static native void nativeInit(
       @NonNull String[] args,
       @Nullable String bundlePath,
       @NonNull String appStoragePath,
-      @NonNull String engineCachesPath,
-      long initTimeMillis);
+      @NonNull String engineCachesPath);
+
+  // TODO(mattcarroll): add javadocs
+  public static native void nativeRecordStartTimestamp(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
@@ -78,7 +78,6 @@ 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.
@@ -114,14 +113,23 @@ public void startInitialization(@NonNull Context applicationContext, @NonNull Se
 
     this.settings = settings;
 
-    initStartTimestampMillis = SystemClock.uptimeMillis();
+    long initStartTimestampMillis = SystemClock.uptimeMillis();
+    initConfig(applicationContext);
     initResources(applicationContext);
 
     System.loadLibrary("flutter");
 
     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);
   }
 
   /**
@@ -194,14 +202,12 @@ 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,
-          initTimeMillis);
+          engineCachesPath);
 
       initialized = true;
     } catch (Exception e) {

Original file line number	Diff line number	Diff line change
`@@ -70,6 +70,8 @@ source_set("runtime") {`
`70`	`70`	`"service_protocol.h",`
`71`	`71`	`"skia_concurrent_executor.cc",`
`72`	`72`	`"skia_concurrent_executor.h",`
	`73`	`+ "start_up.cc",`
	`74`	`+ "start_up.h",`
`73`	`75`	`"window_data.cc",`
`74`	`76`	`"window_data.h",`
`75`	`77`	`]`