Skip to content

Commit

Permalink
Reduce a variety of object allocations across Glide.
Browse files Browse the repository at this point in the history
  • Loading branch information
sjudd committed Nov 22, 2017
1 parent 5dc6460 commit b4d778b
Show file tree
Hide file tree
Showing 25 changed files with 354 additions and 79 deletions.
6 changes: 6 additions & 0 deletions library/findbugs-exclude.xml
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,10 @@
<Bug pattern="DMI_HARDCODED_ABSOLUTE_FILENAME" />
</Match>

<!-- Re-using the byte array saves allocations, there's no serious risk of mutations. -->
<Match>
<Class name="com.bumptech.glide.util.FixedPreloadSizeProvider" />
<Bug pattern="EI_EXPOSE_REP" />
</Match>

</FindBugsFilter>
10 changes: 8 additions & 2 deletions library/src/main/java/com/bumptech/glide/Glide.java
Original file line number Diff line number Diff line change
Expand Up @@ -447,8 +447,14 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
ImageViewTargetFactory imageViewTargetFactory = new ImageViewTargetFactory();
glideContext =
new GlideContext(
context, registry, imageViewTargetFactory, defaultRequestOptions,
defaultTransitionOptions, engine, logLevel);
context,
arrayPool,
registry,
imageViewTargetFactory,
defaultRequestOptions,
defaultTransitionOptions,
engine,
logLevel);
}

