From d83de421024882311c0aa4d835c8fce6a0550f8a Mon Sep 17 00:00:00 2001 From: judds Date: Fri, 18 Aug 2017 09:59:25 -0700 Subject: [PATCH] Support HARDWARE Bitmaps in Android O+ in Glide. From adb shell dumpsys meminfo com.google.android.apps.photos on a 2016 Pixel on OPM1.170816.001: Before: App Summary Pss(KB) ------ Java Heap: 20360 Native Heap: 66032 Code: 93336 Stack: 1192 Graphics: 85892 Private Other: 8956 System: 19944 TOTAL: 295712 TOTAL SWAP PSS: 82 After: App Summary Pss(KB) ------ Java Heap: 20456 Native Heap: 39756 Code: 93384 Stack: 1220 Graphics: 81460 Private Other: 9024 System: 11662 TOTAL: 256962 TOTAL SWAP PSS: 81 These numbers aren't super solid. Some extra invalidations can dump more Bitmaps into our Bitmap pool which will make it look like the steady state memory usage is increasing even though the maximum amount remains the same. That said, I see between a 33% and 50% improvement in native heap usage after this change. This was tested by flinging back and forth in the All grid in Photos. This change is limited to HARDWARE Bitmap support. We ought to also be able to reduce the bitmap pool size in O+ now that we only care about re-using Bitmaps for very small images or while generating thumbnails for the first time. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=165717498 --- .../engine/bitmap_recycle/LruBitmapPool.java | 8 +- .../load/resource/bitmap/Downsampler.java | 39 ++++-- .../resource/bitmap/HardwareConfigState.java | 115 ++++++++++++++++++ 3 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java diff --git a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java index 2cf9c0f37f..892c49d251 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/bitmap_recycle/LruBitmapPool.java @@ -248,9 +248,15 @@ private static LruPoolStrategy getDefaultStrategy() { private static Set getDefaultAllowedConfigs() { Set configs = new HashSet<>(); configs.addAll(Arrays.asList(Bitmap.Config.values())); - if (Build.VERSION.SDK_INT >= 19) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { + // GIFs, among other types, end up with a native Bitmap config that doesn't map to a java + // config and is treated as null in java code. On KitKat+ these Bitmaps can be reconfigured + // and are suitable for re-use. configs.add(null); } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + configs.remove(Bitmap.Config.HARDWARE); + } return Collections.unmodifiableSet(configs); } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java index 1d71f533c2..78ac6813cb 100644 --- a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/Downsampler.java @@ -2,6 +2,7 @@ import android.annotation.TargetApi; import android.graphics.Bitmap; +import android.graphics.Bitmap.Config; import android.graphics.BitmapFactory; import android.os.Build; import android.support.annotation.Nullable; @@ -34,7 +35,7 @@ * Downsamples, decodes, and rotates images according to their exif orientation. */ public final class Downsampler { - private static final String TAG = "Downsampler"; + static final String TAG = "Downsampler"; /** * Indicates the {@link com.bumptech.glide.load.DecodeFormat} that will be used in conjunction * with the image format to determine the {@link android.graphics.Bitmap.Config} to provide to @@ -50,7 +51,6 @@ public final class Downsampler { public static final Option DOWNSAMPLE_STRATEGY = Option.memory("com.bumptech.glide.load.resource.bitmap.Downsampler.DownsampleStrategy", DownsampleStrategy.AT_LEAST); - /** * Ensure that the size of the bitmap is fixed to the requested width and height of the * resource from the caller. The final resource dimensions may differ from the requested @@ -103,6 +103,7 @@ public void onDecodeComplete(BitmapPool bitmapPool, Bitmap downsampled) throws I private final DisplayMetrics displayMetrics; private final ArrayPool byteArrayPool; private final List parsers; + private final HardwareConfigState hardwareConfigState = HardwareConfigState.getInstance(); public Downsampler(List parsers, DisplayMetrics displayMetrics, BitmapPool bitmapPool, ArrayPool byteArrayPool) { @@ -196,16 +197,13 @@ private Bitmap decodeFromWrappedStreams(InputStream is, int orientation = ImageHeaderParserUtils.getOrientation(parsers, is, byteArrayPool); int degreesToRotate = TransformationUtils.getExifOrientationDegrees(orientation); - options.inPreferredConfig = getConfig(is, decodeFormat); - if (options.inPreferredConfig != Bitmap.Config.ARGB_8888) { - options.inDither = true; - } int targetWidth = requestedWidth == Target.SIZE_ORIGINAL ? sourceWidth : requestedWidth; int targetHeight = requestedHeight == Target.SIZE_ORIGINAL ? sourceHeight : requestedHeight; calculateScaling(downsampleStrategy, degreesToRotate, sourceWidth, sourceHeight, targetWidth, targetHeight, options); + calculateConfig(is, decodeFormat, options, targetWidth, targetHeight); boolean isKitKatOrGreater = Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT; // Prior to KitKat, the inBitmap size must exactly match the size of the bitmap we're decoding. @@ -365,11 +363,24 @@ private boolean shouldUsePool(InputStream is) throws IOException { return false; } - private Bitmap.Config getConfig(InputStream is, DecodeFormat format) throws IOException { + private void calculateConfig( + InputStream is, + DecodeFormat format, + BitmapFactory.Options optionsWithScaling, + int targetWidth, + int targetHeight) + throws IOException { + + if (hardwareConfigState.setHardwareConfigIfAllowed( + targetWidth, targetHeight, optionsWithScaling)) { + return; + } + // Changing configs can cause skewing on 4.1, see issue #128. if (format == DecodeFormat.PREFER_ARGB_8888 || Build.VERSION.SDK_INT == Build.VERSION_CODES.JELLY_BEAN) { - return Bitmap.Config.ARGB_8888; + optionsWithScaling.inPreferredConfig = Bitmap.Config.ARGB_8888; + return; } boolean hasAlpha = false; @@ -382,7 +393,13 @@ private Bitmap.Config getConfig(InputStream is, DecodeFormat format) throws IOEx } } - return hasAlpha ? Bitmap.Config.ARGB_8888 : Bitmap.Config.RGB_565; + optionsWithScaling.inPreferredConfig = + hasAlpha ? Bitmap.Config.ARGB_8888 : Bitmap.Config.RGB_565; + if (optionsWithScaling.inPreferredConfig == Config.RGB_565 + || optionsWithScaling.inPreferredConfig == Config.ARGB_4444 + || optionsWithScaling.inPreferredConfig == Config.ALPHA_8) { + optionsWithScaling.inDither = true; + } } /** @@ -500,6 +517,10 @@ private static IOException newIoExceptionForInBitmapAssertion(IllegalArgumentExc private static void setInBitmap(BitmapFactory.Options options, BitmapPool bitmapPool, int width, int height) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O + && options.inPreferredConfig == Config.HARDWARE) { + return; + } // BitmapFactory will clear out the Bitmap before writing to it, so getDirty is safe. options.inBitmap = bitmapPool.getDirty(width, height, options.inPreferredConfig); } diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java new file mode 100644 index 0000000000..58a126cac0 --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java @@ -0,0 +1,115 @@ +package com.bumptech.glide.load.resource.bitmap; + +import android.graphics.Bitmap; +import android.graphics.BitmapFactory; +import android.os.Build; +import android.util.Log; +import java.io.File; + +/** + * State and constants for interacting with {@link android.graphics.Bitmap.Config#HARDWARE} on + * Android O+. + */ +final class HardwareConfigState { + /** + * The minimum size in pixels a {@link Bitmap} must be in both dimensions to be created with the + * {@link Bitmap.Config#HARDWARE} configuration. + * + *

