From df8dc2b4a5520784362804ce8e04afa53a0ca613 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Fri, 16 Oct 2020 20:35:31 -0600 Subject: [PATCH] Apply fixes for EfficientCollections. Drop-in replacements suggested for inefficient collections. Also annotate a handful of places where we're unable to update. Bug: 155703208 Test: none Exempt-From-Owner-Approval: trivial refactoring Change-Id: I48b600508df8160ac9b40fea7afca974b2c972f6 --- core/java/android/content/Intent.java | 1 + core/java/android/os/FileObserver.java | 13 ++++--- core/java/android/os/Parcel.java | 10 ++++-- core/java/android/os/StrictMode.java | 36 ++++++++++--------- .../android/util/proto/ProtoInputStream.java | 7 ++-- core/java/android/view/View.java | 1 + core/java/android/view/ViewGroup.java | 5 +-- .../internal/os/AppIdToPackageMap.java | 12 +++---- .../util/NotificationMessagingUtil.java | 4 +-- .../internal/os/BinderCallsStatsTest.java | 4 +-- 10 files changed, 53 insertions(+), 40 deletions(-) diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index c62194b380dd7..2fa056420c34e 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -7447,6 +7447,7 @@ public class Intent implements Parcelable, Cloneable { /** @hide */ @UnsupportedAppUsage + @SuppressWarnings("AndroidFrameworkEfficientCollections") public static Intent parseCommandArgs(ShellCommand cmd, CommandOptionHandler optionHandler) throws URISyntaxException { Intent intent = new Intent(); diff --git a/core/java/android/os/FileObserver.java b/core/java/android/os/FileObserver.java index ca303d9732357..25bffbc9e8d52 100644 --- a/core/java/android/os/FileObserver.java +++ b/core/java/android/os/FileObserver.java @@ -21,6 +21,7 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.compat.annotation.UnsupportedAppUsage; import android.util.Log; +import android.util.SparseArray; import java.io.File; import java.lang.annotation.Retention; @@ -101,7 +102,9 @@ public abstract class FileObserver { private static final String LOG_TAG = "FileObserver"; private static class ObserverThread extends Thread { + /** Temporarily retained; appears to be missing UnsupportedAppUsage annotation */ private HashMap m_observers = new HashMap(); + private SparseArray mRealObservers = new SparseArray<>(); private int m_fd; public ObserverThread() { @@ -127,10 +130,10 @@ public abstract class FileObserver { final WeakReference fileObserverWeakReference = new WeakReference<>(observer); - synchronized (m_observers) { + synchronized (mRealObservers) { for (int wfd : wfds) { if (wfd >= 0) { - m_observers.put(wfd, fileObserverWeakReference); + mRealObservers.put(wfd, fileObserverWeakReference); } } } @@ -147,12 +150,12 @@ public abstract class FileObserver { // look up our observer, fixing up the map if necessary... FileObserver observer = null; - synchronized (m_observers) { - WeakReference weak = m_observers.get(wfd); + synchronized (mRealObservers) { + WeakReference weak = mRealObservers.get(wfd); if (weak != null) { // can happen with lots of events from a dead wfd observer = (FileObserver) weak.get(); if (observer == null) { - m_observers.remove(wfd); + mRealObservers.remove(wfd); } } } diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java index 13d5f6a9c9d7e..765ef48308aeb 100644 --- a/core/java/android/os/Parcel.java +++ b/core/java/android/os/Parcel.java @@ -2010,13 +2010,13 @@ public final class Parcel { * A map used by {@link #readSquashed} to cache parcelables. It's a map from * an absolute position in a Parcel to the parcelable stored at the position. */ - private ArrayMap mReadSquashableParcelables; + private SparseArray mReadSquashableParcelables; private void ensureReadSquashableParcelables() { if (mReadSquashableParcelables != null) { return; } - mReadSquashableParcelables = new ArrayMap<>(); + mReadSquashableParcelables = new SparseArray<>(); } /** @@ -2112,9 +2112,13 @@ public final class Parcel { final Parcelable p = mReadSquashableParcelables.get(firstAbsolutePos); if (p == null) { + final StringBuilder sb = new StringBuilder(); + for (int i = 0; i < mReadSquashableParcelables.size(); i++) { + sb.append(mReadSquashableParcelables.keyAt(i)).append(' '); + } Slog.wtfStack(TAG, "Map doesn't contain offset " + firstAbsolutePos - + " : contains=" + new ArrayList<>(mReadSquashableParcelables.keySet())); + + " : contains=" + sb.toString()); } return (T) p; } diff --git a/core/java/android/os/StrictMode.java b/core/java/android/os/StrictMode.java index 6c5b04a649e2f..5f4a508a7af18 100644 --- a/core/java/android/os/StrictMode.java +++ b/core/java/android/os/StrictMode.java @@ -60,6 +60,7 @@ import android.util.Log; import android.util.Printer; import android.util.Singleton; import android.util.Slog; +import android.util.SparseLongArray; import android.view.IWindowManager; import com.android.internal.annotations.GuardedBy; @@ -1525,7 +1526,9 @@ public final class StrictMode { // Map from violation stacktrace hashcode -> uptimeMillis of // last violation. No locking needed, as this is only // accessed by the same thread. + /** Temporarily retained; appears to be missing UnsupportedAppUsage annotation */ private ArrayMap mLastViolationTime; + private SparseLongArray mRealLastViolationTime; public AndroidBlockGuardPolicy(@ThreadPolicyMask int threadPolicyMask) { mThreadPolicyMask = threadPolicyMask; @@ -1759,17 +1762,17 @@ public final class StrictMode { long lastViolationTime = 0; long now = SystemClock.uptimeMillis(); if (sLogger == LOGCAT_LOGGER) { // Don't throttle it if there is a non-default logger - if (mLastViolationTime != null) { - Long vtime = mLastViolationTime.get(crashFingerprint); + if (mRealLastViolationTime != null) { + Long vtime = mRealLastViolationTime.get(crashFingerprint); if (vtime != null) { lastViolationTime = vtime; } - clampViolationTimeMap(mLastViolationTime, Math.max(MIN_LOG_INTERVAL_MS, + clampViolationTimeMap(mRealLastViolationTime, Math.max(MIN_LOG_INTERVAL_MS, Math.max(MIN_DIALOG_INTERVAL_MS, MIN_DROPBOX_INTERVAL_MS))); } else { - mLastViolationTime = new ArrayMap<>(1); + mRealLastViolationTime = new SparseLongArray(1); } - mLastViolationTime.put(crashFingerprint, now); + mRealLastViolationTime.put(crashFingerprint, now); } long timeSinceLastViolationMillis = lastViolationTime == 0 ? Long.MAX_VALUE : (now - lastViolationTime); @@ -2231,18 +2234,19 @@ public final class StrictMode { // Map from VM violation fingerprint to uptime millis. @UnsupportedAppUsage private static final HashMap sLastVmViolationTime = new HashMap<>(); + private static final SparseLongArray sRealLastVmViolationTime = new SparseLongArray(); /** * Clamp the given map by removing elements with timestamp older than the given retainSince. */ - private static void clampViolationTimeMap(final @NonNull Map violationTime, + private static void clampViolationTimeMap(final @NonNull SparseLongArray violationTime, final long retainSince) { - final Iterator> iterator = violationTime.entrySet().iterator(); - while (iterator.hasNext()) { - Map.Entry e = iterator.next(); - if (e.getValue() < retainSince) { + for (int i = 0; i < violationTime.size(); ) { + if (violationTime.valueAt(i) < retainSince) { // Remove stale entries - iterator.remove(); + violationTime.removeAt(i); + } else { + i++; } } // Ideally we'd cap the total size of the map, though it'll involve quickselect of topK, @@ -2273,15 +2277,15 @@ public final class StrictMode { long lastViolationTime; long timeSinceLastViolationMillis = Long.MAX_VALUE; if (sLogger == LOGCAT_LOGGER) { // Don't throttle it if there is a non-default logger - synchronized (sLastVmViolationTime) { - if (sLastVmViolationTime.containsKey(fingerprint)) { - lastViolationTime = sLastVmViolationTime.get(fingerprint); + synchronized (sRealLastVmViolationTime) { + if (sRealLastVmViolationTime.indexOfKey(fingerprint) >= 0) { + lastViolationTime = sRealLastVmViolationTime.get(fingerprint); timeSinceLastViolationMillis = now - lastViolationTime; } if (timeSinceLastViolationMillis > MIN_VM_INTERVAL_MS) { - sLastVmViolationTime.put(fingerprint, now); + sRealLastVmViolationTime.put(fingerprint, now); } - clampViolationTimeMap(sLastVmViolationTime, + clampViolationTimeMap(sRealLastVmViolationTime, now - Math.max(MIN_VM_INTERVAL_MS, MIN_LOG_INTERVAL_MS)); } } diff --git a/core/java/android/util/proto/ProtoInputStream.java b/core/java/android/util/proto/ProtoInputStream.java index aa70d07ff787b..9789b10a0a617 100644 --- a/core/java/android/util/proto/ProtoInputStream.java +++ b/core/java/android/util/proto/ProtoInputStream.java @@ -16,10 +16,11 @@ package android.util.proto; +import android.util.LongArray; + import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; -import java.util.ArrayList; /** * Class to read to a protobuf stream. @@ -98,7 +99,7 @@ public final class ProtoInputStream extends ProtoStream { /** * Keeps track of the currently read nested Objects, for end object checking and debug */ - private ArrayList mExpectedObjectTokenStack = null; + private LongArray mExpectedObjectTokenStack = null; /** * Current nesting depth of start calls. @@ -498,7 +499,7 @@ public final class ProtoInputStream extends ProtoStream { int messageSize = (int) readVarint(); if (mExpectedObjectTokenStack == null) { - mExpectedObjectTokenStack = new ArrayList<>(); + mExpectedObjectTokenStack = new LongArray(); } if (++mDepth == mExpectedObjectTokenStack.size()) { // Create a token to keep track of nested Object and extend the object stack diff --git a/core/java/android/view/View.java b/core/java/android/view/View.java index cf5ca56eb188e..bcdacb9ce76ba 100644 --- a/core/java/android/view/View.java +++ b/core/java/android/view/View.java @@ -6150,6 +6150,7 @@ public class View implements Drawable.Callback, KeyEvent.Callback, * was set. */ @NonNull + @SuppressWarnings("AndroidFrameworkEfficientCollections") public Map getAttributeSourceResourceMap() { HashMap map = new HashMap<>(); if (!sDebugViewAttributes || mAttributeSourceResId == null) { diff --git a/core/java/android/view/ViewGroup.java b/core/java/android/view/ViewGroup.java index fd7c2d896b090..eb6c49549100d 100644 --- a/core/java/android/view/ViewGroup.java +++ b/core/java/android/view/ViewGroup.java @@ -50,6 +50,7 @@ import android.os.Bundle; import android.os.Parcelable; import android.os.SystemClock; import android.util.AttributeSet; +import android.util.IntArray; import android.util.Log; import android.util.Pools; import android.util.Pools.SynchronizedPool; @@ -611,7 +612,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager private int mNestedScrollAxes; // Used to manage the list of transient views, added by addTransientView() - private List mTransientIndices = null; + private IntArray mTransientIndices = null; private List mTransientViews = null; /** @@ -4853,7 +4854,7 @@ public abstract class ViewGroup extends View implements ViewParent, ViewManager } if (mTransientIndices == null) { - mTransientIndices = new ArrayList(); + mTransientIndices = new IntArray(); mTransientViews = new ArrayList(); } final int oldSize = mTransientIndices.size(); diff --git a/core/java/com/android/internal/os/AppIdToPackageMap.java b/core/java/com/android/internal/os/AppIdToPackageMap.java index 65aa989bbb38b..98cced89a1745 100644 --- a/core/java/com/android/internal/os/AppIdToPackageMap.java +++ b/core/java/com/android/internal/os/AppIdToPackageMap.java @@ -16,25 +16,23 @@ package com.android.internal.os; - import android.app.AppGlobals; import android.content.pm.PackageInfo; import android.content.pm.PackageManager; import android.os.RemoteException; import android.os.UserHandle; +import android.util.SparseArray; import com.android.internal.annotations.VisibleForTesting; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** Maps AppIds to their package names. */ public final class AppIdToPackageMap { - private final Map mAppIdToPackageMap; + private final SparseArray mAppIdToPackageMap; @VisibleForTesting - public AppIdToPackageMap(Map appIdToPackageMap) { + public AppIdToPackageMap(SparseArray appIdToPackageMap) { mAppIdToPackageMap = appIdToPackageMap; } @@ -50,10 +48,10 @@ public final class AppIdToPackageMap { } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - final Map map = new HashMap<>(); + final SparseArray map = new SparseArray<>(); for (PackageInfo pkg : packages) { final int uid = pkg.applicationInfo.uid; - if (pkg.sharedUserId != null && map.containsKey(uid)) { + if (pkg.sharedUserId != null && map.indexOfKey(uid) >= 0) { // Use sharedUserId string as package name if there are collisions map.put(uid, "shared:" + pkg.sharedUserId); } else { diff --git a/core/java/com/android/internal/util/NotificationMessagingUtil.java b/core/java/com/android/internal/util/NotificationMessagingUtil.java index 28994fd521262..c59647d264f68 100644 --- a/core/java/com/android/internal/util/NotificationMessagingUtil.java +++ b/core/java/com/android/internal/util/NotificationMessagingUtil.java @@ -26,7 +26,7 @@ import android.os.Looper; import android.os.UserHandle; import android.provider.Settings; import android.service.notification.StatusBarNotification; -import android.util.ArrayMap; +import android.util.SparseArray; import java.util.Collection; import java.util.Objects; @@ -39,7 +39,7 @@ public class NotificationMessagingUtil { private static final String DEFAULT_SMS_APP_SETTING = Settings.Secure.SMS_DEFAULT_APPLICATION; private final Context mContext; - private ArrayMap mDefaultSmsApp = new ArrayMap<>(); + private SparseArray mDefaultSmsApp = new SparseArray<>(); public NotificationMessagingUtil(Context context) { mContext = context; diff --git a/core/tests/coretests/src/com/android/internal/os/BinderCallsStatsTest.java b/core/tests/coretests/src/com/android/internal/os/BinderCallsStatsTest.java index 0eb34a993deca..3117935fb3edc 100644 --- a/core/tests/coretests/src/com/android/internal/os/BinderCallsStatsTest.java +++ b/core/tests/coretests/src/com/android/internal/os/BinderCallsStatsTest.java @@ -497,9 +497,9 @@ public class BinderCallsStatsTest { bcs.callEnded(callSession, REQUEST_SIZE, REPLY_SIZE, WORKSOURCE_UID); PrintWriter pw = new PrintWriter(new StringWriter()); - bcs.dump(pw, new AppIdToPackageMap(new HashMap<>()), Process.INVALID_UID, true); + bcs.dump(pw, new AppIdToPackageMap(new SparseArray<>()), Process.INVALID_UID, true); - bcs.dump(pw, new AppIdToPackageMap(new HashMap<>()), WORKSOURCE_UID, true); + bcs.dump(pw, new AppIdToPackageMap(new SparseArray<>()), WORKSOURCE_UID, true); } @Test