From 6ba4b548cae12f3aa53634a59025069834c0ea14 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Fri, 18 Feb 2022 17:14:27 -0800 Subject: [PATCH] Move Glide Registry initialization to a background thread. By making the Registry lazy, we defer initialization until the first time it's used, which will be during the first request to load an image. Since image loads only happen on Glide's background threads, this effectively moves initialization from the UI thread to a background thread. PiperOrigin-RevId: 429678596 --- .../main/java/com/bumptech/glide/Glide.java | 52 ++++++------ .../java/com/bumptech/glide/GlideBuilder.java | 11 ++- .../java/com/bumptech/glide/GlideContext.java | 11 ++- .../com/bumptech/glide/RegistryFactory.java | 82 ++++++++++++++++++- .../com/bumptech/glide/GlideContextTest.java | 8 +- .../java/com/bumptech/glide/GlideTest.java | 27 +++--- 6 files changed, 145 insertions(+), 46 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java index 66a0b0ac69..9270b22387 100644 --- a/library/src/main/java/com/bumptech/glide/Glide.java +++ b/library/src/main/java/com/bumptech/glide/Glide.java @@ -14,6 +14,7 @@ import androidx.annotation.VisibleForTesting; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentActivity; +import com.bumptech.glide.GlideBuilder.EnableLazyGlideRegistry; import com.bumptech.glide.load.DecodeFormat; import com.bumptech.glide.load.engine.Engine; import com.bumptech.glide.load.engine.bitmap_recycle.ArrayPool; @@ -26,11 +27,14 @@ import com.bumptech.glide.load.resource.bitmap.HardwareConfigState; import com.bumptech.glide.manager.ConnectivityMonitorFactory; import com.bumptech.glide.manager.RequestManagerRetriever; +import com.bumptech.glide.module.AppGlideModule; +import com.bumptech.glide.module.GlideModule; import com.bumptech.glide.module.ManifestParser; import com.bumptech.glide.request.RequestListener; import com.bumptech.glide.request.RequestOptions; import com.bumptech.glide.request.target.ImageViewTargetFactory; import com.bumptech.glide.request.target.Target; +import com.bumptech.glide.util.GlideSuppliers.GlideSupplier; import com.bumptech.glide.util.Preconditions; import com.bumptech.glide.util.Util; import java.io.File; @@ -210,7 +214,7 @@ private static void initializeGlide( @NonNull GlideBuilder builder, @Nullable GeneratedAppGlideModule annotationGeneratedModule) { Context applicationContext = context.getApplicationContext(); - List manifestModules = Collections.emptyList(); + List manifestModules = Collections.emptyList(); if (annotationGeneratedModule == null || annotationGeneratedModule.isManifestParsingEnabled()) { manifestModules = new ManifestParser(applicationContext).parse(); } @@ -218,9 +222,9 @@ private static void initializeGlide( if (annotationGeneratedModule != null && !annotationGeneratedModule.getExcludedModuleClasses().isEmpty()) { Set> excludedModuleClasses = annotationGeneratedModule.getExcludedModuleClasses(); - Iterator iterator = manifestModules.iterator(); + Iterator iterator = manifestModules.iterator(); while (iterator.hasNext()) { - com.bumptech.glide.module.GlideModule current = iterator.next(); + GlideModule current = iterator.next(); if (!excludedModuleClasses.contains(current.getClass())) { continue; } @@ -232,7 +236,7 @@ private static void initializeGlide( } if (Log.isLoggable(TAG, Log.DEBUG)) { - for (com.bumptech.glide.module.GlideModule glideModule : manifestModules) { + for (GlideModule glideModule : manifestModules) { Log.d(TAG, "Discovered GlideModule from manifest: " + glideModule.getClass()); } } @@ -242,31 +246,20 @@ private static void initializeGlide( ? annotationGeneratedModule.getRequestManagerFactory() : null; builder.setRequestManagerFactory(factory); - for (com.bumptech.glide.module.GlideModule module : manifestModules) { + for (GlideModule module : manifestModules) { module.applyOptions(applicationContext, builder); } if (annotationGeneratedModule != null) { annotationGeneratedModule.applyOptions(applicationContext, builder); } - Glide glide = builder.build(applicationContext); - for (com.bumptech.glide.module.GlideModule module : manifestModules) { - try { - module.registerComponents(applicationContext, glide, glide.glideContext.getRegistry()); - } catch (AbstractMethodError e) { - throw new IllegalStateException( - "Attempting to register a Glide v3 module. If you see this, you or one of your" - + " dependencies may be including Glide v3 even though you're using Glide v4." - + " You'll need to find and remove (or update) the offending dependency." - + " The v3 module name is: " - + module.getClass().getName(), - e); - } - } - if (annotationGeneratedModule != null) { - annotationGeneratedModule.registerComponents( - applicationContext, glide, glide.glideContext.getRegistry()); - } + Glide glide = builder.build(applicationContext, manifestModules, annotationGeneratedModule); applicationContext.registerComponentCallbacks(glide); + + // Trigger the registry initialization eagerly, similar to the codepath prior to the experiment. + if (!glide.getGlideContext().getExperiments().isEnabled(EnableLazyGlideRegistry.class)) { + glide.getGlideContext().getRegistry(); + } + Glide.glide = glide; } @@ -323,7 +316,9 @@ private static void throwIncorrectGlideModule(Exception e) { @NonNull RequestOptionsFactory defaultRequestOptionsFactory, @NonNull Map, TransitionOptions> defaultTransitionOptions, @NonNull List> defaultRequestListeners, - GlideExperiments experiments) { + @NonNull List manifestModules, + @Nullable AppGlideModule annotationGeneratedModule, + @NonNull GlideExperiments experiments) { this.engine = engine; this.bitmapPool = bitmapPool; this.arrayPool = arrayPool; @@ -332,9 +327,12 @@ private static void throwIncorrectGlideModule(Exception e) { this.connectivityMonitorFactory = connectivityMonitorFactory; this.defaultRequestOptionsFactory = defaultRequestOptionsFactory; - Registry registry = - RegistryFactory.createRegistryAndInitializeDefaults( - context, bitmapPool, arrayPool, experiments); + // This has a circular relationship with Glide and GlideContext in that it depends on both, + // but it's created by Glide's constructor. In practice this shouldn't matter because the + // supplier holding the registry should never be initialized before this constructor finishes. + GlideSupplier registry = + RegistryFactory.lazilyCreateAndInitializeRegistry( + this, manifestModules, annotationGeneratedModule); ImageViewTargetFactory imageViewTargetFactory = new ImageViewTargetFactory(); glideContext = diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java index 0e199394ec..9e776de836 100644 --- a/library/src/main/java/com/bumptech/glide/GlideBuilder.java +++ b/library/src/main/java/com/bumptech/glide/GlideBuilder.java @@ -27,6 +27,8 @@ import com.bumptech.glide.manager.DefaultConnectivityMonitorFactory; import com.bumptech.glide.manager.RequestManagerRetriever; import com.bumptech.glide.manager.RequestManagerRetriever.RequestManagerFactory; +import com.bumptech.glide.module.AppGlideModule; +import com.bumptech.glide.module.GlideModule; import com.bumptech.glide.request.BaseRequestOptions; import com.bumptech.glide.request.RequestListener; import com.bumptech.glide.request.RequestOptions; @@ -504,7 +506,10 @@ GlideBuilder setEngine(Engine engine) { } @NonNull - Glide build(@NonNull Context context) { + Glide build( + @NonNull Context context, + List manifestModules, + AppGlideModule annotationGeneratedGlideModule) { if (sourceExecutor == null) { sourceExecutor = GlideExecutor.newSourceExecutor(); } @@ -580,6 +585,8 @@ Glide build(@NonNull Context context) { defaultRequestOptionsFactory, defaultTransitionOptions, defaultRequestListeners, + manifestModules, + annotationGeneratedGlideModule, experiments); } @@ -603,4 +610,6 @@ static final class EnableImageDecoderForAnimatedWebp implements Experiment {} /** See {@link #setLogRequestOrigins(boolean)}. */ public static final class LogRequestOrigins implements Experiment {} + + static final class EnableLazyGlideRegistry implements Experiment {} } diff --git a/library/src/main/java/com/bumptech/glide/GlideContext.java b/library/src/main/java/com/bumptech/glide/GlideContext.java index f312830b18..7be48425c3 100644 --- a/library/src/main/java/com/bumptech/glide/GlideContext.java +++ b/library/src/main/java/com/bumptech/glide/GlideContext.java @@ -14,6 +14,8 @@ import com.bumptech.glide.request.RequestOptions; import com.bumptech.glide.request.target.ImageViewTargetFactory; import com.bumptech.glide.request.target.ViewTarget; +import com.bumptech.glide.util.GlideSuppliers; +import com.bumptech.glide.util.GlideSuppliers.GlideSupplier; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -29,7 +31,7 @@ public class GlideContext extends ContextWrapper { new GenericTransitionOptions<>(); private final ArrayPool arrayPool; - private final Registry registry; + private final GlideSupplier registry; private final ImageViewTargetFactory imageViewTargetFactory; private final RequestOptionsFactory defaultRequestOptionsFactory; private final List> defaultRequestListeners; @@ -45,7 +47,7 @@ public class GlideContext extends ContextWrapper { public GlideContext( @NonNull Context context, @NonNull ArrayPool arrayPool, - @NonNull Registry registry, + @NonNull GlideSupplier registry, @NonNull ImageViewTargetFactory imageViewTargetFactory, @NonNull RequestOptionsFactory defaultRequestOptionsFactory, @NonNull Map, TransitionOptions> defaultTransitionOptions, @@ -55,7 +57,6 @@ public GlideContext( int logLevel) { super(context.getApplicationContext()); this.arrayPool = arrayPool; - this.registry = registry; this.imageViewTargetFactory = imageViewTargetFactory; this.defaultRequestOptionsFactory = defaultRequestOptionsFactory; this.defaultRequestListeners = defaultRequestListeners; @@ -63,6 +64,8 @@ public GlideContext( this.engine = engine; this.experiments = experiments; this.logLevel = logLevel; + + this.registry = GlideSuppliers.memorize(registry); } public List> getDefaultRequestListeners() { @@ -107,7 +110,7 @@ public Engine getEngine() { @NonNull public Registry getRegistry() { - return registry; + return registry.get(); } public int getLogLevel() { diff --git a/library/src/main/java/com/bumptech/glide/RegistryFactory.java b/library/src/main/java/com/bumptech/glide/RegistryFactory.java index b30af0e7db..da7fe43a4e 100644 --- a/library/src/main/java/com/bumptech/glide/RegistryFactory.java +++ b/library/src/main/java/com/bumptech/glide/RegistryFactory.java @@ -10,6 +10,8 @@ import android.net.Uri; import android.os.Build; import android.os.ParcelFileDescriptor; +import androidx.annotation.Nullable; +import androidx.tracing.Trace; import com.bumptech.glide.GlideBuilder.EnableImageDecoderForAnimatedWebp; import com.bumptech.glide.GlideBuilder.EnableImageDecoderForBitmaps; import com.bumptech.glide.gifdecoder.GifDecoder; @@ -66,6 +68,10 @@ import com.bumptech.glide.load.resource.transcode.BitmapDrawableTranscoder; import com.bumptech.glide.load.resource.transcode.DrawableBytesTranscoder; import com.bumptech.glide.load.resource.transcode.GifDrawableBytesTranscoder; +import com.bumptech.glide.module.AppGlideModule; +import com.bumptech.glide.module.GlideModule; +import com.bumptech.glide.util.GlideSuppliers.GlideSupplier; +import com.bumptech.glide.util.Synthetic; import java.io.File; import java.io.InputStream; import java.net.URL; @@ -76,9 +82,57 @@ final class RegistryFactory { private RegistryFactory() {} - static Registry createRegistryAndInitializeDefaults( - Context context, BitmapPool bitmapPool, ArrayPool arrayPool, GlideExperiments experiments) { + static GlideSupplier lazilyCreateAndInitializeRegistry( + final Glide glide, + final List manifestModules, + @Nullable final AppGlideModule annotationGeneratedModule) { + return new GlideSupplier() { + private boolean isInitializingOrInitialized; + + @Override + public Registry get() { + if (isInitializingOrInitialized) { + throw new IllegalStateException( + "Recursive Registry initialization! In your" + + " AppGlideModule and LibraryGlideModules, Make sure you're using the provided " + + "Registry rather calling glide.getRegistry()!"); + } + isInitializingOrInitialized = true; + + Trace.beginSection("Glide registry"); + try { + return createAndInitRegistry(glide, manifestModules, annotationGeneratedModule); + } finally { + Trace.endSection(); + } + } + }; + } + + @Synthetic + static Registry createAndInitRegistry( + Glide glide, + List manifestModules, + @Nullable AppGlideModule annotationGeneratedModule) { + + BitmapPool bitmapPool = glide.getBitmapPool(); + ArrayPool arrayPool = glide.getArrayPool(); + Context context = glide.getGlideContext().getApplicationContext(); + + GlideExperiments experiments = glide.getGlideContext().getExperiments(); + Registry registry = new Registry(); + initializeDefaults(context, registry, bitmapPool, arrayPool, experiments); + initializeModules(context, glide, registry, manifestModules, annotationGeneratedModule); + return registry; + } + + private static void initializeDefaults( + Context context, + Registry registry, + BitmapPool bitmapPool, + ArrayPool arrayPool, + GlideExperiments experiments) { registry.register(new DefaultImageHeaderParser()); // Right now we're only using this parser for HEIF images, which are only supported on OMR1+. // If we need this for other file types, we should consider removing this restriction. @@ -290,7 +344,29 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm BitmapDrawable.class, new BitmapDrawableDecoder<>(resources, byteBufferVideoDecoder)); } + } - return registry; + private static void initializeModules( + Context context, + Glide glide, + Registry registry, + List manifestModules, + @Nullable AppGlideModule annotationGeneratedModule) { + for (GlideModule module : manifestModules) { + try { + module.registerComponents(context, glide, registry); + } catch (AbstractMethodError e) { + throw new IllegalStateException( + "Attempting to register a Glide v3 module. If you see this, you or one of your" + + " dependencies may be including Glide v3 even though you're using Glide v4." + + " You'll need to find and remove (or update) the offending dependency." + + " The v3 module name is: " + + module.getClass().getName(), + e); + } + } + if (annotationGeneratedModule != null) { + annotationGeneratedModule.registerComponents(context, glide, registry); + } } } diff --git a/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java b/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java index 5ffcf5d951..70b30c1161 100644 --- a/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java +++ b/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java @@ -18,6 +18,7 @@ import com.bumptech.glide.request.RequestListener; import com.bumptech.glide.request.RequestOptions; import com.bumptech.glide.request.target.ImageViewTargetFactory; +import com.bumptech.glide.util.GlideSuppliers.GlideSupplier; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -40,7 +41,12 @@ public void setUp() { new GlideContext( app, new LruArrayPool(), - new Registry(), + new GlideSupplier() { + @Override + public Registry get() { + return new Registry(); + } + }, new ImageViewTargetFactory(), new RequestOptionsFactory() { @NonNull diff --git a/library/test/src/test/java/com/bumptech/glide/GlideTest.java b/library/test/src/test/java/com/bumptech/glide/GlideTest.java index c91ed98377..6d65e0e454 100644 --- a/library/test/src/test/java/com/bumptech/glide/GlideTest.java +++ b/library/test/src/test/java/com/bumptech/glide/GlideTest.java @@ -52,6 +52,7 @@ import com.bumptech.glide.load.resource.gif.GifDrawable; import com.bumptech.glide.manager.Lifecycle; import com.bumptech.glide.manager.RequestManagerTreeNode; +import com.bumptech.glide.module.GlideModule; import com.bumptech.glide.request.Request; import com.bumptech.glide.request.RequestListener; import com.bumptech.glide.request.RequestOptions; @@ -68,6 +69,7 @@ import java.net.MalformedURLException; import java.net.URL; import java.nio.ByteBuffer; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import org.junit.Before; @@ -171,8 +173,7 @@ public Boolean answer(InvocationOnMock invocation) { @Test public void testCanSetMemoryCategory() { MemoryCategory memoryCategory = MemoryCategory.NORMAL; - Glide glide = - new GlideBuilder().setBitmapPool(bitmapPool).setMemoryCache(memoryCache).build(context); + Glide glide = buildGlideWithFakePools(); glide.setMemoryCategory(memoryCategory); verify(memoryCache).setSizeMultiplier(eq(memoryCategory.getMultiplier())); @@ -182,8 +183,7 @@ public void testCanSetMemoryCategory() { @Test public void testCanIncreaseMemoryCategory() { MemoryCategory memoryCategory = MemoryCategory.NORMAL; - Glide glide = - new GlideBuilder().setBitmapPool(bitmapPool).setMemoryCache(memoryCache).build(context); + Glide glide = buildGlideWithFakePools(); glide.setMemoryCategory(memoryCategory); verify(memoryCache).setSizeMultiplier(eq(memoryCategory.getMultiplier())); @@ -201,8 +201,7 @@ public void testCanIncreaseMemoryCategory() { @Test public void testCanDecreaseMemoryCategory() { MemoryCategory memoryCategory = MemoryCategory.NORMAL; - Glide glide = - new GlideBuilder().setBitmapPool(bitmapPool).setMemoryCache(memoryCache).build(context); + Glide glide = buildGlideWithFakePools(); glide.setMemoryCategory(memoryCategory); verify(memoryCache).setSizeMultiplier(eq(memoryCategory.getMultiplier())); @@ -219,8 +218,7 @@ public void testCanDecreaseMemoryCategory() { @Test public void testClearMemory() { - Glide glide = - new GlideBuilder().setBitmapPool(bitmapPool).setMemoryCache(memoryCache).build(context); + Glide glide = buildGlideWithFakePools(); glide.clearMemory(); @@ -230,8 +228,7 @@ public void testClearMemory() { @Test public void testTrimMemory() { - Glide glide = - new GlideBuilder().setBitmapPool(bitmapPool).setMemoryCache(memoryCache).build(context); + Glide glide = buildGlideWithFakePools(); final int level = 123; @@ -241,6 +238,16 @@ public void testTrimMemory() { verify(memoryCache).trimMemory(eq(level)); } + private Glide buildGlideWithFakePools() { + return new GlideBuilder() + .setBitmapPool(bitmapPool) + .setMemoryCache(memoryCache) + .build( + context, + Collections.emptyList(), + /* annotationGeneratedGlideModule=*/ null); + } + @Test public void testFileDefaultLoaderWithInputStream() { registerFailFactory(File.class, ParcelFileDescriptor.class);