Skip to content

Commit

Permalink
Allow starting requests on background threads without posting to the …
Browse files Browse the repository at this point in the history 
…main thread.

-------------
Avoid posting to the main thread when clearing in RequestFutureTarget.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=221803963
  • Loading branch information
@sjudd
sjudd committed Dec 19, 2018
1 parent 617edc3 commit 8f1ea5c
Show file tree
Hide file tree
Showing 29 changed files with 1,208 additions and 844 deletions.
6 changes: 3 additions & 3 deletions annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideRequests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,19 +48,19 @@ public <ResourceType> GlideRequest<ResourceType> as(@NonNull Class<ResourceType>

@Override
@NonNull
public GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
public synchronized GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
return (GlideRequests) super.applyDefaultRequestOptions(options);
}

@Override
@NonNull
public GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
public synchronized GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
return (GlideRequests) super.setDefaultRequestOptions(options);
}

@Override
@NonNull
public GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
public synchronized GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
return (GlideRequests) super.addDefaultRequestListener(listener);
}

Expand Down
6 changes: 3 additions & 3 deletions annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,19 @@ public GlideRequest<Number> asNumber() {

@Override
@NonNull
public GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
public synchronized GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
return (GlideRequests) super.applyDefaultRequestOptions(options);
}

@Override
@NonNull
public GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
public synchronized GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
return (GlideRequests) super.setDefaultRequestOptions(options);
}

@Override
@NonNull
public GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
public synchronized GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
return (GlideRequests) super.addDefaultRequestListener(listener);
}