/**
Expand Down
18 changes: 15 additions & 3 deletions library/src/main/java/com/bumptech/glide/GlideContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import android.support.annotation.VisibleForTesting;
import android.widget.ImageView;
import com.bumptech.glide.load.engine.Engine;
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
import com.bumptech.glide.request.RequestOptions;
import com.bumptech.glide.request.target.ImageViewTargetFactory;
import com.bumptech.glide.request.target.ViewTarget;
Expand All @@ -26,18 +27,25 @@ public class GlideContext extends ContextWrapper {
static final TransitionOptions<?, ?> DEFAULT_TRANSITION_OPTIONS =
new GenericTransitionOptions<>();
private final Handler mainHandler;
private final ArrayPool arrayPool;
private final Registry registry;
private final ImageViewTargetFactory imageViewTargetFactory;
private final RequestOptions defaultRequestOptions;
private final Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions;
private final Engine engine;
private final int logLevel;

public GlideContext(Context context, Registry registry,
ImageViewTargetFactory imageViewTargetFactory, RequestOptions defaultRequestOptions,
Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions, Engine engine,
public GlideContext(
Context context,
ArrayPool arrayPool,
Registry registry,
ImageViewTargetFactory imageViewTargetFactory,
RequestOptions defaultRequestOptions,
Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions,
Engine engine,
int logLevel) {
super(context.getApplicationContext());
this.arrayPool = arrayPool;
this.registry = registry;
this.imageViewTargetFactory = imageViewTargetFactory;
this.defaultRequestOptions = defaultRequestOptions;
Expand Down Expand Up @@ -89,4 +97,8 @@ public Registry getRegistry() {
public int getLogLevel() {
return logLevel;
}

public ArrayPool getArrayPool() {
return arrayPool;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ public static ImageType getType(List<ImageHeaderParser> parsers, @Nullable Input
}

is.mark(MARK_POSITION);
for (ImageHeaderParser parser : parsers) {
int size = parsers.size();
for (int i = 0; i < size; i++) {
ImageHeaderParser parser = parsers.get(i);
try {
ImageType type = parser.getType(is);
if (type != ImageType.UNKNOWN) {
Expand All @@ -52,7 +54,9 @@ public static ImageType getType(List<ImageHeaderParser> parsers, @Nullable ByteB
return ImageType.UNKNOWN;
}

for (ImageHeaderParser parser : parsers) {
int size = parsers.size();
for (int i = 0; i < size; i++) {
ImageHeaderParser parser = parsers.get(i);
ImageType type = parser.getType(buffer);
if (type != ImageType.UNKNOWN) {
return type;
Expand All @@ -74,7 +78,9 @@ public static int getOrientation(List<ImageHeaderParser> parsers, @Nullable Inpu
}

is.mark(MARK_POSITION);
for (ImageHeaderParser parser : parsers) {
int size = parsers.size();
for (int i = 0; i < size; i++) {
ImageHeaderParser parser = parsers.get(i);
try {
int orientation = parser.getOrientation(is, byteArrayPool);
if (orientation != ImageHeaderParser.UNKNOWN_ORIENTATION) {
Expand Down
7 changes: 4 additions & 3 deletions library/src/main/java/com/bumptech/glide/load/Options.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import android.support.v4.util.ArrayMap;
import android.support.v4.util.SimpleArrayMap;
import java.security.MessageDigest;
import java.util.Map;

/**
* A set of {@link Option Options} to apply to in memory and disk cache keys.
Expand Down Expand Up @@ -41,8 +40,10 @@ public int hashCode() {

@Override
public void updateDiskCacheKey(MessageDigest messageDigest) {
for (Map.Entry<Option<?>, Object> entry : values.entrySet()) {
updateDiskCacheKey(entry.getKey(), entry.getValue(), messageDigest);
for (int i = 0; i < values.size(); i++) {
Option<?> key = values.keyAt(i);
Object value = values.valueAt(i);
updateDiskCacheKey(key, value, messageDigest);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import com.bumptech.glide.load.ResourceEncoder;
import com.bumptech.glide.load.Transformation;
import com.bumptech.glide.load.engine.DecodeJob.DiskCacheProvider;
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
import com.bumptech.glide.load.engine.cache.DiskCache;
import com.bumptech.glide.load.model.ModelLoader;
import com.bumptech.glide.load.model.ModelLoader.LoadData;
Expand Down Expand Up @@ -119,6 +120,10 @@ int getHeight() {
return height;
}

ArrayPool getArrayPool() {
return glideContext.getArrayPool();
}

List<Class<?>> getRegisteredResourceClasses() {
return glideContext.getRegistry()
.getRegisteredResourceClasses(model.getClass(), resourceClass, transcodeClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,16 @@ public Resource<Z> onResourceDecoded(Resource<Z> decoded) {
if (encodeStrategy == EncodeStrategy.SOURCE) {
key = new DataCacheKey(currentSourceKey, signature);
} else if (encodeStrategy == EncodeStrategy.TRANSFORMED) {
key = new ResourceCacheKey(currentSourceKey, signature, width, height,
appliedTransformation, resourceSubClass, options);
key =
new ResourceCacheKey(
decodeHelper.getArrayPool(),
currentSourceKey,
signature,
width,
height,
appliedTransformation,
resourceSubClass,
options);
} else {
throw new IllegalArgumentException("Unknown strategy: " + encodeStrategy);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,9 @@ void handleResultOnMainThread() {
engineResource.acquire();
listener.onEngineJobComplete(key, engineResource);

for (ResourceCallback cb : cbs) {
int size = cbs.size();
for (int i = 0; i < size; i++) {
ResourceCallback cb = cbs.get(i);
if (!isInIgnoredCallbacks(cb)) {
engineResource.acquire();
cb.onResourceReady(engineResource, dataSource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,16 @@ public boolean startNext() {
Class<?> resourceClass = resourceClasses.get(resourceClassIndex);
Transformation<?> transformation = helper.getTransformation(resourceClass);

currentKey = new ResourceCacheKey(sourceId, helper.getSignature(), helper.getWidth(),
helper.getHeight(), transformation, resourceClass, helper.getOptions());
currentKey =
new ResourceCacheKey(
helper.getArrayPool(),
sourceId,
helper.getSignature(),
helper.getWidth(),
helper.getHeight(),
transformation,
resourceClass,
helper.getOptions());
cacheFile = helper.getDiskCache().get(currentKey);
if (cacheFile != null) {
this.sourceKey = sourceId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.bumptech.glide.load.Key;
import com.bumptech.glide.load.Options;
import com.bumptech.glide.load.Transformation;
import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool;
import com.bumptech.glide.util.LruCache;
import com.bumptech.glide.util.Util;
import java.nio.ByteBuffer;
Expand All @@ -13,6 +14,7 @@
*/
final class ResourceCacheKey implements Key {
private static final LruCache<Class<?>, byte[]> RESOURCE_CLASS_BYTES = new LruCache<>(50);
private final ArrayPool arrayPool;
private final Key sourceKey;
private final Key signature;
private final int width;
Expand All @@ -21,8 +23,16 @@ final class ResourceCacheKey implements Key {
private final Options options;
private final Transformation<?> transformation;

ResourceCacheKey(Key sourceKey, Key signature, int width, int height,
Transformation<?> appliedTransformation, Class<?> decodedResourceClass, Options options) {
ResourceCacheKey(
ArrayPool arrayPool,
Key sourceKey,
Key signature,
int width,
int height,
Transformation<?> appliedTransformation,
Class<?> decodedResourceClass,
Options options) {
this.arrayPool = arrayPool;
this.sourceKey = sourceKey;
this.signature = signature;
this.width = width;
Expand Down Expand Up @@ -63,7 +73,8 @@ public int hashCode() {
// TODO: Include relevant options?
@Override
public void updateDiskCacheKey(MessageDigest messageDigest) {
byte[] dimensions = ByteBuffer.allocate(8).putInt(width).putInt(height).array();
byte[] dimensions = arrayPool.getExact(8, byte[].class);
ByteBuffer.wrap(dimensions).putInt(width).putInt(height).array();
signature.updateDiskCacheKey(messageDigest);
sourceKey.updateDiskCacheKey(messageDigest);
messageDigest.update(dimensions);
Expand All @@ -72,6 +83,7 @@ public void updateDiskCacheKey(MessageDigest messageDigest) {
}
options.updateDiskCacheKey(messageDigest);
messageDigest.update(getResourceClassBytes());
arrayPool.put(dimensions);
}

private byte[] getResourceClassBytes() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,42 @@ public interface ArrayPool {
*
* <p>Arrays may be ignored, for example if the array is larger than the maximum size of the
* pool.
*
* @deprecated Use {@link #put(Object)Class}
*/
@Deprecated
<T> void put(T array, Class<T> arrayClass);

/**
* Optionally adds the given array of the given type to the pool.
*
* <p>Arrays may be ignored, for example if the array is larger than the maximum size of the
* pool.
*/
<T> void put(T array);

/**
* Returns a non-null array of the given type with a length >= to the given size.
*
* <p>If an array of the given size isn't in the pool, a new one will be allocated.
*
* <p>This class makes no guarantees about the contents of the returned array.
*
* @see #getExact(int, Class)
*/
<T> T get(int size, Class<T> arrayClass);

/**
* Returns a non-null array of the given type with a length exactly equal to the given size.
*
* <p>If an array of the given size isn't in the pool, a new one will be allocated.
*
* <p>This class makes no guarantees about the contents of the returned array.
*
* @see #get(int, Class)
*/
<T> T getExact(int size, Class<T> arrayClass);

/**
* Clears all arrays from the pool.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ public final class LruArrayPool implements ArrayPool {
* The maximum number of times larger an int array may be to be than a requested size to eligible
* to be returned from the pool.
*/
private static final int MAX_OVER_SIZE_MULTIPLE = 8;
@VisibleForTesting
static final int MAX_OVER_SIZE_MULTIPLE = 8;
/** Used to calculate the maximum % of the total pool size a single byte array may consume. */
private static final int SINGLE_ARRAY_MAX_SIZE_DIVISOR = 2;

Expand All @@ -47,8 +48,17 @@ public LruArrayPool(int maxSize) {
this.maxSize = maxSize;
}

@Deprecated
@Override
public synchronized <T> void put(T array, Class<T> arrayClass) {
public <T> void put(T array, Class<T> arrayClass) {
put(array);
}

@Override
public synchronized <T> void put(T array) {
@SuppressWarnings("unchecked")
Class<T> arrayClass = (Class<T>) array.getClass();

ArrayAdapterInterface<T> arrayAdapter = getAdapterFromType(arrayClass);
int size = arrayAdapter.getArrayLength(array);
int arrayBytes = size * arrayAdapter.getElementSizeInBytes();
Expand All @@ -66,30 +76,36 @@ public synchronized <T> void put(T array, Class<T> arrayClass) {
}

@Override
public <T> T get(int size, Class<T> arrayClass) {
ArrayAdapterInterface<T> arrayAdapter = getAdapterFromType(arrayClass);
T result;
synchronized (this) {
Integer possibleSize = getSizesForAdapter(arrayClass).ceilingKey(size);
final Key key;
if (mayFillRequest(size, possibleSize)) {
key = keyPool.get(possibleSize, arrayClass);
} else {
key = keyPool.get(size, arrayClass);
}
public synchronized <T> T getExact(int size, Class<T> arrayClass) {
Key key = keyPool.get(size, arrayClass);
return getForKey(key, arrayClass);
}

result = getArrayForKey(key);
if (result != null) {
currentSize -= arrayAdapter.getArrayLength(result) * arrayAdapter.getElementSizeInBytes();
decrementArrayOfSize(arrayAdapter.getArrayLength(result), arrayClass);
}
@Override
public synchronized <T> T get(int size, Class<T> arrayClass) {
Integer possibleSize = getSizesForAdapter(arrayClass).ceilingKey(size);
final Key key;
if (mayFillRequest(size, possibleSize)) {
key = keyPool.get(possibleSize, arrayClass);
} else {
key = keyPool.get(size, arrayClass);
}
return getForKey(key, arrayClass);
}

private <T> T getForKey(Key key, Class<T> arrayClass) {
ArrayAdapterInterface<T> arrayAdapter = getAdapterFromType(arrayClass);
T result = getArrayForKey(key);
if (result != null) {
currentSize -= arrayAdapter.getArrayLength(result) * arrayAdapter.getElementSizeInBytes();
decrementArrayOfSize(arrayAdapter.getArrayLength(result), arrayClass);
}

if (result == null) {
if (Log.isLoggable(arrayAdapter.getTag(), Log.VERBOSE)) {
Log.v(arrayAdapter.getTag(), "Allocated " + size + " bytes");
Log.v(arrayAdapter.getTag(), "Allocated " + key.size + " bytes");
}
result = arrayAdapter.newArray(size);
result = arrayAdapter.newArray(key.size);
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public boolean encode(InputStream data, File file, Options options) {
// Do nothing.
}
}
byteArrayPool.put(buffer, byte[].class);
byteArrayPool.put(buffer);
}
return success;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ private int getOrientation(Reader reader, ArrayPool byteArrayPool) throws IOExce
try {
return parseExifSegment(reader, exifData, exifSegmentLength);
} finally {
byteArrayPool.put(exifData, byte[].class);
byteArrayPool.put(exifData);
}
}
}
Expand Down
Loading

0 comments on commit b4d778b

Please sign in to comment.