Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7db1b40a authored by Jesse Wilson's avatar Jesse Wilson
Browse files

Callback on any removal, not just evictions.

Don't hold locks while running create or remove callbacks. That gets a bit
ugly because it means a create could be unwanted by the time it returns.

Change-Id: I14b2b3ed41a446750f8ee5a7e35cb8d801c4ce6d
http://b/3461302
parent 261f33c1
Loading
Loading
Loading
Loading
+24 −5
Original line number Diff line number Diff line
@@ -206578,7 +206578,7 @@
 synchronized="false"
 static="false"
 final="false"
 deprecated="not deprecated"
 deprecated="deprecated"
 visibility="protected"
>
<parameter name="key" type="K">
@@ -206586,11 +206586,30 @@
<parameter name="value" type="V">
</parameter>
</method>
<method name="entryRemoved"
 return="void"
 abstract="false"
 native="false"
 synchronized="false"
 static="false"
 final="false"
 deprecated="not deprecated"
 visibility="protected"
>
<parameter name="evicted" type="boolean">
</parameter>
<parameter name="key" type="K">
</parameter>
<parameter name="oldValue" type="V">
</parameter>
<parameter name="newValue" type="V">
</parameter>
</method>
<method name="evictAll"
 return="void"
 abstract="false"
 native="false"
 synchronized="true"
 synchronized="false"
 static="false"
 final="true"
 deprecated="not deprecated"
@@ -206612,7 +206631,7 @@
 return="V"
 abstract="false"
 native="false"
 synchronized="true"
 synchronized="false"
 static="false"
 final="true"
 deprecated="not deprecated"
@@ -206658,7 +206677,7 @@
 return="V"
 abstract="false"
 native="false"
 synchronized="true"
 synchronized="false"
 static="false"
 final="true"
 deprecated="not deprecated"
@@ -206684,7 +206703,7 @@
 return="V"
 abstract="false"
 native="false"
 synchronized="true"
 synchronized="false"
 static="false"
 final="true"
 deprecated="not deprecated"
