From bbdb7312750007a612d4412740cbafd0ce0c02ef Mon Sep 17 00:00:00 2001 From: Leon Scroggins III Date: Thu, 31 Jan 2019 14:35:54 -0500 Subject: [PATCH] Stop double-counting native memory in graphics classes Bug: 121304803 Test: Infeasible Previously, the NativeAllocationRegistry needed to report how much native memory was being held onto by the Java class in order to get a more accurate count of how much memory was in use. But recent changes allow the system to have an even more accurate view of the native memory with mallinfo(). Further, the AOSP change Idccb8b50d26c8c3e93cc34040d784f74dfcdbf51 introduces new APIs that allow distinguishing between large native malloc allocations, which should cause more frequent mallinfo() checks, and whose sizes need be available to ahat etc, and smaller native malloc allocations. Bitmap and AnimatedImageDrawable use the version for large native malloc allocations. The rest pass an implied size of 0. Note that many of the old Registries used somewhat arbitrary sizes, reinforcing the fact that the new way of keeping track of this is better. Add Bitmap::pixelStorageType to differentiate between types of memory that should be accounted for by the Registry. Update Bitmap::getAllocationByteCount to report the actual size of ashmem allocation. Fix a typo in LineBreaker.java, discovered while looking to find existing callers of Bitmap's constructor. Change-Id: I57c407258450aeaf08b47df32432466639d9faed --- core/jni/android/graphics/Bitmap.cpp | 5 +-- graphics/java/android/graphics/Bitmap.java | 33 ++++++++++++++----- graphics/java/android/graphics/Canvas.java | 9 ++--- .../java/android/graphics/ColorFilter.java | 5 +-- .../java/android/graphics/FontFamily.java | 10 +++--- graphics/java/android/graphics/Matrix.java | 11 +++---- graphics/java/android/graphics/Paint.java | 8 ++--- graphics/java/android/graphics/Path.java | 5 +-- .../java/android/graphics/RenderNode.java | 5 +-- graphics/java/android/graphics/Shader.java | 5 +-- graphics/java/android/graphics/Typeface.java | 5 +-- .../drawable/AnimatedImageDrawable.java | 2 +- .../drawable/AnimatedVectorDrawable.java | 1 - .../java/android/graphics/fonts/Font.java | 18 +++++----- .../android/graphics/fonts/FontFamily.java | 4 +-- .../android/graphics/text/LineBreaker.java | 12 ++++--- .../android/graphics/text/MeasuredText.java | 5 +-- libs/hwui/hwui/Bitmap.cpp | 2 ++ libs/hwui/hwui/Bitmap.h | 2 ++ 19 files changed, 86 insertions(+), 61 deletions(-) diff --git a/core/jni/android/graphics/Bitmap.cpp b/core/jni/android/graphics/Bitmap.cpp index 8f007594dd674..e0b7629013fb4 100755 --- a/core/jni/android/graphics/Bitmap.cpp +++ b/core/jni/android/graphics/Bitmap.cpp @@ -206,13 +206,14 @@ jobject createBitmap(JNIEnv* env, Bitmap* bitmap, // The caller needs to have already set the alpha type properly, so the // native SkBitmap stays in sync with the Java Bitmap. assert_premultiplied(bitmap->info(), isPremultiplied); + bool fromMalloc = bitmap->pixelStorageType() == PixelStorageType::Heap; BitmapWrapper* bitmapWrapper = new BitmapWrapper(bitmap); if (!isMutable) { bitmapWrapper->bitmap().setImmutable(); } jobject obj = env->NewObject(gBitmap_class, gBitmap_constructorMethodID, reinterpret_cast(bitmapWrapper), bitmap->width(), bitmap->height(), density, - isPremultiplied, ninePatchChunk, ninePatchInsets); + isPremultiplied, ninePatchChunk, ninePatchInsets, fromMalloc); if (env->ExceptionCheck() != 0) { ALOGE("*** Uncaught exception returned from Java call!\n"); @@ -1224,7 +1225,7 @@ int register_android_graphics_Bitmap(JNIEnv* env) { gBitmap_class = MakeGlobalRefOrDie(env, FindClassOrDie(env, "android/graphics/Bitmap")); gBitmap_nativePtr = GetFieldIDOrDie(env, gBitmap_class, "mNativePtr", "J"); - gBitmap_constructorMethodID = GetMethodIDOrDie(env, gBitmap_class, "", "(JIIIZ[BLandroid/graphics/NinePatch$InsetStruct;)V"); + gBitmap_constructorMethodID = GetMethodIDOrDie(env, gBitmap_class, "", "(JIIIZ[BLandroid/graphics/NinePatch$InsetStruct;Z)V"); gBitmap_reinitMethodID = GetMethodIDOrDie(env, gBitmap_class, "reinit", "(IIZ)V"); gBitmap_getAllocationByteCountMethodID = GetMethodIDOrDie(env, gBitmap_class, "getAllocationByteCount", "()I"); return android::RegisterMethodsOrDie(env, "android/graphics/Bitmap", gBitmapMethods, diff --git a/graphics/java/android/graphics/Bitmap.java b/graphics/java/android/graphics/Bitmap.java index 2e56e09522e90..4bd344f1964fe 100644 --- a/graphics/java/android/graphics/Bitmap.java +++ b/graphics/java/android/graphics/Bitmap.java @@ -122,13 +122,22 @@ public final class Bitmap implements Parcelable { } /** - * Private constructor that must received an already allocated native bitmap + * Private constructor that must receive an already allocated native bitmap * int (pointer). */ - // called from JNI - @UnsupportedAppUsage - Bitmap(long nativeBitmap, int width, int height, int density, boolean requestPremultiplied, - byte[] ninePatchChunk, NinePatch.InsetStruct ninePatchInsets) { + // JNI now calls the version below this one. This is preserved due to UnsupportedAppUsage. + @UnsupportedAppUsage(maxTargetSdk = 28) + Bitmap(long nativeBitmap, int width, int height, int density, + boolean requestPremultiplied, byte[] ninePatchChunk, + NinePatch.InsetStruct ninePatchInsets) { + this(nativeBitmap, width, height, density, requestPremultiplied, ninePatchChunk, + ninePatchInsets, true); + } + + // called from JNI and Bitmap_Delegate. + Bitmap(long nativeBitmap, int width, int height, int density, + boolean requestPremultiplied, byte[] ninePatchChunk, + NinePatch.InsetStruct ninePatchInsets, boolean fromMalloc) { if (nativeBitmap == 0) { throw new RuntimeException("internal error: native bitmap is 0"); } @@ -144,13 +153,21 @@ public final class Bitmap implements Parcelable { } mNativePtr = nativeBitmap; - long nativeSize = NATIVE_ALLOCATION_SIZE + getAllocationByteCount(); - NativeAllocationRegistry registry = new NativeAllocationRegistry( - Bitmap.class.getClassLoader(), nativeGetNativeFinalizer(), nativeSize); + + final int allocationByteCount = getAllocationByteCount(); + NativeAllocationRegistry registry; + if (fromMalloc) { + registry = NativeAllocationRegistry.createMalloced( + Bitmap.class.getClassLoader(), nativeGetNativeFinalizer(), allocationByteCount); + } else { + registry = NativeAllocationRegistry.createNonmalloced( + Bitmap.class.getClassLoader(), nativeGetNativeFinalizer(), allocationByteCount); + } registry.registerNativeAllocation(this, nativeBitmap); if (ResourcesImpl.TRACE_FOR_DETAILED_PRELOAD) { sPreloadTracingNumInstantiatedBitmaps++; + long nativeSize = NATIVE_ALLOCATION_SIZE + allocationByteCount; sPreloadTracingTotalBitmapsSize += nativeSize; } } diff --git a/graphics/java/android/graphics/Canvas.java b/graphics/java/android/graphics/Canvas.java index 8f46e1acc2bad..7b3f3da111d5c 100644 --- a/graphics/java/android/graphics/Canvas.java +++ b/graphics/java/android/graphics/Canvas.java @@ -76,14 +76,11 @@ public class Canvas extends BaseCanvas { // (see SkCanvas.cpp, SkDraw.cpp) private static final int MAXMIMUM_BITMAP_SIZE = 32766; - // The approximate size of the native allocation associated with - // a Canvas object. - private static final long NATIVE_ALLOCATION_SIZE = 525; - // Use a Holder to allow static initialization of Canvas in the boot image. private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Canvas.class.getClassLoader(), nGetNativeFinalizer(), NATIVE_ALLOCATION_SIZE); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Canvas.class.getClassLoader(), nGetNativeFinalizer()); } // This field is used to finalize the native Canvas properly diff --git a/graphics/java/android/graphics/ColorFilter.java b/graphics/java/android/graphics/ColorFilter.java index b24b9885d1b0e..4c2ef84404e24 100644 --- a/graphics/java/android/graphics/ColorFilter.java +++ b/graphics/java/android/graphics/ColorFilter.java @@ -26,8 +26,9 @@ import libcore.util.NativeAllocationRegistry; public class ColorFilter { private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - ColorFilter.class.getClassLoader(), nativeGetFinalizer(), 50); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + ColorFilter.class.getClassLoader(), nativeGetFinalizer()); } /** diff --git a/graphics/java/android/graphics/FontFamily.java b/graphics/java/android/graphics/FontFamily.java index 5d8ba938f6d5f..5af0da85bb395 100644 --- a/graphics/java/android/graphics/FontFamily.java +++ b/graphics/java/android/graphics/FontFamily.java @@ -44,13 +44,15 @@ public class FontFamily { private static String TAG = "FontFamily"; - private static final NativeAllocationRegistry sBuilderRegistry = new NativeAllocationRegistry( - FontFamily.class.getClassLoader(), nGetBuilderReleaseFunc(), 64); + private static final NativeAllocationRegistry sBuilderRegistry = + NativeAllocationRegistry.createMalloced( + FontFamily.class.getClassLoader(), nGetBuilderReleaseFunc()); private @Nullable Runnable mNativeBuilderCleaner; - private static final NativeAllocationRegistry sFamilyRegistry = new NativeAllocationRegistry( - FontFamily.class.getClassLoader(), nGetFamilyReleaseFunc(), 64); + private static final NativeAllocationRegistry sFamilyRegistry = + NativeAllocationRegistry.createMalloced( + FontFamily.class.getClassLoader(), nGetFamilyReleaseFunc()); /** * @hide diff --git a/graphics/java/android/graphics/Matrix.java b/graphics/java/android/graphics/Matrix.java index f8cb366c7b925..22b6401fdc2e0 100644 --- a/graphics/java/android/graphics/Matrix.java +++ b/graphics/java/android/graphics/Matrix.java @@ -16,12 +16,13 @@ package android.graphics; +import android.annotation.UnsupportedAppUsage; + import dalvik.annotation.optimization.CriticalNative; import dalvik.annotation.optimization.FastNative; import libcore.util.NativeAllocationRegistry; -import android.annotation.UnsupportedAppUsage; import java.io.PrintWriter; /** @@ -222,12 +223,10 @@ public class Matrix { } }; - // sizeof(SkMatrix) is 9 * sizeof(float) + uint32_t - private static final long NATIVE_ALLOCATION_SIZE = 40; - private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Matrix.class.getClassLoader(), nGetNativeFinalizer(), NATIVE_ALLOCATION_SIZE); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Matrix.class.getClassLoader(), nGetNativeFinalizer()); } /** diff --git a/graphics/java/android/graphics/Paint.java b/graphics/java/android/graphics/Paint.java index 452f7c93f8aa4..79ee4a9c27461 100644 --- a/graphics/java/android/graphics/Paint.java +++ b/graphics/java/android/graphics/Paint.java @@ -58,13 +58,11 @@ public class Paint { private long mNativeShader; private long mNativeColorFilter; - // The approximate size of a native paint object. - private static final long NATIVE_PAINT_SIZE = 98; - // Use a Holder to allow static initialization of Paint in the boot image. private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Paint.class.getClassLoader(), nGetNativeFinalizer(), NATIVE_PAINT_SIZE); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Paint.class.getClassLoader(), nGetNativeFinalizer()); } @ColorLong private long mColor; diff --git a/graphics/java/android/graphics/Path.java b/graphics/java/android/graphics/Path.java index 405ab0bbcb34b..7282d52d6e23c 100644 --- a/graphics/java/android/graphics/Path.java +++ b/graphics/java/android/graphics/Path.java @@ -36,8 +36,9 @@ import libcore.util.NativeAllocationRegistry; */ public class Path { - private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Path.class.getClassLoader(), nGetFinalizer(), 48 /* dummy size */); + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Path.class.getClassLoader(), nGetFinalizer()); /** * @hide diff --git a/graphics/java/android/graphics/RenderNode.java b/graphics/java/android/graphics/RenderNode.java index 5e48ea1c98d84..9e588c09404ce 100644 --- a/graphics/java/android/graphics/RenderNode.java +++ b/graphics/java/android/graphics/RenderNode.java @@ -198,8 +198,9 @@ public final class RenderNode { // Use a Holder to allow static initialization in the boot image. private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - RenderNode.class.getClassLoader(), nGetNativeFinalizer(), 1024); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + RenderNode.class.getClassLoader(), nGetNativeFinalizer()); } /** diff --git a/graphics/java/android/graphics/Shader.java b/graphics/java/android/graphics/Shader.java index d555128d0bbac..3050d1dae5e47 100644 --- a/graphics/java/android/graphics/Shader.java +++ b/graphics/java/android/graphics/Shader.java @@ -33,8 +33,9 @@ import libcore.util.NativeAllocationRegistry; public class Shader { private static class NoImagePreloadHolder { - public static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Shader.class.getClassLoader(), nativeGetFinalizer(), 50); + public static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Shader.class.getClassLoader(), nativeGetFinalizer()); } /** diff --git a/graphics/java/android/graphics/Typeface.java b/graphics/java/android/graphics/Typeface.java index 86f658b4a48ed..64f75913f8dec 100644 --- a/graphics/java/android/graphics/Typeface.java +++ b/graphics/java/android/graphics/Typeface.java @@ -74,8 +74,9 @@ public class Typeface { private static String TAG = "Typeface"; - private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Typeface.class.getClassLoader(), nativeGetReleaseFunc(), 64); + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Typeface.class.getClassLoader(), nativeGetReleaseFunc()); /** The default NORMAL typeface object */ public static final Typeface DEFAULT; diff --git a/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java b/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java index bb6bf243bc768..82f5870864283 100644 --- a/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java +++ b/graphics/java/android/graphics/drawable/AnimatedImageDrawable.java @@ -313,7 +313,7 @@ public class AnimatedImageDrawable extends Drawable implements Animatable2 { extended, cropRect), inputStream, afd); final long nativeSize = nNativeByteSize(mState.mNativePtr); - NativeAllocationRegistry registry = new NativeAllocationRegistry( + NativeAllocationRegistry registry = NativeAllocationRegistry.createMalloced( AnimatedImageDrawable.class.getClassLoader(), nGetNativeFinalizer(), nativeSize); registry.registerNativeAllocation(mState, mState.mNativePtr); } diff --git a/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java b/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java index d7aee7767524b..d9dab98c2be10 100644 --- a/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java +++ b/graphics/java/android/graphics/drawable/AnimatedVectorDrawable.java @@ -1253,7 +1253,6 @@ public class AnimatedVectorDrawable extends Drawable implements Animatable2 { private boolean mInitialized = false; private boolean mIsReversible = false; private boolean mIsInfinite = false; - // TODO: Consider using NativeAllocationRegistery to track native allocation private final VirtualRefBasePtr mSetRefBasePtr; private WeakReference mLastSeenTarget = null; private int mLastListenerId = 0; diff --git a/graphics/java/android/graphics/fonts/Font.java b/graphics/java/android/graphics/fonts/Font.java index f92802f5b574d..eb546ffd2cdea 100644 --- a/graphics/java/android/graphics/fonts/Font.java +++ b/graphics/java/android/graphics/fonts/Font.java @@ -54,13 +54,13 @@ public final class Font { * A builder class for creating new Font. */ public static final class Builder { - private static final NativeAllocationRegistry sAssetByteBufferRegistroy = - new NativeAllocationRegistry(ByteBuffer.class.getClassLoader(), - nGetReleaseNativeAssetFunc(), 64); + private static final NativeAllocationRegistry sAssetByteBufferRegistry = + NativeAllocationRegistry.createMalloced(ByteBuffer.class.getClassLoader(), + nGetReleaseNativeAssetFunc()); - private static final NativeAllocationRegistry sFontRegistory = - new NativeAllocationRegistry(Font.class.getClassLoader(), - nGetReleaseNativeFont(), 64); + private static final NativeAllocationRegistry sFontRegistry = + NativeAllocationRegistry.createMalloced(Font.class.getClassLoader(), + nGetReleaseNativeFont()); private @Nullable ByteBuffer mBuffer; private @Nullable File mFile; @@ -171,7 +171,7 @@ public final class Font { return; } final ByteBuffer b = nGetAssetBuffer(nativeAsset); - sAssetByteBufferRegistroy.registerNativeAllocation(b, nativeAsset); + sAssetByteBufferRegistry.registerNativeAllocation(b, nativeAsset); if (b == null) { mException = new FileNotFoundException(path + " not found"); return; @@ -206,7 +206,7 @@ public final class Font { return; } final ByteBuffer b = nGetAssetBuffer(nativeAsset); - sAssetByteBufferRegistroy.registerNativeAllocation(b, nativeAsset); + sAssetByteBufferRegistry.registerNativeAllocation(b, nativeAsset); if (b == null) { mException = new FileNotFoundException(str + " not found"); return; @@ -395,7 +395,7 @@ public final class Font { mTtcIndex); final Font font = new Font(ptr, readonlyBuffer, mFile, new FontStyle(mWeight, slant), mTtcIndex, mAxes, mLocaleList); - sFontRegistory.registerNativeAllocation(font, ptr); + sFontRegistry.registerNativeAllocation(font, ptr); return font; } diff --git a/graphics/java/android/graphics/fonts/FontFamily.java b/graphics/java/android/graphics/fonts/FontFamily.java index 4772c1cff7ebd..75ea120629299 100644 --- a/graphics/java/android/graphics/fonts/FontFamily.java +++ b/graphics/java/android/graphics/fonts/FontFamily.java @@ -63,8 +63,8 @@ public final class FontFamily { */ public static final class Builder { private static final NativeAllocationRegistry sFamilyRegistory = - new NativeAllocationRegistry(FontFamily.class.getClassLoader(), - nGetReleaseNativeFamily(), 64); + NativeAllocationRegistry.createMalloced(FontFamily.class.getClassLoader(), + nGetReleaseNativeFamily()); private final ArrayList mFonts = new ArrayList<>(); private final HashSet mStyleHashSet = new HashSet<>(); diff --git a/graphics/java/android/graphics/text/LineBreaker.java b/graphics/java/android/graphics/text/LineBreaker.java index 9cabf1ce185df..e49f216f32e2a 100644 --- a/graphics/java/android/graphics/text/LineBreaker.java +++ b/graphics/java/android/graphics/text/LineBreaker.java @@ -69,7 +69,7 @@ import java.lang.annotation.RetentionPolicy; * } * * // Draw text to the canvas - * Bitmap bmp = new Bitmap.createBitmap(240, totalHeight, Bitmap.Config.ARGB_8888); + * Bitmap bmp = Bitmap.createBitmap(240, totalHeight, Bitmap.Config.ARGB_8888); * Canvas c = new Canvas(bmp); * float yOffset = 0f; * int prevOffset = 0; @@ -349,8 +349,9 @@ public class LineBreaker { private static final int END_HYPHEN_MASK = 0x7; // 0b00111 private static final int START_HYPHEN_BITS_SHIFT = 3; - private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - Result.class.getClassLoader(), nGetReleaseResultFunc(), 32); + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + Result.class.getClassLoader(), nGetReleaseResultFunc()); private final long mPtr; private Result(long ptr) { @@ -444,8 +445,9 @@ public class LineBreaker { } } - private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - LineBreaker.class.getClassLoader(), nGetReleaseFunc(), 64); + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + LineBreaker.class.getClassLoader(), nGetReleaseFunc()); private final long mNativePtr; diff --git a/graphics/java/android/graphics/text/MeasuredText.java b/graphics/java/android/graphics/text/MeasuredText.java index 480aff289dfce..66bcd8650b95b 100644 --- a/graphics/java/android/graphics/text/MeasuredText.java +++ b/graphics/java/android/graphics/text/MeasuredText.java @@ -169,8 +169,9 @@ public class MeasuredText { * Note: The appendStyle and appendReplacementRun should be called to cover the text length. */ public static final class Builder { - private static final NativeAllocationRegistry sRegistry = new NativeAllocationRegistry( - MeasuredText.class.getClassLoader(), nGetReleaseFunc(), 1024); + private static final NativeAllocationRegistry sRegistry = + NativeAllocationRegistry.createMalloced( + MeasuredText.class.getClassLoader(), nGetReleaseFunc()); private long mNativePtr; diff --git a/libs/hwui/hwui/Bitmap.cpp b/libs/hwui/hwui/Bitmap.cpp index 3bbee18c6dd1e..219d04055eae7 100644 --- a/libs/hwui/hwui/Bitmap.cpp +++ b/libs/hwui/hwui/Bitmap.cpp @@ -287,6 +287,8 @@ size_t Bitmap::getAllocationByteCount() const { switch (mPixelStorageType) { case PixelStorageType::Heap: return mPixelStorage.heap.size; + case PixelStorageType::Ashmem: + return mPixelStorage.ashmem.size; default: return rowBytes() * height(); } diff --git a/libs/hwui/hwui/Bitmap.h b/libs/hwui/hwui/Bitmap.h index 01e45166e0a38..dd98b25ac7e8c 100644 --- a/libs/hwui/hwui/Bitmap.h +++ b/libs/hwui/hwui/Bitmap.h @@ -103,6 +103,8 @@ public: bool isHardware() const { return mPixelStorageType == PixelStorageType::Hardware; } + PixelStorageType pixelStorageType() const { return mPixelStorageType; } + GraphicBuffer* graphicBuffer(); /**