Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix incorrect cache size reporting after Bitmap is recycled. #1726

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 22 additions & 12 deletions picasso/src/main/java/com/squareup/picasso/LruCache.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

/** A memory cache which uses a least-recently used eviction policy. */
public class LruCache implements Cache {
final LinkedHashMap<String, Bitmap> map;
final LinkedHashMap<String, BitmapAndSize> map;
private final int maxSize;

private int size;
Expand Down Expand Up @@ -55,12 +55,12 @@ public LruCache(int maxSize) {
throw new NullPointerException("key == null");
}

Bitmap mapValue;
BitmapAndSize mapValue;
synchronized (this) {
mapValue = map.get(key);
if (mapValue != null) {
hitCount++;
return mapValue;
return mapValue.bitmap;
}
missCount++;
}
Expand All @@ -81,9 +81,9 @@ public LruCache(int maxSize) {
synchronized (this) {
putCount++;
size += addedSize;
Bitmap previous = map.put(key, bitmap);
BitmapAndSize previous = map.put(key, new BitmapAndSize(bitmap, addedSize));
if (previous != null) {
size -= Utils.getBitmapBytes(previous);
size -= previous.allocationSize;
}
}

Expand All @@ -93,7 +93,7 @@ public LruCache(int maxSize) {
private void trimToSize(int maxSize) {
while (true) {
String key;
Bitmap value;
BitmapAndSize value;
synchronized (this) {
if (size < 0 || (map.isEmpty() && size != 0)) {
throw new IllegalStateException(
Expand All @@ -104,11 +104,11 @@ private void trimToSize(int maxSize) {
break;
}

Map.Entry<String, Bitmap> toEvict = map.entrySet().iterator().next();
Map.Entry<String, BitmapAndSize> toEvict = map.entrySet().iterator().next();
key = toEvict.getKey();
value = toEvict.getValue();
map.remove(key);
size -= Utils.getBitmapBytes(value);
size -= value.allocationSize;
evictionCount++;
}
}
Expand All @@ -133,14 +133,14 @@ public final void evictAll() {

@Override public final synchronized void clearKeyUri(String uri) {
int uriLength = uri.length();
for (Iterator<Map.Entry<String, Bitmap>> i = map.entrySet().iterator(); i.hasNext();) {
Map.Entry<String, Bitmap> entry = i.next();
for (Iterator<Map.Entry<String, BitmapAndSize>> i = map.entrySet().iterator(); i.hasNext();) {
Map.Entry<String, BitmapAndSize> entry = i.next();
String key = entry.getKey();
Bitmap value = entry.getValue();
BitmapAndSize value = entry.getValue();
int newlineIndex = key.indexOf(KEY_SEPARATOR);
if (newlineIndex == uriLength && key.substring(0, newlineIndex).equals(uri)) {
i.remove();
size -= Utils.getBitmapBytes(value);
size -= value.allocationSize;
}
}
}
Expand All @@ -164,4 +164,14 @@ public final synchronized int putCount() {
public final synchronized int evictionCount() {
return evictionCount;
}

static final class BitmapAndSize {
final Bitmap bitmap;
final long allocationSize;

BitmapAndSize(Bitmap bitmap, long allocationSize) {
this.bitmap = bitmap;
this.allocationSize = allocationSize;
}
}
}
4 changes: 2 additions & 2 deletions picasso/src/test/java/com/squareup/picasso/LruCacheTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,9 @@ private void assertStatistics(LruCache cache) {

private void assertSnapshot(LruCache cache, Object... keysAndValues) {
List<Object> actualKeysAndValues = new ArrayList<>();
for (Map.Entry<String, Bitmap> entry : cache.map.entrySet()) {
for (Map.Entry<String, LruCache.BitmapAndSize> entry : cache.map.entrySet()) {
actualKeysAndValues.add(entry.getKey());
actualKeysAndValues.add(entry.getValue());
actualKeysAndValues.add(entry.getValue().bitmap);
}

// assert using lists because order is important for LRUs
Expand Down