Reland "android: Experiment setup for the reached code profiler."

This is a reland of 48f72ca

Failed tests that caused the revert in https://crbug.com/930262 were fixed
in https://crrev.com/c/1464581

Original change's description:
> android: Experiment setup for the reached code profiler.
>
> This CL prepares the reached code profiler for the finch experiment. It adds:
> - base::Feature "ReachedCodeProfiler", which state is cached in
> - Android shared preference "reached_code_profiler_enabled", that determines
> whether to set
> - command line switch "enable-reached-code-profiler", that eventually enables
> the profiler
>
> We cannot simply use a base::Feature for the reached code profiler because
> we have to know the feature state very early in the startup, before the
> FeatureList is initialized.
>
> To work around this limitation we cache the feature state in Android shared
> preferences that are available in Java before native is initialized.
>
> Since only the browser process has a right to read the shared preferences we
> pass the value of the cached feature state as a command line flag to all
> processes.
>
> Bug: 916263
> Change-Id: I730b98c5484ca595bdfda46592572f5853784aa8
> Reviewed-on: https://chromium-review.googlesource.com/c/1393328
> Reviewed-by: Andrew Grieve <[email protected]>
> Reviewed-by: Bo <[email protected]>
> Reviewed-by: Gabriel Charette <[email protected]>
> Reviewed-by: Egor Pasko <[email protected]>
> Commit-Queue: Alex Ilin <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#630298}

TBR: [email protected], [email protected], [email protected], [email protected]
Bug: 916263
Change-Id: Ib3da0a08976ebebff3ece9d42acbd02e069f3287
Reviewed-on: https://chromium-review.googlesource.com/c/1466521
Reviewed-by: Alex Ilin <[email protected]>
Commit-Queue: Alex Ilin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#631177}

base/BUILD.gn

