Commit 3a9557c7 by olly Committed by Oliver Woodman

Don't pass maxCacheFileSize to CacheEvictor if real size is unknown

CacheEvictor.onStartFile currently receives a length, which is the
maximum size of the content that might be written. The LRU cache
evictor will make sure there's sufficient space for the specified
size. If the actual size of the content is unknown, CacheDataSink
passes maxCacheFileSize.

The current behavior isn't ideal. It seems valid for a developer to
specify maxCacheFileSize=Long.MAX_VALUE if they don't want any
file fragmentation in the cache. However if they then attempt to
cache something with unknown length, LRU cache will try and make
room for content of length Long.MAX_VALUE, and end up evicting the
entire cache to do so.

This change alters the logic so a length is only passed if the
actual size of the content is known. In other cases C.LENGTH_UNSET
is now passed. The LRU evictor will not evict until after the file
is committed. Note a custom LRU evictor could still opt to evict to
ensure some application specified amount of space, if that's the
desired behavior.

PiperOrigin-RevId: 227509525
parent 243b12f3
...@@ -16,6 +16,7 @@ ...@@ -16,6 +16,7 @@
package com.google.android.exoplayer2.upstream.cache; package com.google.android.exoplayer2.upstream.cache;
import android.support.annotation.Nullable; import android.support.annotation.Nullable;
import com.google.android.exoplayer2.C;
import java.io.File; import java.io.File;
import java.io.IOException; import java.io.IOException;
import java.util.NavigableSet; import java.util.NavigableSet;
...@@ -169,12 +170,12 @@ public interface Cache { ...@@ -169,12 +170,12 @@ public interface Cache {
* *
* @param key The cache key for the data. * @param key The cache key for the data.
* @param position The starting position of the data. * @param position The starting position of the data.
* @param maxLength The maximum length of the data to be written. Used only to ensure that there * @param length The length of the data being written, or {@link C#LENGTH_UNSET} if unknown. Used
* is enough space in the cache. * only to ensure that there is enough space in the cache.
* @return The file into which data should be written. * @return The file into which data should be written.
* @throws CacheException If an error is encountered. * @throws CacheException If an error is encountered.
*/ */
File startFile(String key, long position, long maxLength) throws CacheException; File startFile(String key, long position, long length) throws CacheException;
/** /**
* Commits a file into the cache. Must only be called when holding a corresponding hole {@link * Commits a file into the cache. Must only be called when holding a corresponding hole {@link
......
...@@ -170,10 +170,13 @@ public final class CacheDataSink implements DataSink { ...@@ -170,10 +170,13 @@ public final class CacheDataSink implements DataSink {
} }
private void openNextOutputStream() throws IOException { private void openNextOutputStream() throws IOException {
long maxLength = dataSpec.length == C.LENGTH_UNSET ? maxCacheFileSize long length =
: Math.min(dataSpec.length - dataSpecBytesWritten, maxCacheFileSize); dataSpec.length == C.LENGTH_UNSET
file = cache.startFile(dataSpec.key, dataSpec.absoluteStreamPosition + dataSpecBytesWritten, ? C.LENGTH_UNSET
maxLength); : Math.min(dataSpec.length - dataSpecBytesWritten, maxCacheFileSize);
file =
cache.startFile(
dataSpec.key, dataSpec.absoluteStreamPosition + dataSpecBytesWritten, length);
underlyingFileOutputStream = new FileOutputStream(file); underlyingFileOutputStream = new FileOutputStream(file);
if (bufferSize > 0) { if (bufferSize > 0) {
if (bufferedOutputStream == null) { if (bufferedOutputStream == null) {
......
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
*/ */
package com.google.android.exoplayer2.upstream.cache; package com.google.android.exoplayer2.upstream.cache;
import com.google.android.exoplayer2.C;
/** /**
* Evicts data from a {@link Cache}. Implementations should call {@link Cache#removeSpan(CacheSpan)} * Evicts data from a {@link Cache}. Implementations should call {@link Cache#removeSpan(CacheSpan)}
* to evict cache entries based on their eviction policies. * to evict cache entries based on their eviction policies.
...@@ -32,8 +34,7 @@ public interface CacheEvictor extends Cache.Listener { ...@@ -32,8 +34,7 @@ public interface CacheEvictor extends Cache.Listener {
* @param cache The source of the event. * @param cache The source of the event.
* @param key The key being written. * @param key The key being written.
* @param position The starting position of the data being written. * @param position The starting position of the data being written.
* @param maxLength The maximum length of the data being written. * @param length The length of the data being written, or {@link C#LENGTH_UNSET} if unknown.
*/ */
void onStartFile(Cache cache, String key, long position, long maxLength); void onStartFile(Cache cache, String key, long position, long length);
} }
...@@ -15,6 +15,7 @@ ...@@ -15,6 +15,7 @@
*/ */
package com.google.android.exoplayer2.upstream.cache; package com.google.android.exoplayer2.upstream.cache;
import com.google.android.exoplayer2.C;
import com.google.android.exoplayer2.upstream.cache.Cache.CacheException; import com.google.android.exoplayer2.upstream.cache.Cache.CacheException;
import java.util.Comparator; import java.util.Comparator;
import java.util.TreeSet; import java.util.TreeSet;
...@@ -40,8 +41,10 @@ public final class LeastRecentlyUsedCacheEvictor implements CacheEvictor, Compar ...@@ -40,8 +41,10 @@ public final class LeastRecentlyUsedCacheEvictor implements CacheEvictor, Compar
} }
@Override @Override
public void onStartFile(Cache cache, String key, long position, long maxLength) { public void onStartFile(Cache cache, String key, long position, long length) {
evictCache(cache, maxLength); if (length != C.LENGTH_UNSET) {
evictCache(cache, length);
}
} }
@Override @Override
......
...@@ -259,8 +259,7 @@ public final class SimpleCache implements Cache { ...@@ -259,8 +259,7 @@ public final class SimpleCache implements Cache {
} }
@Override @Override
public synchronized File startFile(String key, long position, long maxLength) public synchronized File startFile(String key, long position, long length) throws CacheException {
throws CacheException {
Assertions.checkState(!released); Assertions.checkState(!released);
CachedContent cachedContent = index.get(key); CachedContent cachedContent = index.get(key);
Assertions.checkNotNull(cachedContent); Assertions.checkNotNull(cachedContent);
...@@ -270,7 +269,7 @@ public final class SimpleCache implements Cache { ...@@ -270,7 +269,7 @@ public final class SimpleCache implements Cache {
cacheDir.mkdirs(); cacheDir.mkdirs();
removeStaleSpans(); removeStaleSpans();
} }
evictor.onStartFile(this, key, position, maxLength); evictor.onStartFile(this, key, position, length);
return SimpleCacheSpan.getCacheFile( return SimpleCacheSpan.getCacheFile(
cacheDir, cachedContent.id, position, System.currentTimeMillis()); cacheDir, cachedContent.id, position, System.currentTimeMillis());
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment