android histogram: ChromiumAndroidLinker.LoadLibraryStatus
Record sample-per-attempt enum histogram to allow following what kind of
retries are happening, all in one histogram. This allows knowing
separately:
* whether loading succeeded
* which process it happened in (browser/child)
* whether loading at fixed address was attempted
* whether it was a retry
Not recording the status for loading the crazylinker's library because we did
not change that code there recently.
One tiny awkwardness turned out to be necessary: buffering the samples
until the process type is known (moved to a separate class). I tried to
make the process type to be known earlier, at loading time, instead of
library initialization, but it turned out not to be possible because
webview zygote loads the library without knowing the process type, and
then later initializes the library with the newly arrived info
(presumably after forking).
Bug: 981599
Change-Id: If8fbb0ea6c51b3caaf2b7ad2b2303d3c5105a939
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1713640
Commit-Queue: Egor Pasko <pasko@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682230}
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 87a0cc41..c04aa71 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -3244,6 +3244,7 @@
"android/java/src/org/chromium/base/library_loader/LibraryPrefetcher.java",
"android/java/src/org/chromium/base/library_loader/Linker.java",
"android/java/src/org/chromium/base/library_loader/LoaderErrors.java",
+ "android/java/src/org/chromium/base/library_loader/LoadStatusRecorder.java",
"android/java/src/org/chromium/base/library_loader/ModernLinker.java",
"android/java/src/org/chromium/base/library_loader/NativeLibraryPreloader.java",
"android/java/src/org/chromium/base/library_loader/ProcessInitException.java",
diff --git a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
index a8ec604..40dd77fd 100644
--- a/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
+++ b/base/android/java/src/org/chromium/base/library_loader/LibraryLoader.java
@@ -38,6 +38,8 @@
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
+import javax.annotation.concurrent.GuardedBy;
+
/**
* This class provides functionality to load and register the native libraries.
* Callers are allowed to separate loading the libraries from initializing them.
@@ -74,7 +76,7 @@
// Also, starting with M, the issue doesn't exist if shared libraries are stored
// uncompressed in the APK (as Chromium does), because the system linker can access them
// directly, and the PackageManager will thus never extract them in the first place.
- static public final boolean PLATFORM_REQUIRES_NATIVE_FALLBACK_EXTRACTION =
+ public static final boolean PLATFORM_REQUIRES_NATIVE_FALLBACK_EXTRACTION =
Build.VERSION.SDK_INT <= VERSION_CODES.KITKAT;
// Location of extracted native libraries.
@@ -117,6 +119,10 @@
// will be reported via UMA. Set once when the libraries are done loading.
private long mLibraryLoadTimeMs;
+ // Stores information about attempts to load the library and eventually emits those bits as
+ // UMA histograms.
+ private static final LoadStatusRecorder sLoadStatusRecorder = new LoadStatusRecorder();
+
/**
* Call this method to determine if the chromium project must load the library
* directly from a zip file.
@@ -288,15 +294,25 @@
}
// Helper for loadAlreadyLocked(). Load a native shared library with the Chromium linker.
- // Sets UMA flags depending on the results of loading.
- private void loadLibraryWithCustomLinkerAlreadyLocked(Linker linker, String libFilePath) {
- assert Thread.holdsLock(mLock);
+ // Records UMA histograms depending on the results of loading.
+ @GuardedBy("mLock")
+ private void loadLibraryWithCustomLinkerAlreadyLocked(
+ Linker linker, String libFilePath, boolean isFirstAttempt) {
// Attempt shared RELROs, and if that fails then retry without.
+ boolean loadAtFixedAddress = true;
+ boolean success = true;
try {
linker.loadLibrary(libFilePath);
} catch (UnsatisfiedLinkError e) {
Log.w(TAG, "Failed to load native library with shared RELRO, retrying without");
+ sLoadStatusRecorder.recordLoadAttempt(
+ false /* success */, isFirstAttempt, true /* loadAtFixedAddress */);
+ loadAtFixedAddress = false;
+ success = false;
linker.loadLibraryNoFixedAddress(libFilePath);
+ success = true;
+ } finally {
+ sLoadStatusRecorder.recordLoadAttempt(success, isFirstAttempt, loadAtFixedAddress);
}
}
@@ -350,13 +366,13 @@
try {
// Load the library using this Linker. May throw UnsatisfiedLinkError.
- loadLibraryWithCustomLinkerAlreadyLocked(
- linker, System.mapLibraryName(library));
+ loadLibraryWithCustomLinkerAlreadyLocked(linker,
+ System.mapLibraryName(library), true /* isFirstAttempt */);
} catch (UnsatisfiedLinkError e) {
- if (!isInZipFile()
- && PLATFORM_REQUIRES_NATIVE_FALLBACK_EXTRACTION) {
- loadLibraryWithCustomLinkerAlreadyLocked(
- linker, getExtractedLibraryPath(appInfo, library));
+ if (!isInZipFile() && PLATFORM_REQUIRES_NATIVE_FALLBACK_EXTRACTION) {
+ loadLibraryWithCustomLinkerAlreadyLocked(linker,
+ getExtractedLibraryPath(appInfo, library),
+ false /* isFirstAttempt */);
} else {
Log.e(TAG, "Unable to load library: " + library);
throw(e);
@@ -486,6 +502,7 @@
return;
}
mLibraryProcessType = processType;
+ sLoadStatusRecorder.setProcessType(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.
diff --git a/base/android/java/src/org/chromium/base/library_loader/LoadStatusRecorder.java b/base/android/java/src/org/chromium/base/library_loader/LoadStatusRecorder.java
new file mode 100644
index 0000000..5bafddd
--- /dev/null
+++ b/base/android/java/src/org/chromium/base/library_loader/LoadStatusRecorder.java
@@ -0,0 +1,91 @@
+// 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.library_loader;
+
+import android.support.annotation.IntDef;
+
+import org.chromium.base.VisibleForTesting;
+import org.chromium.base.metrics.CachedMetrics.EnumeratedHistogramSample;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.util.ArrayList;
+
+/**
+ * Buffers and records UMA histogram(s) about library loading attempts.
+ *
+ * This class is not thread safe. This class relies on CachedMetrics, hence only supports process
+ * types that CacheMetrics support.
+ */
+public class LoadStatusRecorder {
+ // Used to record an UMA histogram. Since these value are persisted to logs, they should never
+ // be renumbered nor reused.
+ @IntDef({LoadLibraryStatus.IS_BROWSER, LoadLibraryStatus.AT_FIXED_ADDRESS,
+ LoadLibraryStatus.FIRST_ATTEMPT, LoadLibraryStatus.WAS_SUCCESSFUL,
+ LoadLibraryStatus.BOUNDARY})
+ @Retention(RetentionPolicy.SOURCE)
+ @VisibleForTesting
+ public @interface LoadLibraryStatus {
+ int WAS_SUCCESSFUL = 1;
+ int AT_FIXED_ADDRESS = 1 << 1;
+ int FIRST_ATTEMPT = 1 << 2;
+ int IS_BROWSER = 1 << 3;
+ int BOUNDARY = 1 << 4;
+ }
+
+ private EnumeratedHistogramSample mHistogramSample = new EnumeratedHistogramSample(
+ "ChromiumAndroidLinker.LoadLibraryStatus", LoadLibraryStatus.BOUNDARY);
+
+ private @LibraryProcessType int mProcessType = LibraryProcessType.PROCESS_UNINITIALIZED;
+
+ private ArrayList<Integer> mBufferedAttempts = new ArrayList<>(1);
+
+ /**
+ * Emits information about library loading attempt as UMA histogram or buffers it until process
+ * type is set via {@code setProcessType()}.
+ * @param success Whether the attempt to load the library succeeded.
+ * @param isFirstAttempt Whether it was the first attempt (not a retry).
+ * @param loadAtFixedAddress Whether loading at fixed address was attempted.
+ */
+ public void recordLoadAttempt(
+ boolean success, boolean isFirstAttempt, boolean loadAtFixedAddress) {
+ int sample = convertAttemptToInt(success, isFirstAttempt, loadAtFixedAddress);
+ if (processTypeWasSet()) {
+ recordWithProcessType(sample);
+ return;
+ }
+ mBufferedAttempts.add(sample);
+ }
+
+ /** Sets the process type and flushes the buffered recorded attempts to UMA. */
+ public void setProcessType(@LibraryProcessType int processType) {
+ assert processType != LibraryProcessType.PROCESS_UNINITIALIZED;
+ assert !processTypeWasSet() || mProcessType == processType;
+ mProcessType = processType;
+ for (int sample : mBufferedAttempts) recordWithProcessType(sample);
+ mBufferedAttempts.clear();
+ }
+
+ private boolean processTypeWasSet() {
+ return mProcessType != LibraryProcessType.PROCESS_UNINITIALIZED;
+ }
+
+ private static int convertAttemptToInt(
+ boolean success, boolean isFirstAttempt, boolean loadAtFixedAddress) {
+ int sample = 0;
+ if (success) sample |= LoadLibraryStatus.WAS_SUCCESSFUL;
+ if (loadAtFixedAddress) sample |= LoadLibraryStatus.AT_FIXED_ADDRESS;
+ if (isFirstAttempt) sample |= LoadLibraryStatus.FIRST_ATTEMPT;
+ return sample;
+ }
+
+ private void recordWithProcessType(int sample) {
+ if (mProcessType == LibraryProcessType.PROCESS_BROWSER
+ || mProcessType == LibraryProcessType.PROCESS_WEBVIEW) {
+ sample |= LoadLibraryStatus.IS_BROWSER;
+ }
+ mHistogramSample.record(sample);
+ }
+}
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/StartupLoadingMetricsTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/StartupLoadingMetricsTest.java
index 5f3dbdf..3693c7b7 100644
--- a/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/StartupLoadingMetricsTest.java
+++ b/chrome/android/javatests/src/org/chromium/chrome/browser/metrics/StartupLoadingMetricsTest.java
@@ -6,6 +6,7 @@
import android.content.Context;
import android.content.Intent;
+import android.os.Build;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.LargeTest;
@@ -16,6 +17,9 @@
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.chromium.base.Log;
+import org.chromium.base.library_loader.LibraryLoader;
+import org.chromium.base.library_loader.LoadStatusRecorder.LoadLibraryStatus;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.RetryOnFailure;
@@ -42,6 +46,7 @@
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add(ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE)
public class StartupLoadingMetricsTest {
+ private static final String TAG = "StartupLoadingTest";
private static final String TEST_PAGE = "/chrome/test/data/android/google.html";
private static final String TEST_PAGE_2 = "/chrome/test/data/android/test.html";
private static final String ERROR_PAGE = "/close-socket";
@@ -50,6 +55,8 @@
"Startup.Android.Cold.TimeToFirstNavigationCommit";
private static final String FIRST_CONTENTFUL_PAINT_HISTOGRAM =
"Startup.Android.Cold.TimeToFirstContentfulPaint";
+ private static final String LOAD_LIBRARY_STATUS_HISTOGRAM =
+ "ChromiumAndroidLinker.LoadLibraryStatus";
private static final String TABBED_SUFFIX = ChromeTabbedActivity.STARTUP_UMA_HISTOGRAM_SUFFIX;
private static final String WEBAPK_SUFFIX = WebApkActivity.STARTUP_UMA_HISTOGRAM_SUFFIX;
@@ -137,7 +144,8 @@
}
/**
- * Tests that the startup loading histograms are recorded only once on startup.
+ * Tests that the startup loading histograms are recorded only once on startup. In addition
+ * tests that library loading histograms were recorded at startup.
*/
@Test
@LargeTest
@@ -148,6 +156,42 @@
assertHistogramsRecorded(1, TABBED_SUFFIX);
loadUrlAndWaitForPageLoadMetricsRecorded(mTabbedActivityTestRule, mTestPage2);
assertHistogramsRecorded(1, TABBED_SUFFIX);
+
+ // LibraryLoader checks.
+ if (!LibraryLoader.useChromiumLinker()) {
+ Log.w(TAG, "Skipping test because not using ChromiumLinker.");
+ return;
+ }
+ // TODO(pasko): Make the checks stricter once renderer-side histograms become available for
+ // testing. Once fixed, the http://crbug.com/987288 should help with it.
+ Assert.assertTrue("At least the browser process should record a sample.",
+ 1 <= RecordHistogram.getHistogramTotalCountForTesting(
+ LOAD_LIBRARY_STATUS_HISTOGRAM));
+
+ // The specific values are explained in LoadLibraryStatus in
+ // tools/metrics/histograms/enums.xml.
+ final int browserQuickSuccess = 15;
+ Assert.assertEquals(browserQuickSuccess,
+ LoadLibraryStatus.WAS_SUCCESSFUL | LoadLibraryStatus.IS_BROWSER
+ | LoadLibraryStatus.AT_FIXED_ADDRESS | LoadLibraryStatus.FIRST_ATTEMPT);
+ if (Build.VERSION.SDK_INT != Build.VERSION_CODES.KITKAT) {
+ Assert.assertEquals("Browser-side sample should be present.", 1,
+ getLibraryStatusHistogramValueCount(browserQuickSuccess));
+ } else {
+ // On KitKat it is likely to fall back to loading without fixed address.
+ if (0 == getLibraryStatusHistogramValueCount(browserQuickSuccess)) {
+ final int browserNoFixedSuccess = 13;
+ Assert.assertEquals(browserNoFixedSuccess,
+ browserQuickSuccess & ~LoadLibraryStatus.AT_FIXED_ADDRESS);
+ Assert.assertEquals("Browser-side fallback to no-fixed address should happen", 1,
+ getLibraryStatusHistogramValueCount(browserNoFixedSuccess));
+ }
+ }
+ }
+
+ private static int getLibraryStatusHistogramValueCount(int value) {
+ return RecordHistogram.getHistogramValueCountForTesting(
+ LOAD_LIBRARY_STATUS_HISTOGRAM, value);
}
/**
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index d2d667b..87a7087 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -33729,6 +33729,51 @@
<int value="4" label="LoadLibraryExW Unavailable, LoadLibraryW Fail"/>
</enum>
+<enum name="LoadLibraryStatus">
+ <int value="0"
+ label="Failed to load, no fixed address, on retry, child process"/>
+ <int value="1"
+ label="Successfully loaded, no fixed address, on retry, child process"/>
+ <int value="2"
+ label="Failed to load at fixed address, on retry, child process"/>
+ <int value="3"
+ label="Successfully loaded at fixed address, on retry, child process"/>
+ <int value="4"
+ label="Failed to load, no fixed address, first attempt, child process"/>
+ <int value="5"
+ label="Successfully loaded, no fixed address, first attempt, child
+ process"/>
+ <int value="6"
+ label="Failed to load at fixed address, first attempt, child process"/>
+ <int value="7"
+ label="Successfully loaded at fixed address, first attempt, child
+ process"/>
+ <int value="8"
+ label="Failed to load, no fixed address, on retry, browser/webview
+ process"/>
+ <int value="9"
+ label="Successfully loaded, no fixed address, on retry, browser/webview
+ process"/>
+ <int value="10"
+ label="Failed to load at fixed address, on retry, browser/webview
+ process"/>
+ <int value="11"
+ label="Successfully loaded at fixed address, on retry, browser/webview
+ process"/>
+ <int value="12"
+ label="Failed to load, no fixed address, first attempt, browser/webview
+ process"/>
+ <int value="13"
+ label="Successfully loaded, no fixed address, first attempt,
+ browser/webview process"/>
+ <int value="14"
+ label="Failed to load at fixed address, first attempt, browser/webview
+ process"/>
+ <int value="15"
+ label="Successfully loaded at fixed address, first attempt,
+ browser/webview process"/>
+</enum>
+
<enum name="LoadRulesetResult">
<int value="0" label="Load succeeded"/>
<int value="1" label="Load failed - Invalid path"/>
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 599cc85..d083de25 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -19189,6 +19189,17 @@
<summary>Load at fixed address failed.</summary>
</histogram>
+<histogram name="ChromiumAndroidLinker.LoadLibraryStatus"
+ enum="LoadLibraryStatus" expires_after="M80">
+ <owner>lizeb@chromium.org</owner>
+ <owner>pasko@chromium.org</owner>
+ <summary>
+ Status of each attempt to load the native library with a custom linker.
+ Recorded after each attempt to load. Not recorded when all attempts fail in
+ a process.
+ </summary>
+</histogram>
+
<histogram name="ChromiumAndroidLinker.RelinkerFallbackCount"
enum="BooleanIsUseRelinker">
<obsolete>