Skip to content

Commit

Permalink
Add an animation executor to decode GIF frames on.
Browse files Browse the repository at this point in the history
Our normal source executor is often used to load data from networks. Glide’s default networking library runs all networking operations on Glide’s source executor. OkHttp runs the initial connection on its own
thread pool, but we still read and write the body of each request to our ache on Glide’s source executor. Even libraries like Volley that buffer
the entire request into memory will still require a few hundred milliseconds of Glide’s source executor’s time to write the request to 
cache.

Scrolling through a list of images will enqueue a large number of requests. If each request is relatively time consuming, it’s likely that
all of Glide’s source executor threads will end up blocked for 
significant periods of time, even if the longer requests are lower priority. Using a different executor for GIF frames and other animations
lets us avoid junk during playback. 

Unfortunately there is a CPU and memory hit to using an additional executor. I’ve tried to limit that here by using only one or two threads
and a cached thread pool so that few threads are created and are only
created when an animation is actually run.

Fixes #899
  • Loading branch information
sjudd committed Oct 11, 2017
1 parent 6cd8289 commit c4db04e
Show file tree
Hide file tree
Showing 11 changed files with 366 additions and 97 deletions.
10 changes: 8 additions & 2 deletions library/src/main/java/com/bumptech/glide/GlideBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,14 @@ public Glide build(Context context) {
}

if (engine == null) {
engine = new Engine(memoryCache, diskCacheFactory, diskCacheExecutor, sourceExecutor,
GlideExecutor.newUnlimitedSourceExecutor());
engine =
new Engine(
memoryCache,
diskCacheFactory,
diskCacheExecutor,
sourceExecutor,
GlideExecutor.newUnlimitedSourceExecutor(),
GlideExecutor.newAnimationExecutor());
}

RequestManagerRetriever requestManagerRetriever = new RequestManagerRetriever(
Expand Down
50 changes: 38 additions & 12 deletions library/src/main/java/com/bumptech/glide/load/engine/Engine.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,21 @@ public Engine(MemoryCache memoryCache,
DiskCache.Factory diskCacheFactory,
GlideExecutor diskCacheExecutor,
GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor) {
this(memoryCache, diskCacheFactory, diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor,
null, null, null, null, null, null);
GlideExecutor sourceUnlimitedExecutor,
GlideExecutor animationExecutor) {
this(
memoryCache,
diskCacheFactory,
diskCacheExecutor,
sourceExecutor,
sourceUnlimitedExecutor,
animationExecutor,
/*jobs=*/ null,
/*keyFactory=*/ null,
/*activeResources=*/ null,
/*engineJobFactory=*/ null,
/*decodeJobFactory=*/ null,
/*resourceRecycler=*/ null);
}

// Visible for testing.
Expand All @@ -77,6 +89,7 @@ public Engine(MemoryCache memoryCache,
GlideExecutor diskCacheExecutor,
GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor,
GlideExecutor animationExecutor,
Map<Key, EngineJob<?>> jobs,
EngineKeyFactory keyFactory,
Map<Key, WeakReference<EngineResource<?>>> activeResources,
Expand All @@ -102,8 +115,9 @@ public Engine(MemoryCache memoryCache,
this.jobs = jobs;

if (engineJobFactory == null) {
engineJobFactory = new EngineJobFactory(diskCacheExecutor, sourceExecutor,
sourceUnlimitedExecutor, this);
engineJobFactory =
new EngineJobFactory(
diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, animationExecutor, this);
}
this.engineJobFactory = engineJobFactory;

Expand Down Expand Up @@ -155,6 +169,7 @@ public <R> LoadStatus load(
Options options,
boolean isMemoryCacheable,
boolean useUnlimitedSourceExecutorPool,
boolean useAnimationPool,
boolean onlyRetrieveFromCache,
ResourceCallback cb) {
Util.assertMainThread();
Expand Down Expand Up @@ -191,7 +206,7 @@ public <R> LoadStatus load(
}

EngineJob<R> engineJob = engineJobFactory.build(key, isMemoryCacheable,
useUnlimitedSourceExecutorPool);
useUnlimitedSourceExecutorPool, useAnimationPool);
DecodeJob<R> decodeJob = decodeJobFactory.build(
glideContext,
model,
Expand Down Expand Up @@ -453,29 +468,40 @@ static class EngineJobFactory {
@Synthetic final GlideExecutor diskCacheExecutor;
@Synthetic final GlideExecutor sourceExecutor;
@Synthetic final GlideExecutor sourceUnlimitedExecutor;
@Synthetic final GlideExecutor animationExecutor;
@Synthetic final EngineJobListener listener;
@Synthetic final Pools.Pool<EngineJob<?>> pool = FactoryPools.simple(JOB_POOL_SIZE,
new FactoryPools.Factory<EngineJob<?>>() {
@Override
public EngineJob<?> create() {
return new EngineJob<Object>(diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor,
listener, pool);
return new EngineJob<Object>(
diskCacheExecutor,
sourceExecutor,
sourceUnlimitedExecutor,
animationExecutor,
listener,
pool);
}
});

EngineJobFactory(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor, EngineJobListener listener) {
EngineJobFactory(
GlideExecutor diskCacheExecutor,
GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor,
GlideExecutor animationExecutor,
EngineJobListener listener) {
this.diskCacheExecutor = diskCacheExecutor;
this.sourceExecutor = sourceExecutor;
this.sourceUnlimitedExecutor = sourceUnlimitedExecutor;
this.animationExecutor = animationExecutor;
this.listener = listener;
}

@SuppressWarnings("unchecked")
<R> EngineJob<R> build(Key key, boolean isMemoryCacheable,
boolean useUnlimitedSourceGeneratorPool) {
boolean useUnlimitedSourceGeneratorPool, boolean isAnimation) {
EngineJob<R> result = (EngineJob<R>) pool.acquire();
return result.init(key, isMemoryCacheable, useUnlimitedSourceGeneratorPool);
return result.init(key, isMemoryCacheable, useUnlimitedSourceGeneratorPool, isAnimation);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,12 @@ class EngineJob<R> implements DecodeJob.Callback<R>,
private final GlideExecutor diskCacheExecutor;
private final GlideExecutor sourceExecutor;
private final GlideExecutor sourceUnlimitedExecutor;
private final GlideExecutor animationExecutor;

private Key key;
private boolean isCacheable;
private boolean useUnlimitedSourceGeneratorPool;
private boolean isAnimation;
private Resource<?> resource;
private DataSource dataSource;
private boolean hasResource;
Expand All @@ -57,31 +59,51 @@ class EngineJob<R> implements DecodeJob.Callback<R>,
// Checked primarily on the main thread, but also on other threads in reschedule.
private volatile boolean isCancelled;

EngineJob(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor,
EngineJob(
GlideExecutor diskCacheExecutor,
GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor,
EngineJobListener listener, Pools.Pool<EngineJob<?>> pool) {
this(diskCacheExecutor, sourceExecutor, sourceUnlimitedExecutor, listener, pool,
GlideExecutor animationExecutor,
EngineJobListener listener,
Pools.Pool<EngineJob<?>> pool) {
this(
diskCacheExecutor,
sourceExecutor,
sourceUnlimitedExecutor,
animationExecutor,
listener,
pool,
DEFAULT_FACTORY);
}

// Visible for testing.
EngineJob(GlideExecutor diskCacheExecutor, GlideExecutor sourceExecutor,
EngineJob(
GlideExecutor diskCacheExecutor,
GlideExecutor sourceExecutor,
GlideExecutor sourceUnlimitedExecutor,
EngineJobListener listener, Pools.Pool<EngineJob<?>> pool,
GlideExecutor animationExecutor,
EngineJobListener listener,
Pools.Pool<EngineJob<?>> pool,
EngineResourceFactory engineResourceFactory) {
this.diskCacheExecutor = diskCacheExecutor;
this.sourceExecutor = sourceExecutor;
this.sourceUnlimitedExecutor = sourceUnlimitedExecutor;
this.animationExecutor = animationExecutor;
this.listener = listener;
this.pool = pool;
this.engineResourceFactory = engineResourceFactory;
}

// Visible for testing.
EngineJob<R> init(Key key, boolean isCacheable, boolean useUnlimitedSourceGeneratorPool) {
EngineJob<R> init(
Key key,
boolean isCacheable,
boolean useUnlimitedSourceGeneratorPool,
boolean isAnimation) {
this.key = key;
this.isCacheable = isCacheable;
this.useUnlimitedSourceGeneratorPool = useUnlimitedSourceGeneratorPool;
this.isAnimation = isAnimation;
return this;
}

Expand Down Expand Up @@ -119,6 +141,9 @@ public void removeCallback(ResourceCallback cb) {
}

private GlideExecutor getActiveSourceExecutor() {
if (isAnimation) {
return animationExecutor;
}
return useUnlimitedSourceGeneratorPool ? sourceUnlimitedExecutor : sourceExecutor;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ public final class GlideExecutor extends ThreadPoolExecutor {
private static final String CPU_LOCATION = "/sys/devices/system/cpu/";
// Don't use more than four threads when automatically determining thread count..
private static final int MAXIMUM_AUTOMATIC_THREAD_COUNT = 4;
// May be accessed on other threads, but this is an optimization only so it's ok if we set its
// value more than once.
private static volatile int bestThreadCount;
private final boolean executeSynchronously;

/**
Expand All @@ -52,10 +55,11 @@ public final class GlideExecutor extends ThreadPoolExecutor {
*/
private static final String SOURCE_UNLIMITED_EXECUTOR_NAME = "source-unlimited";
/**
* The default keep alive time for threads in source unlimited executor pool in milliseconds.
* The default keep alive time for threads in our cached thread pools in milliseconds.
*/
private static final long SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS =
TimeUnit.SECONDS.toMillis(10);
private static final long KEEP_ALIVE_TIME_MS = TimeUnit.SECONDS.toMillis(10);

private static final String ANIMATION_EXECUTOR_NAME = "animation";

/**
* Returns a new fixed thread pool with the default thread count returned from
Expand Down Expand Up @@ -160,7 +164,7 @@ public static GlideExecutor newSourceExecutor(int threadCount, String name,

/**
* Returns a new unlimited thread pool with zero core thread count to make sure no threads are
* created by default, {@link #SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS} keep alive
* created by default, {@link #KEEP_ALIVE_TIME_MS} keep alive
* time, the {@link #SOURCE_UNLIMITED_EXECUTOR_NAME} thread name prefix, the
* {@link com.bumptech.glide.load.engine.executor.GlideExecutor.UncaughtThrowableStrategy#DEFAULT}
* uncaught throwable strategy, and the {@link SynchronousQueue} since using default unbounded
Expand All @@ -175,14 +179,33 @@ public static GlideExecutor newSourceExecutor(int threadCount, String name,
public static GlideExecutor newUnlimitedSourceExecutor() {
return new GlideExecutor(0 /* corePoolSize */,
Integer.MAX_VALUE /* maximumPoolSize */,
SOURCE_UNLIMITED_EXECUTOR_KEEP_ALIVE_TIME_MS,
KEEP_ALIVE_TIME_MS,
SOURCE_UNLIMITED_EXECUTOR_NAME,
UncaughtThrowableStrategy.DEFAULT,
false /*preventNetworkOperations*/,
false /*executeSynchronously*/,
new SynchronousQueue<Runnable>());
}

public static GlideExecutor newAnimationExecutor() {
int bestThreadCount = calculateBestThreadCount();
// We don't want to add a ton of threads running animations in parallel with our source and
// disk cache executors. Doing so adds unnecessary CPU load and can also dramatically increase
// our maximum memory usage. Typically one thread is sufficient here, but for higher end devices
// with more cores, two threads can provide better performance if lots of GIFs are showing at
// once.
int maximumPoolSize = bestThreadCount >= 4 ? 2 : 1;
return new GlideExecutor(
/*corePoolSize=*/ 0,
maximumPoolSize,
KEEP_ALIVE_TIME_MS,
ANIMATION_EXECUTOR_NAME,
UncaughtThrowableStrategy.DEFAULT,
/*preventNetworkOperations=*/ true,
/*executeSynchronously=*/ false,
new PriorityBlockingQueue<Runnable>());
}

// Visible for testing.
GlideExecutor(int poolSize, String name,
UncaughtThrowableStrategy uncaughtThrowableStrategy, boolean preventNetworkOperations,
Expand Down Expand Up @@ -280,31 +303,35 @@ public <T> Future<T> submit(Callable<T> task) {
* http://goo.gl/8H670N.
*/
public static int calculateBestThreadCount() {
// We override the current ThreadPolicy to allow disk reads.
// This shouldn't actually do disk-IO and accesses a device file.
// See: https://github.com/bumptech/glide/issues/1170
ThreadPolicy originalPolicy = StrictMode.allowThreadDiskReads();
File[] cpus = null;
try {
File cpuInfo = new File(CPU_LOCATION);
final Pattern cpuNamePattern = Pattern.compile(CPU_NAME_REGEX);
cpus = cpuInfo.listFiles(new FilenameFilter() {
@Override
public boolean accept(File file, String s) {
return cpuNamePattern.matcher(s).matches();
if (bestThreadCount == 0) {
// We override the current ThreadPolicy to allow disk reads.
// This shouldn't actually do disk-IO and accesses a device file.
// See: https://github.com/bumptech/glide/issues/1170
ThreadPolicy originalPolicy = StrictMode.allowThreadDiskReads();
File[] cpus = null;
try {
File cpuInfo = new File(CPU_LOCATION);
final Pattern cpuNamePattern = Pattern.compile(CPU_NAME_REGEX);
cpus = cpuInfo.listFiles(new FilenameFilter() {
@Override
public boolean accept(File file, String s) {
return cpuNamePattern.matcher(s).matches();
}
});
} catch (Throwable t) {
if (Log.isLoggable(TAG, Log.ERROR)) {
Log.e(TAG, "Failed to calculate accurate cpu count", t);
}
});
} catch (Throwable t) {
if (Log.isLoggable(TAG, Log.ERROR)) {
Log.e(TAG, "Failed to calculate accurate cpu count", t);
} finally {
StrictMode.setThreadPolicy(originalPolicy);
}
} finally {
StrictMode.setThreadPolicy(originalPolicy);
}

int cpuCount = cpus != null ? cpus.length : 0;
int availableProcessors = Math.max(1, Runtime.getRuntime().availableProcessors());
return Math.min(MAXIMUM_AUTOMATIC_THREAD_COUNT, Math.max(availableProcessors, cpuCount));
int cpuCount = cpus != null ? cpus.length : 0;
int availableProcessors = Math.max(1, Runtime.getRuntime().availableProcessors());
bestThreadCount =
Math.min(MAXIMUM_AUTOMATIC_THREAD_COUNT, Math.max(availableProcessors, cpuCount));
}
return bestThreadCount;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ private static int getSampleSize(GifHeader gifHeader, int targetWidth, int targe
// Although functionally equivalent to 0 for BitmapFactory, 1 is a safer default for our code
// than 0.
int sampleSize = Math.max(1, powerOfTwoSampleSize);
if (Log.isLoggable(TAG, Log.VERBOSE)) {
if (Log.isLoggable(TAG, Log.VERBOSE) && sampleSize > 1) {
Log.v(TAG, "Downsampling GIF"
+ ", sampleSize: " + sampleSize
+ ", target dimens: [" + targetWidth + "x" + targetHeight + "]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ private void loadNextFrame() {

gifDecoder.advance();
next = new DelayTarget(handler, gifDecoder.getCurrentFrameIndex(), targetTime);
requestBuilder.clone().apply(signatureOf(getFrameSignature())).load(gifDecoder).into(next);
requestBuilder.apply(signatureOf(getFrameSignature())).load(gifDecoder).into(next);
}

private void recycleFirstFrame() {
Expand Down Expand Up @@ -298,11 +298,12 @@ private static RequestBuilder<Bitmap> getRequestBuilder(
.asBitmap()
.apply(
diskCacheStrategyOf(DiskCacheStrategy.NONE)
.useAnimationPool(true)
.skipMemoryCache(true)
.override(width, height));
}

static Key getFrameSignature() {
private static Key getFrameSignature() {
// Some devices seem to have crypto bugs that throw exceptions when you create a new UUID.
// See #1510.
return new ObjectKey(Math.random());
Expand Down
Loading

0 comments on commit c4db04e

Please sign in to comment.