Expand Down
84 changes: 56 additions & 28 deletions instrumentation/src/androidTest/java/com/bumptech/glide/CachingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import com.bumptech.glide.load.engine.DiskCacheStrategy;
import com.bumptech.glide.load.engine.cache.LruResourceCache;
import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
import com.bumptech.glide.load.engine.executor.GlideExecutor;
import com.bumptech.glide.load.engine.executor.MockGlideExecutor;
import com.bumptech.glide.request.FutureTarget;
import com.bumptech.glide.request.RequestListener;
import com.bumptech.glide.request.target.Target;
Expand Down Expand Up @@ -67,12 +69,11 @@ public class CachingTest {
private Context context;

@Before
public void setUp() throws InterruptedException {
public void setUp() {
MockitoAnnotations.initMocks(this);
context = InstrumentationRegistry.getTargetContext();

Glide.init(
context, new GlideBuilder().setMemoryCache(new LruResourceCache(CACHE_SIZE_BYTES)));
Glide.init(context, new GlideBuilder().setMemoryCache(new LruResourceCache(CACHE_SIZE_BYTES)));
}

@Test
Expand Down Expand Up @@ -164,8 +165,18 @@ public void run() {
}

@Test
public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCache()
throws InterruptedException, ExecutionException, TimeoutException {
public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCache() {
// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
// by making our clear and EngineJob's clear run on the same thread.
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
Glide.init(
context,
new GlideBuilder()
.setSourceExecutor(mainThreadExecutor)
.setDiskCacheExecutor(mainThreadExecutor)
.setAnimationExecutor(mainThreadExecutor));

FutureTarget<Drawable> future = GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.diskCacheStrategy(DiskCacheStrategy.DATA)
Expand All @@ -192,8 +203,7 @@ public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCac
}

@Test
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFromMemoryCache()
throws InterruptedException, TimeoutException, ExecutionException {
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFromMemoryCache() {
// We can't allow any mocks (RequestListner, Target etc) to reference this request or the test
// will fail due to the transient strong reference to the request.
concurrency.get(
Expand Down Expand Up @@ -224,8 +234,7 @@ public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFr
}

@Test
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecycleBitmap()
throws InterruptedException, TimeoutException, ExecutionException {
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecycleBitmap() {
// We can't allow any mocks (RequestListener, Target etc) to reference this request or the test
// will fail due to the transient strong reference to the request.
Bitmap bitmap =
Expand Down Expand Up @@ -255,34 +264,51 @@ public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecy
}

@Test
public void clearDiskCache_doesNotPreventFutureLoads()
throws ExecutionException, InterruptedException, TimeoutException {
FutureTarget<Drawable> future = GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.diskCacheStrategy(DiskCacheStrategy.DATA)
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
public void clearDiskCache_doesNotPreventFutureLoads() {
// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
// by making our clear and EngineJob's clear run on the same thread.
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
Glide.init(
context,
new GlideBuilder()
.setSourceExecutor(mainThreadExecutor)
.setDiskCacheExecutor(mainThreadExecutor)
.setAnimationExecutor(mainThreadExecutor));

// Load the request once.
FutureTarget<Drawable> future =
GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.diskCacheStrategy(DiskCacheStrategy.DATA)
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
concurrency.get(future);
// Clear the result from all of our caches.
GlideApp.with(context).clear(future);

clearMemoryCacheOnMainThread();
GlideApp.get(context).clearDiskCache();

future = GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.diskCacheStrategy(DiskCacheStrategy.DATA)
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
// Load the request a second time into the disk cache.
future =
GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.diskCacheStrategy(DiskCacheStrategy.DATA)
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
concurrency.get(future);

// Clear the second request from everywhere but the disk cache.
GlideApp.with(context).clear(future);
clearMemoryCacheOnMainThread();

// Load the request a third time.
concurrency.get(
GlideApp.with(context)
.load(ResourceIds.raw.canonical)
.listener(requestListener)
.diskCacheStrategy(DiskCacheStrategy.DATA)
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS));

// Assert that the third request comes from the disk cache (which was populated by the second
// request).
verify(requestListener)
.onResourceReady(
anyDrawable(),
Expand Down Expand Up @@ -351,12 +377,14 @@ public void run() {
// Verify that the request that didn't have retrieve from cache succeeds
assertThat(concurrency.get(expectedFuture)).isNotNull();
// The first request only from cache should fail because the item is not in cache.
assertThrows(RuntimeException.class, new ThrowingRunnable() {
@Override
public void run() throws Throwable {
concurrency.get(firstQueuedFuture);
}
});
assertThrows(
RuntimeException.class,
new ThrowingRunnable() {
@Override
public void run() {
concurrency.get(firstQueuedFuture);
}
});
}

@Test
Expand Down Expand Up @@ -482,7 +510,7 @@ public void loadIntoView_withSkipMemoryCache_doesNotLoadFromMemoryCacheIfPresent
anyBoolean());
}

private void clearMemoryCacheOnMainThread() throws InterruptedException {
private void clearMemoryCacheOnMainThread() {
concurrency.runOnMainThread(new Runnable() {
@Override
public void run() {
Expand Down
102 changes: 69 additions & 33 deletions instrumentation/src/androidTest/java/com/bumptech/glide/LoadBitmapTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import com.bumptech.glide.load.engine.bitmap_recycle.LruBitmapPool;
import com.bumptech.glide.load.engine.cache.LruResourceCache;
import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
import com.bumptech.glide.load.engine.executor.GlideExecutor;
import com.bumptech.glide.load.engine.executor.MockGlideExecutor;
import com.bumptech.glide.request.RequestListener;
import com.bumptech.glide.request.target.Target;
import com.bumptech.glide.test.ConcurrencyHelper;
Expand All @@ -45,18 +47,31 @@ public class LoadBitmapTest {

private final ConcurrencyHelper concurrency = new ConcurrencyHelper();
private Context context;
private GlideBuilder glideBuilder;

@Before
public void setUp() {
MockitoAnnotations.initMocks(this);
context = InstrumentationRegistry.getTargetContext();

// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
// by making our clear and EngineJob's clear run on the same thread.
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
glideBuilder =
new GlideBuilder()
.setSourceExecutor(mainThreadExecutor)
.setDiskCacheExecutor(mainThreadExecutor)
.setAnimationExecutor(mainThreadExecutor);
}

@Test
public void clearFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Target<Drawable> target =
concurrency.wait(
Expand All @@ -74,9 +89,11 @@ public void clearFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBi

@Test
public void transformFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
concurrency.wait(
GlideApp.with(context)
Expand All @@ -90,9 +107,11 @@ public void transformFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecyc

@Test
public void clearFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Target<Drawable> target =
concurrency.wait(
Expand All @@ -109,9 +128,11 @@ public void clearFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {

@Test
public void transformFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
concurrency.wait(
GlideApp.with(context)
Expand All @@ -124,9 +145,11 @@ public void transformFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap()

@Test
public void clearFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Target<Bitmap> target =
concurrency.wait(
Expand All @@ -144,9 +167,11 @@ public void clearFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitm

@Test
public void transformFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
Glide.init(context, new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Glide.init(
context,
new GlideBuilder()
.setMemoryCache(new MemoryCacheAdapter())
.setBitmapPool(new BitmapPoolAdapter()));
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
concurrency.wait(
GlideApp.with(context)
Expand All @@ -161,9 +186,11 @@ public void transformFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycle
@Test
public void loadFromRequestManager_withBitmap_doesNotLoadFromDiskCache() {
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Glide.init(context, new GlideBuilder()
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Glide.init(
context,
glideBuilder
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Target<Drawable> target =
concurrency.wait(
GlideApp.with(context)
Expand Down Expand Up @@ -200,9 +227,11 @@ public void run() {
@Test
public void loadFromRequestBuilder_asDrawable_withBitmap_doesNotLoadFromDiskCache() {
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Glide.init(context, new GlideBuilder()
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Glide.init(
context,
glideBuilder
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Target<Drawable> target =
concurrency.wait(
GlideApp.with(context)
Expand Down Expand Up @@ -240,9 +269,11 @@ public void run() {
@Test
public void loadFromRequestBuilder_asDrawable_withBitmapAndStrategyBeforeLoad_notFromCache() {
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Glide.init(context, new GlideBuilder()
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Glide.init(
context,
glideBuilder
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Target<Drawable> target =
concurrency.wait(
GlideApp.with(context)
Expand Down Expand Up @@ -281,9 +312,11 @@ public void run() {
@Test
public void loadFromRequestBuilder_asBitmap_withBitmap_doesNotLoadFromDiskCache() {
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Glide.init(context, new GlideBuilder()
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Glide.init(
context,
glideBuilder
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Target<Bitmap> target =
concurrency.wait(
GlideApp.with(context)
Expand Down Expand Up @@ -322,9 +355,12 @@ public void run() {
@Test
public void loadFromRequestBuilder_asBitmap_withBitmapAndStrategyBeforeLoad_notFromCache() {
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
Glide.init(context, new GlideBuilder()
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
Glide.init(
context,
glideBuilder
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));

Target<Bitmap> target =
concurrency.wait(
GlideApp.with(context)
Expand Down
Loading

0 comments on commit 8f1ea5c

Please sign in to comment.