From f1e9c0ab81adb3d6ba25c79dbf807595dd44401f Mon Sep 17 00:00:00 2001 From: Fares Alhassen Date: Tue, 21 Jul 2026 05:47:21 -0700 Subject: [PATCH] Remove path name memoization from Glide disk cache. PiperOrigin-RevId: 951436844 --- .../engine/cache/DiskLruCacheWrapper.java | 29 +------ third_party/disklrucache/build.gradle.kts | 33 ++++---- .../glide/disklrucache/DiskLruCache.java | 77 ++----------------- .../glide/disklrucache/DiskLruCacheTest.java | 2 +- 4 files changed, 26 insertions(+), 115 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java index 01164d7722..fbabb0cb2a 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/cache/DiskLruCacheWrapper.java @@ -27,7 +27,6 @@ public class DiskLruCacheWrapper implements DiskCache { private final SafeKeyGenerator safeKeyGenerator; private final File directory; private final long maxSize; - private final boolean memoizePathNames; private final DiskCacheWriteLocker writeLocker = new DiskCacheWriteLocker(); private DiskLruCache diskLruCache; @@ -61,24 +60,7 @@ public static synchronized DiskCache get(File directory, long maxSize) { */ @SuppressWarnings("deprecation") public static DiskCache create(File directory, long maxSize) { - return new DiskLruCacheWrapper(directory, maxSize, /* memoizePathNames= */ true); - } - - /** - * Create a new DiskCache in the given directory with a specified max size and memoization - * behavior. - * - * @param directory The directory for the disk cache - * @param maxSize The max size for the disk cache - * @param memoizePathNames Whether to memoize path names - * @return The new disk cache with the given arguments - * @deprecated Disabling the memoization is an experimental setting that may be removed in a - * future version. - */ - @SuppressWarnings("deprecation") - @Deprecated - public static DiskCache create(File directory, long maxSize, boolean memoizePathNames) { - return new DiskLruCacheWrapper(directory, maxSize, memoizePathNames); + return new DiskLruCacheWrapper(directory, maxSize); } /** @@ -88,21 +70,14 @@ public static DiskCache create(File directory, long maxSize, boolean memoizePath // Deprecated public API. @SuppressWarnings({"WeakerAccess", "DeprecatedIsStillUsed"}) protected DiskLruCacheWrapper(File directory, long maxSize) { - this(directory, maxSize, /* memoizePathNames= */ true); - } - - protected DiskLruCacheWrapper(File directory, long maxSize, boolean memoizePathNames) { this.directory = directory; this.maxSize = maxSize; - this.memoizePathNames = memoizePathNames; this.safeKeyGenerator = new SafeKeyGenerator(); } private synchronized DiskLruCache getDiskCache() throws IOException { if (diskLruCache == null) { - diskLruCache = - DiskLruCache.experimentalOpen( - directory, APP_VERSION, VALUE_COUNT, maxSize, memoizePathNames); + diskLruCache = DiskLruCache.open(directory, APP_VERSION, VALUE_COUNT, maxSize); } return diskLruCache; } diff --git a/third_party/disklrucache/build.gradle.kts b/third_party/disklrucache/build.gradle.kts index adc66f04e9..547867a9bb 100644 --- a/third_party/disklrucache/build.gradle.kts +++ b/third_party/disklrucache/build.gradle.kts @@ -1,35 +1,32 @@ plugins { - id("com.android.library") - checkstyle + id("com.android.library") + checkstyle } checkstyle { - toolVersion = "6.19" - configFile = file("checkstyle.xml") + toolVersion = "6.19" + configFile = file("checkstyle.xml") } android { - namespace = "com.bumptech.glide.disklrucache" - compileSdk = libs.versions.compile.sdk.version.get().toInt() + namespace = "com.bumptech.glide.disklrucache" + compileSdk = libs.versions.compile.sdk.version.get().toInt() - defaultConfig { - minSdk = libs.versions.min.sdk.version.get().toInt() - } + defaultConfig { minSdk = libs.versions.min.sdk.version.get().toInt() } - compileOptions { - sourceCompatibility = JavaVersion.VERSION_1_8 - targetCompatibility = JavaVersion.VERSION_1_8 - } + compileOptions { + sourceCompatibility = JavaVersion.VERSION_1_8 + targetCompatibility = JavaVersion.VERSION_1_8 + } } dependencies { - implementation(libs.androidx.annotation) - testImplementation(libs.junit) - testImplementation(libs.truth) + testImplementation(libs.junit) + testImplementation(libs.truth) } val uploaderScript = "${rootProject.projectDir}/scripts/upload.gradle.kts" if (file(uploaderScript).exists()) { - apply(from = "${rootProject.projectDir}/scripts/upload.gradle.kts") -} \ No newline at end of file + apply(from = "${rootProject.projectDir}/scripts/upload.gradle.kts") +} diff --git a/third_party/disklrucache/src/main/java/com/bumptech/glide/disklrucache/DiskLruCache.java b/third_party/disklrucache/src/main/java/com/bumptech/glide/disklrucache/DiskLruCache.java index 29a724c2a4..183dfb6520 100644 --- a/third_party/disklrucache/src/main/java/com/bumptech/glide/disklrucache/DiskLruCache.java +++ b/third_party/disklrucache/src/main/java/com/bumptech/glide/disklrucache/DiskLruCache.java @@ -20,7 +20,6 @@ import android.os.Build.VERSION; import android.os.Build.VERSION_CODES; import android.os.StrictMode; -import androidx.annotation.Nullable; import java.io.BufferedWriter; import java.io.Closeable; import java.io.EOFException; @@ -147,7 +146,6 @@ public final class DiskLruCache implements Closeable { private final int appVersion; private long maxSize; private final int valueCount; - private final boolean memoizePathNames; private long size = 0; private Writer journalWriter; private final LinkedHashMap lruEntries = @@ -181,8 +179,7 @@ public Void call() throws Exception { } }; - private DiskLruCache( - File directory, int appVersion, int valueCount, long maxSize, boolean memoizePathNames) { + private DiskLruCache(File directory, int appVersion, int valueCount, long maxSize) { this.directory = directory; this.appVersion = appVersion; this.journalFile = new File(directory, JOURNAL_FILE); @@ -190,7 +187,6 @@ private DiskLruCache( this.journalFileBackup = new File(directory, JOURNAL_FILE_BACKUP); this.valueCount = valueCount; this.maxSize = maxSize; - this.memoizePathNames = memoizePathNames; } /** @@ -204,27 +200,6 @@ private DiskLruCache( */ public static DiskLruCache open(File directory, int appVersion, int valueCount, long maxSize) throws IOException { - return experimentalOpen( - directory, appVersion, valueCount, maxSize, /* memoizePathNames= */ true); - } - - /** - * Opens the cache in {@code directory}, creating a cache if none exists there, with explicit - * control over path name memoization. - * - *

Disabling the memoization is an experimental setting that may be removed in a future - * version. - * - * @param directory a writable directory - * @param appVersion the application's current version code - * @param valueCount the number of values per cache entry. Must be positive. - * @param maxSize the maximum number of bytes this cache should use to store - * @param memoizePathNames whether to memoize path names - * @return The new disk cache with the given arguments - */ - public static DiskLruCache experimentalOpen( - File directory, int appVersion, int valueCount, long maxSize, boolean memoizePathNames) - throws IOException { if (maxSize <= 0) { throw new IllegalArgumentException("maxSize <= 0"); } @@ -245,8 +220,7 @@ public static DiskLruCache experimentalOpen( } // Prefer to pick up where we left off. - DiskLruCache cache = - new DiskLruCache(directory, appVersion, valueCount, maxSize, memoizePathNames); + DiskLruCache cache = new DiskLruCache(directory, appVersion, valueCount, maxSize); if (cache.journalFile.exists()) { try { cache.readJournal(); @@ -265,7 +239,7 @@ public static DiskLruCache experimentalOpen( // Create a new empty cache. directory.mkdirs(); - cache = new DiskLruCache(directory, appVersion, valueCount, maxSize, memoizePathNames); + cache = new DiskLruCache(directory, appVersion, valueCount, maxSize); cache.rebuildJournal(); return cache; } @@ -463,7 +437,7 @@ public synchronized Value get(String key) throws IOException { executorService.submit(cleanupCallable); } - return new Value(key, entry.sequenceNumber, entry.cleanFiles, entry.lengths); + return new Value(key, entry.sequenceNumber, entry.lengths); } /** @@ -748,12 +722,10 @@ public final class Value { private final String key; private final long sequenceNumber; private final long[] lengths; - private final File[] files; - private Value(String key, long sequenceNumber, File[] files, long[] lengths) { + private Value(String key, long sequenceNumber, long[] lengths) { this.key = key; this.sequenceNumber = sequenceNumber; - this.files = files; this.lengths = lengths; } @@ -767,9 +739,6 @@ public Editor edit() throws IOException { } public File getFile(int index) { - if (files != null) { - return files[index]; - } return new File(directory, key + "." + index); } @@ -888,15 +857,6 @@ private final class Entry { /** Lengths of this entry's files. */ private final long[] lengths; - /** - * Memoized File objects for this entry to avoid char[] allocations. - * - *

If the disk cache is configured to not memoize path names, these will be null. Otherwise, - * they're always non-null, with one entry per value. - */ - @Nullable File[] cleanFiles; - @Nullable File[] dirtyFiles; - /** True if this entry has ever been published. */ private boolean readable; @@ -907,26 +867,11 @@ private final class Entry { private long sequenceNumber; private Entry(String key) { + if (key == null) { + throw new NullPointerException("key == null"); + } this.key = key; this.lengths = new long[valueCount]; - - if (!memoizePathNames) { - return; - } - - cleanFiles = new File[valueCount]; - dirtyFiles = new File[valueCount]; - - // The names are repetitive so re-use the same builder to avoid allocations. - StringBuilder fileBuilder = new StringBuilder(key).append('.'); - int truncateTo = fileBuilder.length(); - for (int i = 0; i < valueCount; i++) { - fileBuilder.append(i); - cleanFiles[i] = new File(directory, fileBuilder.toString()); - fileBuilder.append(".tmp"); - dirtyFiles[i] = new File(directory, fileBuilder.toString()); - fileBuilder.setLength(truncateTo); - } } public String getLengths() throws IOException { @@ -957,16 +902,10 @@ private IOException invalidLengths(String[] strings) throws IOException { } public File getCleanFile(int i) { - if (cleanFiles != null) { - return cleanFiles[i]; - } return new File(directory, key + "." + i); } public File getDirtyFile(int i) { - if (dirtyFiles != null) { - return dirtyFiles[i]; - } return new File(directory, key + "." + i + ".tmp"); } } diff --git a/third_party/disklrucache/src/test/java/com/bumptech/glide/disklrucache/DiskLruCacheTest.java b/third_party/disklrucache/src/test/java/com/bumptech/glide/disklrucache/DiskLruCacheTest.java index f3db6e2b5e..12eb938efc 100644 --- a/third_party/disklrucache/src/test/java/com/bumptech/glide/disklrucache/DiskLruCacheTest.java +++ b/third_party/disklrucache/src/test/java/com/bumptech/glide/disklrucache/DiskLruCacheTest.java @@ -524,7 +524,7 @@ public static void setUpClass() { @Test public void readingTheSameFileMultipleTimes() throws Exception { set("a", "a", "b"); DiskLruCache.Value value = cache.get("a"); - assertThat(value.getFile(0)).isSameInstanceAs(value.getFile(0)); + assertThat(value.getFile(0)).isEqualTo(value.getFile(0)); } @Test public void rebuildJournalOnRepeatedReads() throws Exception {