+122 −42
Original line number Diff line number Diff line
@@ -26,8 +26,7 @@ import java.util.Map;
 * become eligible for garbage collection.
 *
 * <p>If your cached values hold resources that need to be explicitly released,
 * override {@link #entryEvicted}. This method is only invoked when values are
 * evicted. Values replaced by calls to {@link #put} must be released manually.
 * override {@link #entryRemoved}.
 *
 * <p>If a cache miss should be computed on demand for the corresponding keys,
 * override {@link #create}. This simplifies the calling code, allowing it to
@@ -88,29 +87,52 @@ public class LruCache<K, V> {
     * head of the queue. This returns null if a value is not cached and cannot
     * be created.
     */
    public synchronized final V get(K key) {
    public final V get(K key) {
        if (key == null) {
            throw new NullPointerException("key == null");
        }

        V result = map.get(key);
        if (result != null) {
        V mapValue;
        synchronized (this) {
            mapValue = map.get(key);
            if (mapValue != null) {
                hitCount++;
            return result;
                return mapValue;
            }

            missCount++;
        }

        /*
         * Attempt to create a value. This may take a long time, and the map
         * may be different when create() returns. If a conflicting value was
         * added to the map while create() was working, we leave that value in
         * the map and release the created value.
         */

        // TODO: release the lock while calling this potentially slow user code
        result = create(key);
        V createdValue = create(key);
        if (createdValue == null) {
            return null;
        }

        if (result != null) {
        synchronized (this) {
            createCount++;
            size += safeSizeOf(key, result);
            map.put(key, result);
            mapValue = map.put(key, createdValue);

            if (mapValue != null) {
                // There was a conflict so undo that last put
                map.put(key, mapValue);
            } else {
                size += safeSizeOf(key, createdValue);
            }
        }

        if (mapValue != null) {
            entryRemoved(false, key, createdValue, mapValue);
            return mapValue;
        } else {
            trimToSize(maxSize);
            return createdValue;
        }
        return result;
    }

    /**
@@ -120,41 +142,60 @@ public class LruCache<K, V> {
     * @return the previous value mapped by {@code key}. Although that entry is
     *     no longer cached, it has not been passed to {@link #entryEvicted}.
     */
    public synchronized final V put(K key, V value) {
    public final V put(K key, V value) {
        if (key == null || value == null) {
            throw new NullPointerException("key == null || value == null");
        }

        V previous;
        synchronized (this) {
            putCount++;
            size += safeSizeOf(key, value);
        V previous = map.put(key, value);
            previous = map.put(key, value);
            if (previous != null) {
                size -= safeSizeOf(key, previous);
            }
        }

        if (previous != null) {
            entryRemoved(false, key, previous, value);
        }

        trimToSize(maxSize);
        return previous;
    }

    /**
     * @param maxSize the maximum size of the cache before returning. May be -1
     *     to evict even 0-sized elements.
     */
    private void trimToSize(int maxSize) {
        while (size > maxSize) {
            Map.Entry<K, V> toEvict = map.eldest(); // equal to map.entrySet().iterator().next();
        while (true) {
            K key;
            V value;
            synchronized (this) {
                if (size < 0 || (map.isEmpty() && size != 0)) {
                    throw new IllegalStateException(getClass().getName()
                            + ".sizeOf() is reporting inconsistent results!");
                }

                if (size <= maxSize) {
                    break;
                }

                Map.Entry<K, V> toEvict = map.eldest();
                if (toEvict == null) {
                break; // map is empty; if size is not 0 then throw an error below
                    break;
                }

            K key = toEvict.getKey();
            V value = toEvict.getValue();
                key = toEvict.getKey();
                value = toEvict.getValue();
                map.remove(key);
                size -= safeSizeOf(key, value);
                evictionCount++;

            // TODO: release the lock while calling this potentially slow user code
            entryEvicted(key, value);
            }

        if (size < 0 || (map.isEmpty() && size != 0)) {
            throw new IllegalStateException(getClass().getName()
                    + ".sizeOf() is reporting inconsistent results!");
            entryEvicted(key, value);
        }
    }

@@ -164,28 +205,67 @@ public class LruCache<K, V> {
     * @return the previous value mapped by {@code key}. Although that entry is
     *     no longer cached, it has not been passed to {@link #entryEvicted}.
     */
    public synchronized final V remove(K key) {
    public final V remove(K key) {
        if (key == null) {
            throw new NullPointerException("key == null");
        }

        V previous = map.remove(key);
        V previous;
        synchronized (this) {
            previous = map.remove(key);
            if (previous != null) {
                size -= safeSizeOf(key, previous);
            }
        }

        if (previous != null) {
            entryRemoved(false, key, previous, null);
        }

        return previous;
    }

    /**
     * Called for entries that have reached the tail of the least recently used
     * queue and are be removed. The default implementation does nothing.
     * Calls {@link #entryRemoved}.
     *
     * @deprecated replaced by entryRemoved
     */
    protected void entryEvicted(K key, V value) {}
    @Deprecated
    protected void entryEvicted(K key, V value) {
        entryRemoved(true, key, value, null);
    }

    /**
     * Called for entries that have been evicted or removed. This method is
     * invoked when a value is evicted to make space, removed by a call to
     * {@link #remove}, or replaced by a call to {@link #put}. The default
     * implementation does nothing.
     *
     * <p>The method is called without synchronization: other threads may
     * access the cache while this method is executing.
     *
     * @param evicted true if the entry is being removed to make space, false
     *     if the removal was caused by a {@link #put} or {@link #remove}.
     * @param newValue the new value for {@code key}, if it exists. If non-null,
     *     this removal was caused by a {@link #put}. Otherwise it was caused by
     *     an eviction or a {@link #remove}.
     */
    protected void entryRemoved(boolean evicted, K key, V oldValue, V newValue) {}

    /**
     * Called after a cache miss to compute a value for the corresponding key.
     * Returns the computed value or null if no value can be computed. The
     * default implementation returns null.
     *
     * <p>The method is called without synchronization: other threads may
     * access the cache while this method is executing.
     *
     * <p>If a value for {@code key} exists in the cache when this method
     * returns, the created value will be released with {@link #entryRemoved}
     * and discarded. This can occur when multiple threads request the same key
     * at the same time (causing multiple values to be created), or when one
     * thread calls {@link #put} while another is creating a value for the same
     * key.
     */
    protected V create(K key) {
        return null;
@@ -213,7 +293,7 @@ public class LruCache<K, V> {
    /**
     * Clear the cache, calling {@link #entryEvicted} on each removed entry.
     */
    public synchronized final void evictAll() {
    public final void evictAll() {
        trimToSize(-1); // -1 will evict 0-sized elements
    }

+113 −28
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.util;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import junit.framework.TestCase;
@@ -166,22 +167,16 @@ public final class LruCacheTest extends TestCase {
    }

    public void testEntryEvictedWhenFull() {
        List<String> expectedEvictionLog = new ArrayList<String>();
        final List<String> evictionLog = new ArrayList<String>();
        LruCache<String, String> cache = new LruCache<String, String>(3) {
            @Override protected void entryEvicted(String key, String value) {
                evictionLog.add(key + "=" + value);
            }
        };
        List<String> log = new ArrayList<String>();
        LruCache<String, String> cache = newRemovalLogCache(log);

        cache.put("a", "A");
        cache.put("b", "B");
        cache.put("c", "C");
        assertEquals(expectedEvictionLog, evictionLog);
        assertEquals(Collections.<String>emptyList(), log);

        cache.put("d", "D");
        expectedEvictionLog.add("a=A");
        assertEquals(expectedEvictionLog, evictionLog);
        assertEquals(Arrays.asList("a=A"), log);
    }

    /**
@@ -309,19 +304,14 @@ public final class LruCacheTest extends TestCase {
    }

    public void testEvictAll() {
        final List<String> evictionLog = new ArrayList<String>();
        LruCache<String, String> cache = new LruCache<String, String>(10) {
            @Override protected void entryEvicted(String key, String value) {
                evictionLog.add(key + "=" + value);
            }
        };

        List<String> log = new ArrayList<String>();
        LruCache<String, String> cache = newRemovalLogCache(log);
        cache.put("a", "A");
        cache.put("b", "B");
        cache.put("c", "C");
        cache.evictAll();
        assertEquals(0, cache.size());
        assertEquals(Arrays.asList("a=A", "b=B", "c=C"), evictionLog);
        assertEquals(Arrays.asList("a=A", "b=B", "c=C"), log);
    }

    public void testEvictAllEvictsSizeZeroElements() {
@@ -337,16 +327,6 @@ public final class LruCacheTest extends TestCase {
        assertSnapshot(cache);
    }

    public void testRemoveDoesNotCallEntryEvicted() {
        LruCache<String, String> cache = new LruCache<String, String>(10) {
            @Override protected void entryEvicted(String key, String value) {
                fail();
            }
        };
        cache.put("a", "A");
        assertEquals("A", cache.remove("a"));
    }

    public void testRemoveWithCustomSizes() {
        LruCache<String, String> cache = new LruCache<String, String>(10) {
            @Override protected int sizeOf(String key, String value) {
@@ -376,6 +356,99 @@ public final class LruCacheTest extends TestCase {
        }
    }

    public void testRemoveCallsEntryRemoved() {
        List<String> log = new ArrayList<String>();
        LruCache<String, String> cache = newRemovalLogCache(log);
        cache.put("a", "A");
        cache.remove("a");
        assertEquals(Arrays.asList("a=A>null"), log);
    }

    public void testPutCallsEntryRemoved() {
        List<String> log = new ArrayList<String>();
        LruCache<String, String> cache = newRemovalLogCache(log);
        cache.put("a", "A");
        cache.put("a", "A2");
        assertEquals(Arrays.asList("a=A>A2"), log);
    }

    public void testEntryRemovedIsCalledWithoutSynchronization() {
        LruCache<String, String> cache = new LruCache<String, String>(3) {
            @Override protected void entryRemoved(
                    boolean evicted, String key, String oldValue, String newValue) {
                assertFalse(Thread.holdsLock(this));
            }
        };

        cache.put("a", "A");
        cache.put("a", "A2"); // replaced
        cache.put("b", "B");
        cache.put("c", "C");
        cache.put("d", "D");  // single eviction
        cache.remove("a");    // removed
        cache.evictAll();     // multiple eviction
    }

    public void testCreateIsCalledWithoutSynchronization() {
        LruCache<String, String> cache = new LruCache<String, String>(3) {
            @Override protected String create(String key) {
                assertFalse(Thread.holdsLock(this));
                return null;
            }
        };

        cache.get("a");
    }

    /**
     * Test what happens when a value is added to the map while create is
     * working. The map value should be returned by get(), and the created value
     * should be released with entryRemoved().
     */
    public void testCreateWithConcurrentPut() {
        final List<String> log = new ArrayList<String>();
        LruCache<String, String> cache = new LruCache<String, String>(3) {
            @Override protected String create(String key) {
                put(key, "B");
                return "A";
            }
            @Override protected void entryRemoved(
                    boolean evicted, String key, String oldValue, String newValue) {
                log.add(key + "=" + oldValue + ">" + newValue);
            }
        };

        assertEquals("B", cache.get("a"));
        assertEquals(Arrays.asList("a=A>B"), log);
    }

    /**
     * Test what happens when two creates happen concurrently. The result from
     * the first create to return is returned by both gets. The other created
     * values should be released with entryRemove().
     */
    public void testCreateWithConcurrentCreate() {
        final List<String> log = new ArrayList<String>();
        LruCache<String, Integer> cache = new LruCache<String, Integer>(3) {
            int callCount = 0;
            @Override protected Integer create(String key) {
                if (callCount++ == 0) {
                    assertEquals(2, get(key).intValue());
                    return 1;
                } else {
                    return 2;
                }
            }
            @Override protected void entryRemoved(
                    boolean evicted, String key, Integer oldValue, Integer newValue) {
                log.add(key + "=" + oldValue + ">" + newValue);
            }
        };

        assertEquals(2, cache.get("a").intValue());
        assertEquals(Arrays.asList("a=1>2"), log);
    }

    private LruCache<String, String> newCreatingCache() {
        return new LruCache<String, String>(3) {
            @Override protected String create(String key) {
@@ -384,6 +457,18 @@ public final class LruCacheTest extends TestCase {
        };
    }

    private LruCache<String, String> newRemovalLogCache(final List<String> log) {
        return new LruCache<String, String>(3) {
            @Override protected void entryRemoved(
                    boolean evicted, String key, String oldValue, String newValue) {
                String message = evicted
                        ? (key + "=" + oldValue)
                        : (key + "=" + oldValue + ">" + newValue);
                log.add(message);
            }
        };
    }

    private void assertHit(LruCache<String, String> cache, String key, String value) {
        assertEquals(value, cache.get(key));
        expectedHitCount++;