From 47032a209517ccdc50bbcc27b2f4e5bbaaf28552 Mon Sep 17 00:00:00 2001 From: Tim Murray Date: Mon, 11 Apr 2022 21:44:01 -0700 Subject: [PATCH] Icon: make Icon's Bitmaps immutable Because Bitmap mutability is propagated across Parcel and Binder, the following problem can happen when sending an Icon containing a Bitmap as part of a Notification: 1. App creates Icon with a mutable Bitmap (1 alloc) 2. App sends Icon to system_server, often creating ashmem Bitmap (2 allocs) 3. system_server converts ashmem Bitmap back to heap Bitmap (3 allocs) 4. system_server sends heap Bitmap to each NotificationListener individually, converting the heap Bitmap to ashmem each time (3+N allocs) 5. NotificationListener converts ashmem Bitmap to heap Bitmap (3+2N allocs) This is inefficient. This is especially bad because the API to update a Notification involves sending a full Notification object, including Icons, repeatedly, and some apps do that several times per second. Instead, ensure that all Bitmaps transmitted as part of Icons are immutable. Instead, we get: 1. App creates Icon, which may copy to immutable Bitamp (1 or 2 allocs) 2. App sends Icon to system_server over ashmem (still 1 or 2 allocs) 3. system_server uses received ashmem Bitmap directly (still 1 or 2 allocs) 4. system_server sends same ashmem Bitmap to NotificationListeners (1 or 2) 5. Each NotificationListener uses the same ashmem Bitmap (1 or 2) This solves the per-NotificationListener amplification, but it does not solve sending what is likely the same Bitmap N times per second when apps update Notifications. That will require either app changes or Notification API changes to avoid bytewise comparisons for all Bitmaps. Test: boot, notifications work, memory in maps/gearhead remains good Bug: 227920378 Change-Id: If92835f647a76da599f358c7c02888e3e2f59235 --- .../java/android/graphics/drawable/Icon.java | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/graphics/java/android/graphics/drawable/Icon.java b/graphics/java/android/graphics/drawable/Icon.java index b04b82629b921..a76d74edc0f4d 100644 --- a/graphics/java/android/graphics/drawable/Icon.java +++ b/graphics/java/android/graphics/drawable/Icon.java @@ -128,6 +128,7 @@ public final class Icon implements Parcelable { // TYPE_RESOURCE: Resources // TYPE_DATA: DataBytes private Object mObj1; + private boolean mCachedAshmem = false; // TYPE_RESOURCE: package name // TYPE_URI: uri string @@ -156,6 +157,8 @@ public final class Icon implements Parcelable { /** * @return The {@link android.graphics.Bitmap} held by this {@link #TYPE_BITMAP} or * {@link #TYPE_ADAPTIVE_BITMAP} Icon. + * + * Note that this will always return an immutable Bitmap. * @hide */ @UnsupportedAppUsage @@ -166,8 +169,20 @@ public final class Icon implements Parcelable { return (Bitmap) mObj1; } + /** + * Sets the Icon's contents to a particular Bitmap. Note that this may make a copy of the Bitmap + * if the supplied Bitmap is mutable. In that case, the value returned by getBitmap() may not + * equal the Bitmap passed to setBitmap(). + * + * @hide + */ private void setBitmap(Bitmap b) { - mObj1 = b; + if (b.isMutable()) { + mObj1 = b.copy(b.getConfig(), false); + } else { + mObj1 = b; + } + mCachedAshmem = false; } /** @@ -488,6 +503,7 @@ public final class Icon implements Parcelable { getBitmap().getAllocationByteCount() >= MIN_ASHMEM_ICON_SIZE) { setBitmap(getBitmap().asShared()); } + mCachedAshmem = true; } /** @@ -913,7 +929,10 @@ public final class Icon implements Parcelable { switch (mType) { case TYPE_BITMAP: case TYPE_ADAPTIVE_BITMAP: - final Bitmap bits = getBitmap(); + if (!mCachedAshmem) { + mObj1 = ((Bitmap) mObj1).asShared(); + mCachedAshmem = true; + } getBitmap().writeToParcel(dest, flags); break; case TYPE_RESOURCE: