diff --git a/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java b/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java index bd70f82ffc..9d87a765b6 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/executor/GlideExecutor.java @@ -6,34 +6,35 @@ import android.support.annotation.VisibleForTesting; import android.util.Log; import com.bumptech.glide.util.Synthetic; -import java.io.File; -import java.io.FilenameFilter; -import java.util.concurrent.BlockingQueue; +import java.util.Collection; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.concurrent.PriorityBlockingQueue; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; -import java.util.regex.Pattern; +import java.util.concurrent.TimeoutException; /** * A prioritized {@link ThreadPoolExecutor} for running jobs in Glide. */ -public final class GlideExecutor extends ThreadPoolExecutor { - +public final class GlideExecutor implements ExecutorService { /** * The default thread name prefix for executors used to load/decode/transform data not found in * cache. */ public static final String DEFAULT_SOURCE_EXECUTOR_NAME = "source"; + /** * The default thread name prefix for executors used to load/decode/transform data found in * Glide's cache. */ public static final String DEFAULT_DISK_CACHE_EXECUTOR_NAME = "disk-cache"; + /** * The default thread count for executors used to load/decode/transform data found in Glide's * cache. @@ -41,26 +42,28 @@ public final class GlideExecutor extends ThreadPoolExecutor { public static final int DEFAULT_DISK_CACHE_EXECUTOR_THREADS = 1; private static final String TAG = "GlideExecutor"; - private static final String CPU_NAME_REGEX = "cpu[0-9]+"; - 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; /** * The default thread name prefix for executors from unlimited thread pool used to * load/decode/transform data not found in cache. */ private static final String SOURCE_UNLIMITED_EXECUTOR_NAME = "source-unlimited"; + + private static final String ANIMATION_EXECUTOR_NAME = "animation"; + /** * The default keep alive time for threads in our cached thread pools in milliseconds. */ private static final long KEEP_ALIVE_TIME_MS = TimeUnit.SECONDS.toMillis(10); - private static final String ANIMATION_EXECUTOR_NAME = "animation"; + // 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 ExecutorService delegate; /** * Returns a new fixed thread pool with the default thread count returned from @@ -72,8 +75,10 @@ public final class GlideExecutor extends ThreadPoolExecutor { *

Disk cache executors do not allow network operations on their threads. */ public static GlideExecutor newDiskCacheExecutor() { - return newDiskCacheExecutor(DEFAULT_DISK_CACHE_EXECUTOR_THREADS, - DEFAULT_DISK_CACHE_EXECUTOR_NAME, UncaughtThrowableStrategy.DEFAULT); + return newDiskCacheExecutor( + DEFAULT_DISK_CACHE_EXECUTOR_THREADS, + DEFAULT_DISK_CACHE_EXECUTOR_NAME, + UncaughtThrowableStrategy.DEFAULT); } /** @@ -90,8 +95,10 @@ public static GlideExecutor newDiskCacheExecutor() { */ public static GlideExecutor newDiskCacheExecutor( UncaughtThrowableStrategy uncaughtThrowableStrategy) { - return newDiskCacheExecutor(DEFAULT_DISK_CACHE_EXECUTOR_THREADS, - DEFAULT_DISK_CACHE_EXECUTOR_NAME, uncaughtThrowableStrategy); + return newDiskCacheExecutor( + DEFAULT_DISK_CACHE_EXECUTOR_THREADS, + DEFAULT_DISK_CACHE_EXECUTOR_NAME, + uncaughtThrowableStrategy); } /** @@ -108,8 +115,13 @@ public static GlideExecutor newDiskCacheExecutor( */ public static GlideExecutor newDiskCacheExecutor(int threadCount, String name, UncaughtThrowableStrategy uncaughtThrowableStrategy) { - return new GlideExecutor(threadCount, name, uncaughtThrowableStrategy, - true /*preventNetworkOperations*/, false /*executeSynchronously*/); + return new GlideExecutor(new ThreadPoolExecutor( + threadCount /* corePoolSize */, + threadCount /* maximumPoolSize */, + 0 /* keepAliveTime */, + TimeUnit.MILLISECONDS, + new PriorityBlockingQueue(), + new DefaultThreadFactory(name, uncaughtThrowableStrategy, true))); } /** @@ -122,7 +134,9 @@ public static GlideExecutor newDiskCacheExecutor(int threadCount, String name, *

Source executors allow network operations on their threads. */ public static GlideExecutor newSourceExecutor() { - return newSourceExecutor(calculateBestThreadCount(), DEFAULT_SOURCE_EXECUTOR_NAME, + return newSourceExecutor( + calculateBestThreadCount(), + DEFAULT_SOURCE_EXECUTOR_NAME, UncaughtThrowableStrategy.DEFAULT); } @@ -140,9 +154,11 @@ public static GlideExecutor newSourceExecutor() { * handle uncaught exceptions. */ public static GlideExecutor newSourceExecutor( - UncaughtThrowableStrategy uncaughtThrowableStrategy) { - return newSourceExecutor(DEFAULT_DISK_CACHE_EXECUTOR_THREADS, - DEFAULT_DISK_CACHE_EXECUTOR_NAME, uncaughtThrowableStrategy); + UncaughtThrowableStrategy uncaughtThrowableStrategy) { + return newSourceExecutor( + DEFAULT_DISK_CACHE_EXECUTOR_THREADS, + DEFAULT_DISK_CACHE_EXECUTOR_NAME, + uncaughtThrowableStrategy); } /** @@ -157,10 +173,15 @@ public static GlideExecutor newSourceExecutor( * com.bumptech.glide.load.engine.executor.GlideExecutor.UncaughtThrowableStrategy} to use to * handle uncaught exceptions. */ - public static GlideExecutor newSourceExecutor(int threadCount, String name, - UncaughtThrowableStrategy uncaughtThrowableStrategy) { - return new GlideExecutor(threadCount, name, uncaughtThrowableStrategy, - false /*preventNetworkOperations*/, false /*executeSynchronously*/); + public static GlideExecutor newSourceExecutor( + int threadCount, String name, UncaughtThrowableStrategy uncaughtThrowableStrategy) { + return new GlideExecutor(new ThreadPoolExecutor( + threadCount /* corePoolSize */, + threadCount /* maximumPoolSize */, + 0 /* keepAliveTime */, + TimeUnit.MILLISECONDS, + new PriorityBlockingQueue(), + new DefaultThreadFactory(name, uncaughtThrowableStrategy, false))); } /** @@ -178,14 +199,16 @@ public static GlideExecutor newSourceExecutor(int threadCount, String name, *

Source executors allow network operations on their threads. */ public static GlideExecutor newUnlimitedSourceExecutor() { - return new GlideExecutor(0 /* corePoolSize */, - Integer.MAX_VALUE /* maximumPoolSize */, + return new GlideExecutor(new ThreadPoolExecutor( + 0, + Integer.MAX_VALUE, KEEP_ALIVE_TIME_MS, - SOURCE_UNLIMITED_EXECUTOR_NAME, - UncaughtThrowableStrategy.DEFAULT, - false /*preventNetworkOperations*/, - false /*executeSynchronously*/, - new SynchronousQueue()); + TimeUnit.MILLISECONDS, + new SynchronousQueue(), + new DefaultThreadFactory( + SOURCE_UNLIMITED_EXECUTOR_NAME, + UncaughtThrowableStrategy.DEFAULT, + false))); } public static GlideExecutor newAnimationExecutor() { @@ -196,141 +219,116 @@ public static GlideExecutor newAnimationExecutor() { // 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, + return new GlideExecutor(new ThreadPoolExecutor( + 0 /* corePoolSize */, maximumPoolSize, KEEP_ALIVE_TIME_MS, - ANIMATION_EXECUTOR_NAME, - UncaughtThrowableStrategy.DEFAULT, - /*preventNetworkOperations=*/ true, - /*executeSynchronously=*/ false, - new PriorityBlockingQueue()); + TimeUnit.MILLISECONDS, + new PriorityBlockingQueue(), + new DefaultThreadFactory( + ANIMATION_EXECUTOR_NAME, + UncaughtThrowableStrategy.DEFAULT, + true))); } @VisibleForTesting - GlideExecutor(int poolSize, String name, - UncaughtThrowableStrategy uncaughtThrowableStrategy, boolean preventNetworkOperations, - boolean executeSynchronously) { - this( - poolSize /* corePoolSize */, - poolSize /* maximumPoolSize */, - 0 /* keepAliveTimeInMs */, - name, - uncaughtThrowableStrategy, - preventNetworkOperations, - executeSynchronously); + GlideExecutor(ExecutorService delegate) { + this.delegate = delegate; } - GlideExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTimeInMs, String name, - UncaughtThrowableStrategy uncaughtThrowableStrategy, boolean preventNetworkOperations, - boolean executeSynchronously) { - this( - corePoolSize, - maximumPoolSize, - keepAliveTimeInMs, - name, - uncaughtThrowableStrategy, - preventNetworkOperations, - executeSynchronously, - new PriorityBlockingQueue()); + @Override + public void execute(Runnable command) { + delegate.execute(command); } - GlideExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTimeInMs, String name, - UncaughtThrowableStrategy uncaughtThrowableStrategy, boolean preventNetworkOperations, - boolean executeSynchronously, BlockingQueue queue) { - super( - corePoolSize, - maximumPoolSize, - keepAliveTimeInMs, - TimeUnit.MILLISECONDS, - queue, - new DefaultThreadFactory(name, uncaughtThrowableStrategy, preventNetworkOperations)); - this.executeSynchronously = executeSynchronously; + @NonNull + @Override + public Future submit(Runnable task) { + return delegate.submit(task); } + @NonNull @Override - public void execute(Runnable command) { - if (executeSynchronously) { - command.run(); - } else { - super.execute(command); - } + public List> invokeAll(@NonNull Collection> tasks) + throws InterruptedException { + return delegate.invokeAll(tasks); } @NonNull @Override - public Future submit(Runnable task) { - return maybeWait(super.submit(task)); + public List> invokeAll( + @NonNull Collection> tasks, + long timeout, + @NonNull TimeUnit unit) throws InterruptedException { + return delegate.invokeAll(tasks, timeout, unit); } - private Future maybeWait(Future future) { - if (executeSynchronously) { - boolean interrupted = false; - try { - while (!future.isDone()) { - try { - future.get(); - } catch (ExecutionException e) { - throw new RuntimeException(e); - } catch (InterruptedException e) { - interrupted = true; - } - } - } finally { - if (interrupted) { - Thread.currentThread().interrupt(); - } - } - } - return future; + @NonNull + @Override + public T invokeAny(@NonNull Collection> tasks) + throws InterruptedException, ExecutionException { + return delegate.invokeAny(tasks); + } + + @Override + public T invokeAny( + @NonNull Collection> tasks, + long timeout, + @NonNull TimeUnit unit) throws InterruptedException, ExecutionException, TimeoutException { + return delegate.invokeAny(tasks, timeout, unit); } @NonNull @Override public Future submit(Runnable task, T result) { - return maybeWait(super.submit(task, result)); + return delegate.submit(task, result); } @Override public Future submit(Callable task) { - return maybeWait(super.submit(task)); + return delegate.submit(task); + } + + @Override + public void shutdown() { + delegate.shutdown(); + } + + @NonNull + @Override + public List shutdownNow() { + return delegate.shutdownNow(); + } + + @Override + public boolean isShutdown() { + return delegate.isShutdown(); + } + + @Override + public boolean isTerminated() { + return delegate.isTerminated(); + } + + @Override + public boolean awaitTermination(long timeout, @NonNull TimeUnit unit) + throws InterruptedException { + return delegate.awaitTermination(timeout, unit); + } + + @Override + public String toString() { + return delegate.toString(); } /** * Determines the number of cores available on the device. - * - *

{@link Runtime#availableProcessors()} returns the number of awake cores, which may not - * be the number of available cores depending on the device's current state. See - * http://goo.gl/8H670N. */ public static int calculateBestThreadCount() { 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); - } - } finally { - StrictMode.setThreadPolicy(originalPolicy); - } - - 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)); + bestThreadCount = Math.min( + MAXIMUM_AUTOMATIC_THREAD_COUNT, + RuntimeCompat.availableProcessors()); } return bestThreadCount; } @@ -383,6 +381,9 @@ public void handle(Throwable t) { * android.os.Process#THREAD_PRIORITY_BACKGROUND}. */ private static final class DefaultThreadFactory implements ThreadFactory { + private static final int DEFAULT_PRIORITY = android.os.Process.THREAD_PRIORITY_BACKGROUND + + android.os.Process.THREAD_PRIORITY_MORE_FAVORABLE; + private final String name; @Synthetic final UncaughtThrowableStrategy uncaughtThrowableStrategy; @Synthetic final boolean preventNetworkOperations; @@ -400,9 +401,7 @@ public synchronized Thread newThread(@NonNull Runnable runnable) { final Thread result = new Thread(runnable, "glide-" + name + "-thread-" + threadNum) { @Override public void run() { - android.os.Process.setThreadPriority( - android.os.Process.THREAD_PRIORITY_BACKGROUND - + android.os.Process.THREAD_PRIORITY_MORE_FAVORABLE); + android.os.Process.setThreadPriority(DEFAULT_PRIORITY); if (preventNetworkOperations) { StrictMode.setThreadPolicy( new ThreadPolicy.Builder() diff --git a/library/src/main/java/com/bumptech/glide/load/engine/executor/RuntimeCompat.java b/library/src/main/java/com/bumptech/glide/load/engine/executor/RuntimeCompat.java new file mode 100644 index 0000000000..bd9f0baf9d --- /dev/null +++ b/library/src/main/java/com/bumptech/glide/load/engine/executor/RuntimeCompat.java @@ -0,0 +1,65 @@ +package com.bumptech.glide.load.engine.executor; + +import android.os.Build; +import android.os.StrictMode; +import android.os.StrictMode.ThreadPolicy; +import android.util.Log; +import java.io.File; +import java.io.FilenameFilter; +import java.util.regex.Pattern; + +/** + * Compatibility methods for {@link java.lang.Runtime}. + */ +final class RuntimeCompat { + private static final String TAG = "GlideRuntimeCompat"; + private static final String CPU_NAME_REGEX = "cpu[0-9]+"; + private static final String CPU_LOCATION = "/sys/devices/system/cpu/"; + + private RuntimeCompat() {} + + /** + * Determines the number of cores available on the device. + */ + static int availableProcessors() { + int cpus = Runtime.getRuntime().availableProcessors(); + if (Build.VERSION.SDK_INT < 17) { + cpus = Math.max(getCoreCountPre17(), cpus); + } + return cpus; + } + + /** + * Determines the number of cores available on the device (pre-v17). + * + *

Before Jellybean, {@link Runtime#availableProcessors()} returned the number of awake cores, + * which may not be the number of available cores depending on the device's current state. See + * https://stackoverflow.com/a/30150409. + * + * @return the maximum number of processors available to the VM; never smaller than one + */ + private static int getCoreCountPre17() { + // 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 + File[] cpus = null; + ThreadPolicy originalPolicy = StrictMode.allowThreadDiskReads(); + 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); + } + } finally { + StrictMode.setThreadPolicy(originalPolicy); + } + return Math.max(1, cpus != null ? cpus.length : 0); + } +} diff --git a/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java b/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java index d9ed0b3f7d..24e39f8773 100644 --- a/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java +++ b/library/src/test/java/com/bumptech/glide/load/engine/EngineJobTest.java @@ -423,7 +423,7 @@ public void testSubmitsDecodeJobToSourceServiceOnSubmitForSource() { public void testSubmitsDecodeJobToDiskCacheServiceWhenDecodingFromCacheOnStart() { EngineJob job = harness.getJob(); when(harness.decodeJob.willDecodeFromCache()).thenReturn(true); - harness.diskCacheService.shutdownNow(); + harness.sourceService.shutdownNow(); job.start(harness.decodeJob); verify(harness.decodeJob).run(); diff --git a/library/src/test/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java b/library/src/test/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java index af0dce2206..a06b865797 100644 --- a/library/src/test/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java +++ b/library/src/test/java/com/bumptech/glide/load/engine/executor/MockGlideExecutor.java @@ -1,24 +1,109 @@ package com.bumptech.glide.load.engine.executor; +import android.os.StrictMode; +import android.support.annotation.VisibleForTesting; +import com.google.common.util.concurrent.ForwardingExecutorService; +import com.google.common.util.concurrent.MoreExecutors; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; + /** * Creates mock {@link GlideExecutor}s. */ +@VisibleForTesting public final class MockGlideExecutor { + // Utility class. + private MockGlideExecutor() {} - private MockGlideExecutor() { } + public static GlideExecutor newTestExecutor(ExecutorService executorService) { + return new GlideExecutor(executorService); + } public static GlideExecutor newMainThreadExecutor() { - return new GlideExecutor(1 /*poolSize*/, "mock-glide-executor", - GlideExecutor.UncaughtThrowableStrategy.THROW, false /*preventNetworkOperations*/, - true /*runAllOnMainThread*/); + return newTestExecutor(new DirectExecutorService()); } + /** + * @deprecated Use {@link #newMainThreadExecutor} instead. + */ + @Deprecated public static GlideExecutor newMainThreadUnlimitedExecutor() { - return new GlideExecutor(0 /* corePoolSize */, - Integer.MAX_VALUE /* maximumPoolSize */, - java.util.concurrent.TimeUnit.SECONDS.toMillis(10) /* keepAliveTimeInMs */, - "mock-unlimited-glide-executor", - GlideExecutor.UncaughtThrowableStrategy.THROW, false /*preventNetworkOperations*/, - true /*runAllOnMainThread*/); + return newMainThreadExecutor(); + } + + /** + * DirectExecutorService that enforces StrictMode and converts ExecutionExceptions into + * RuntimeExceptions. + */ + private static final class DirectExecutorService extends ForwardingExecutorService { + private static final StrictMode.ThreadPolicy THREAD_POLICY = + new StrictMode.ThreadPolicy.Builder() + .detectNetwork() + .penaltyDeath() + .build(); + + private final ExecutorService delegate; + + DirectExecutorService() { + delegate = MoreExecutors.newDirectExecutorService(); + } + + @Override + protected ExecutorService delegate() { + return delegate; + } + + @Override + public Future submit(Runnable task, T result) { + return getUninterruptibly(super.submit(task, result)); + } + + @Override + public Future submit(Callable task) { + return getUninterruptibly(super.submit(task)); + } + + @Override + public Future submit(Runnable task) { + return getUninterruptibly(super.submit(task)); + } + + @Override + public void execute(Runnable command) { + delegate.execute(new Runnable() { + @Override + public void run() { + StrictMode.ThreadPolicy oldPolicy = StrictMode.getThreadPolicy(); + StrictMode.setThreadPolicy(THREAD_POLICY); + try { + command.run(); + } finally { + StrictMode.setThreadPolicy(oldPolicy); + } + } + }); + } + + private Future getUninterruptibly(Future future) { + boolean interrupted = false; + try { + while (!future.isDone()) { + try { + future.get(); + } catch (ExecutionException e) { + throw new RuntimeException(e); + } catch (InterruptedException e) { + interrupted = true; + } + } + } finally { + if (interrupted) { + Thread.currentThread().interrupt(); + } + } + return future; + } } }