This is a quick check that lets us skip wasting FDs (see {@link #FD_SIZE_LIST}) on small + * {@link Bitmap}s with relatively low memory costs. + * + * @see #FD_SIZE_LIST + */ + private static final int MIN_HARDWARE_DIMENSION = 128; + + /** + * Allows us to check to make sure we're not exceeding the FD limit for a process with hardware + * {@link Bitmap}s. + * + *

{@link Bitmap.Config#HARDWARE} {@link Bitmap}s require two FDs (depending on the driver). + * Processes have an FD limit of 1024 (at least on O). With sufficiently small {@link Bitmap}s + * and/or a sufficiently large {@link com.bumptech.glide.load.engine.cache.MemoryCache}, we can + * end up with enough {@link Bitmap}s in memory that we blow through the FD limit, which causes + * graphics errors, Binder errors, and a variety of crashes. + * + *

Calling list.size() should be relatively efficient (hopefully < 1ms on average) because + * /proc is an in-memory FS. + */ + private static final File FD_SIZE_LIST = new File("/proc/self/fd"); + + /** + * Each FD check takes 1-2ms, so to avoid overhead, only check every N decodes. 50 is more or less + * arbitrary. + */ + private static final int MINIMUM_DECODES_BETWEEN_FD_CHECKS = 50; + + /** + * 700 with an error of 50 Bitmaps in between at two FDs each lets us use up to 800 FDs for + * hardware Bitmaps. + */ + private static final int MAXIMUM_FDS_FOR_HARDWARE_CONFIGS = 700; + + /** + * The minimum size that will trigger downsampling in {@link BitmapFactory}. + * + *

From {@link android.graphics.BitmapFactory.Options#inSampleSize}. + */ + private static final int MINIMUM_SAMPLE_SIZE = 2; + + private volatile int decodesSinceLastFdCheck; + private volatile boolean isHardwareConfigAllowed = true; + + private static volatile HardwareConfigState instance; + + static HardwareConfigState getInstance() { + if (instance == null) { + synchronized (HardwareConfigState.class) { + if (instance == null) { + instance = new HardwareConfigState(); + } + } + } + return instance; + } + + private HardwareConfigState() { + // Singleton constructor. + } + + boolean setHardwareConfigIfAllowed( + int targetWidth, int targetHeight, BitmapFactory.Options optionsWithScaling) { + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { + return false; + } + + boolean result = !optionsWithScaling.inScaled + && optionsWithScaling.inSampleSize < MINIMUM_SAMPLE_SIZE + && targetWidth >= MIN_HARDWARE_DIMENSION + && targetHeight >= MIN_HARDWARE_DIMENSION + // Make sure to call isFdSizeBelowHardwareLimit last because it has side affects. + && isFdSizeBelowHardwareLimit(); + + if (result) { + optionsWithScaling.inPreferredConfig = Bitmap.Config.HARDWARE; + optionsWithScaling.inMutable = false; + } + return result; + } + + private synchronized boolean isFdSizeBelowHardwareLimit() { + if (++decodesSinceLastFdCheck >= MINIMUM_DECODES_BETWEEN_FD_CHECKS) { + decodesSinceLastFdCheck = 0; + int currentFds = FD_SIZE_LIST.list().length; + isHardwareConfigAllowed = currentFds < MAXIMUM_FDS_FOR_HARDWARE_CONFIGS; + + if (!isHardwareConfigAllowed && Log.isLoggable(Downsampler.TAG, Log.WARN)) { + Log.w(Downsampler.TAG, + "Excluding HARDWARE bitmap config because we're over the file descriptor limit" + + ", file descriptors " + currentFds + + ", limit " + MAXIMUM_FDS_FOR_HARDWARE_CONFIGS); + } + } + + return isHardwareConfigAllowed; + } +}