@@ -3147,6 +3147,7 @@ if (is_android) {
     ]
 
     java_files = [
+      "test/android/javatests/src/org/chromium/base/test/ReachedCodeProfiler.java",
       "test/android/javatests/src/org/chromium/base/test/BaseJUnit4ClassRunner.java",
       "test/android/javatests/src/org/chromium/base/test/BaseChromiumAndroidJUnitRunner.java",
       "test/android/javatests/src/org/chromium/base/test/BaseChromiumRunnerCommon.java",

base/android/java/src/org/chromium/base/BaseSwitches.java

@@ -27,6 +27,9 @@ public abstract class BaseSwitches {
     // Default country code to be used for search engine localization.
     public static final String DEFAULT_COUNTRY_CODE_AT_INSTALL = "default-country-code";
 
+    // Enables the reached code profiler.
+    public static final String ENABLE_REACHED_CODE_PROFILER = "enable-reached-code-profiler";
+
     // Prevent instantiation.
     private BaseSwitches() {}
 }

base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java

@@ -17,13 +17,15 @@
 import android.support.v4.content.ContextCompat;
 import android.system.Os;
 
+import org.chromium.base.BaseSwitches;
 import org.chromium.base.BuildConfig;
 import org.chromium.base.BuildInfo;
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
 import org.chromium.base.FileUtils;
 import org.chromium.base.Log;
 import org.chromium.base.StreamUtil;
+import org.chromium.base.StrictModeContext;
 import org.chromium.base.SysUtils;
 import org.chromium.base.TraceEvent;
 import org.chromium.base.VisibleForTesting;
@@ -85,6 +87,9 @@ public class LibraryLoader {
     // SharedPreferences key for "don't prefetch libraries" flag
     private static final String DONT_PREFETCH_LIBRARIES_KEY = "dont_prefetch_libraries";
 
+    // Shared preferences key for the reached code profiler.
+    private static final String REACHED_CODE_PROFILER_ENABLED_KEY = "reached_code_profiler_enabled";
+
     private static final EnumeratedHistogramSample sRelinkerCountHistogram =
             new EnumeratedHistogramSample("ChromiumAndroidLinker.RelinkerFallbackCount", 2);
 
@@ -306,6 +311,33 @@ private static boolean isNotPrefetchingLibraries() {
         }
     }
 
+    /**
+     * Enables the reached code profiler. The value comes from "ReachedCodeProfiler"
+     * finch experiment, and is pushed on every run. I.e. the effect of the finch experiment
+     * lags by one run, which is the best we can do considering that the profiler has to be enabled
+     * before finch is initialized. Note that since LibraryLoader is in //base, it can't depend
+     * on ChromeFeatureList, and has to rely on external code pushing the value.
+     *
+     * @param enabled whether to enable the reached code profiler.
+     */
+    public static void setReachedCodeProfilerEnabledOnNextRuns(boolean enabled) {
+        ContextUtils.getAppSharedPreferences()
+                .edit()
+                .putBoolean(REACHED_CODE_PROFILER_ENABLED_KEY, enabled)
+                .apply();
+    }
+
+    /**
+     * @return whether to enable reached code profiler (see
+     *         setReachedCodeProfilerEnabledOnNextRuns()).
+     */
+    private static boolean isReachedCodeProfilerEnabled() {
+        try (StrictModeContext unused = StrictModeContext.allowDiskReads()) {
+            return ContextUtils.getAppSharedPreferences().getBoolean(
+                    REACHED_CODE_PROFILER_ENABLED_KEY, false);
+        }
+    }
+
     /** Prefetches the native libraries in a background thread.
      *
      * Launches an AsyncTask that, through a short-lived forked process, reads a
@@ -585,6 +617,13 @@ private void initializeAlreadyLocked(@LibraryProcessType int processType)
         }
         mLibraryProcessType = processType;
 
+        // Add a switch for the reached code profiler as late as possible since it requires a read
+        // from the shared preferences. At this point the shared preferences are usually warmed up.
+        if (mLibraryProcessType == LibraryProcessType.PROCESS_BROWSER
+                && isReachedCodeProfilerEnabled()) {
+            CommandLine.getInstance().appendSwitch(BaseSwitches.ENABLE_REACHED_CODE_PROFILER);
+        }
+
         ensureCommandLineSwitchedAlreadyLocked();
 
         if (!nativeLibraryLoaded(mLibraryProcessType)) {

base/android/reached_code_profiler.cc

@@ -13,8 +13,10 @@
 
 #include "base/android/library_loader/anchor_functions.h"
 #include "base/android/orderfile/orderfile_buildflags.h"
+#include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/important_file_writer.h"
@@ -40,6 +42,19 @@ namespace android {
 
 namespace {
 
+#if !defined(NDEBUG) || defined(COMPONENT_BUILD)
+// Always disabled for debug builds to avoid hitting a limit of signal
+// interrupts that can get delivered into a single HANDLE_EINTR. Also
+// debugging experience would be bad if there are a lot of signals flying
+// around.
+// Always disabled for component builds because in this case the code is not
+// organized in one contiguous region which is required for the reached code
+// profiler.
+constexpr const bool kConfigurationSupported = false;
+#else
+constexpr const bool kConfigurationSupported = true;
+#endif
+
 constexpr const char kDumpToFileFlag[] = "reached-code-profiler-dump-to-file";
 
 // Enough for 1 << 29 bytes of code, 512MB.
@@ -290,20 +305,11 @@ class ReachedCodeProfiler {
 };
 
 bool ShouldEnableReachedCodeProfiler() {
-#if !defined(NDEBUG) || defined(COMPONENT_BUILD)
-  // Always disabled for debug builds to avoid hitting a limit of signal
-  // interrupts that can get delivered into a single HANDLE_EINTR. Also
-  // debugging experience would be bad if there are a lot of signals flying
-  // around.
-  // Always disabled for component builds because in this case the code is not
-  // organized in one contiguous region which is required for the reached code
-  // profiler.
-  return false;
-#else
-  // TODO(crbug.com/916263): this should be set up according to the finch
-  // experiment.
-  return false;
-#endif
+  if (!kConfigurationSupported)
+    return false;
+
+  const base::CommandLine* cmdline = base::CommandLine::ForCurrentProcess();
+  return cmdline->HasSwitch(switches::kEnableReachedCodeProfiler);
 }
 
 }  // namespace
@@ -323,5 +329,9 @@ bool IsReachedCodeProfilerEnabled() {
   return ReachedCodeProfiler::GetInstance()->IsEnabled();
 }
 
+bool IsReachedCodeProfilerSupported() {
+  return kConfigurationSupported;
+}
+
 }  // namespace android
 }  // namespace base

base/android/reached_code_profiler.h

@@ -24,6 +24,10 @@ BASE_EXPORT void InitReachedCodeProfilerAtStartup(
 // Returns whether the reached code profiler is enabled.
 BASE_EXPORT bool IsReachedCodeProfilerEnabled();
 
+// Returns whether the reached code profiler can be possibly enabled for the
+// current build configuration.
+BASE_EXPORT bool IsReachedCodeProfilerSupported();
+
 }  // namespace android
 }  // namespace base
 

base/android/reached_code_profiler_stub.cc

@@ -14,5 +14,9 @@ bool IsReachedCodeProfilerEnabled() {
   return false;
 }
 
+bool IsReachedCodeProfilerSupported() {
+  return false;
+}
+
 }  // namespace android
 }  // namespace base

base/base_switches.cc

@@ -123,6 +123,10 @@ const char kEnableCrashReporterForTesting[] =
 #endif
 
 #if defined(OS_ANDROID)
+// Enables the reached code profiler that samples all threads in all processes
+// to determine which functions are almost never executed.
+const char kEnableReachedCodeProfiler[] = "enable-reached-code-profiler";
+
 // Specifies optimization of memory layout of the native library using the
 // orderfile symbols given in base/android/library_loader/anchor_functions.h,
 // via madvise and changing the library prefetch behavior.

base/base_switches.h

@@ -45,6 +45,7 @@ extern const char kEnableCrashReporterForTesting[];
 #endif
 
 #if defined(OS_ANDROID)
+extern const char kEnableReachedCodeProfiler[];
 extern const char kOrderfileMemoryOptimization[];
 #endif
 

base/test/BUILD.gn

@@ -93,6 +93,7 @@ static_library("test_support") {
     "perf_time_logger.h",
     "power_monitor_test_base.cc",
     "power_monitor_test_base.h",
+    "reached_code_profiler_android.cc",
     "scoped_command_line.cc",
     "scoped_command_line.h",
     "scoped_environment_variable_override.cc",
@@ -435,6 +436,7 @@ if (is_android) {
     sources = [
       "android/java/src/org/chromium/base/MainReturnCodeResult.java",
       "android/java/src/org/chromium/base/MultiprocessTestClientLauncher.java",
+      "android/javatests/src/org/chromium/base/test/ReachedCodeProfiler.java",
       "android/javatests/src/org/chromium/base/test/task/TaskSchedulerTestHelpers.java",
       "android/javatests/src/org/chromium/base/test/util/UrlUtils.java",
     ]

base/test/android/javatests/src/org/chromium/base/test/ReachedCodeProfiler.java

@@ -0,0 +1,33 @@
+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.base.test;
+
+import org.chromium.base.annotations.JNINamespace;
+
+/**
+ * Class containing only static methods for querying the status of the reached code profiler.
+ */
+@JNINamespace("base::android")
+public class ReachedCodeProfiler {
+    private ReachedCodeProfiler() {}
+
+    /**
+     * @return Whether the reached code profiler is enabled.
+     */
+    public static boolean isEnabled() {
+        return nativeIsReachedCodeProfilerEnabled();
+    }
+
+    /**
+     * @return Whether the currently used version of native library supports the reached code
+     *         profiler.
+     */
+    public static boolean isSupported() {
+        return nativeIsReachedCodeProfilerSupported();
+    }
+
+    private static native boolean nativeIsReachedCodeProfilerEnabled();
+    private static native boolean nativeIsReachedCodeProfilerSupported();
+}

base/test/reached_code_profiler_android.cc

@@ -0,0 +1,25 @@
+// Copyright 2019 The Chromium 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 "base/android/jni_android.h"
+#include "base/android/reached_code_profiler.h"
+#include "jni/ReachedCodeProfiler_jni.h"
+
+// This file provides functions to query the state of the reached code profiler
+// from Java. It's used only for tests.
+namespace base {
+namespace android {
+
+static jboolean JNI_ReachedCodeProfiler_IsReachedCodeProfilerEnabled(
+    JNIEnv* env) {
+  return IsReachedCodeProfilerEnabled();
+}
+
+static jboolean JNI_ReachedCodeProfiler_IsReachedCodeProfilerSupported(
+    JNIEnv* env) {
+  return IsReachedCodeProfilerSupported();
+}
+
+}  // namespace android
+}  // namespace base

chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java

@@ -281,6 +281,7 @@ public static boolean getFieldTrialParamByFeatureAsBoolean(
             "PredictivePrefetchingAllowedOnAllConnectionTypes";
     public static final String PROGRESS_BAR_THROTTLE = "ProgressBarThrottle";
     public static final String PWA_PERSISTENT_NOTIFICATION = "PwaPersistentNotification";
+    public static final String REACHED_CODE_PROFILER = "ReachedCodeProfiler";
     public static final String READER_MODE_IN_CCT = "ReaderModeInCCT";
     public static final String REMOVE_NAVIGATION_HISTORY = "RemoveNavigationHistory";
     public static final String SEARCH_READY_OMNIBOX = "SearchReadyOmnibox";

chrome/android/java/src/org/chromium/chrome/browser/init/ChromeBrowserInitializer.java

@@ -242,6 +242,9 @@ private void preInflationStartup() {
         ChromeStrictMode.configureStrictMode();
         ChromeWebApkHost.init();
 
+        // Time this call takes in background from test devices:
+        // - Pixel 2: ~10 ms
+        // - Nokia 1 (Android Go): 20-200 ms
         warmUpSharedPrefs();
 
         DeviceUtils.addDeviceSpecificUserAgentSwitch();

chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java

@@ -195,11 +195,13 @@ public static void cacheNativeFlags() {
         cacheNightModeAvailable();
         cacheDownloadAutoResumptionEnabledInNative();
 
-        // Propagate DONT_PREFETCH_LIBRARIES feature value to LibraryLoader. This can't
-        // be done in LibraryLoader itself because it lives in //base and can't depend
-        // on ChromeFeatureList.
+        // Propagate DONT_PREFETCH_LIBRARIES and REACHED_CODE_PROFILER feature values to
+        // LibraryLoader. This can't be done in LibraryLoader itself because it lives in //base and
+        // can't depend on ChromeFeatureList.
         LibraryLoader.setDontPrefetchLibrariesOnNextRuns(
                 ChromeFeatureList.isEnabled(ChromeFeatureList.DONT_PREFETCH_LIBRARIES));
+        LibraryLoader.setReachedCodeProfilerEnabledOnNextRuns(
+                ChromeFeatureList.isEnabled(ChromeFeatureList.REACHED_CODE_PROFILER));
     }
 
     /**

chrome/android/java_sources.gni

@@ -1903,6 +1903,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/PowerBroadcastReceiverTest.java",
   "javatests/src/org/chromium/chrome/browser/PrerenderTest.java",
   "javatests/src/org/chromium/chrome/browser/ProcessIsolationTest.java",
+  "javatests/src/org/chromium/chrome/browser/ReachedCodeProfilerTest.java",
   "javatests/src/org/chromium/chrome/browser/RestoreHistogramTest.java",
   "javatests/src/org/chromium/chrome/browser/SafeBrowsingTest.java",
   "javatests/src/org/chromium/chrome/browser/SelectFileDialogTest.java",