diff --git a/RELEASENOTES.md b/RELEASENOTES.md index 307b0534a1..fb251646fb 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -44,6 +44,8 @@ * Fix issue with reusing a `ClippingMediaSource` with an inner `ExtractorMediaSource` and a non-zero start position ([#5351](https://github.com/google/ExoPlayer/issues/5351)). +* Downloading/Caching: Improve cache performance + ([#4253](https://github.com/google/ExoPlayer/issues/4253)). ### 2.9.3 ### diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloaderConstructorHelper.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloaderConstructorHelper.java index 100e1a03fe..59a11934b1 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloaderConstructorHelper.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/DownloaderConstructorHelper.java @@ -109,16 +109,18 @@ public final class DownloaderConstructorHelper { cacheReadDataSourceFactory != null ? cacheReadDataSourceFactory : new FileDataSourceFactory(); - DataSink.Factory writeDataSinkFactory = - cacheWriteDataSinkFactory != null - ? cacheWriteDataSinkFactory - : new CacheDataSinkFactory(cache, CacheDataSink.DEFAULT_MAX_CACHE_FILE_SIZE); + if (cacheWriteDataSinkFactory == null) { + CacheDataSinkFactory factory = + new CacheDataSinkFactory(cache, CacheDataSink.DEFAULT_FRAGMENT_SIZE); + factory.experimental_setRespectCacheFragmentationFlag(true); + cacheWriteDataSinkFactory = factory; + } onlineCacheDataSourceFactory = new CacheDataSourceFactory( cache, upstreamFactory, readDataSourceFactory, - writeDataSinkFactory, + cacheWriteDataSinkFactory, CacheDataSource.FLAG_BLOCK_ON_CACHE, /* eventListener= */ null, cacheKeyFactory); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java index 41f0944b75..25b4e07bcd 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/offline/ProgressiveDownloader.java @@ -53,7 +53,11 @@ public final class ProgressiveDownloader implements Downloader { Uri uri, @Nullable String customCacheKey, DownloaderConstructorHelper constructorHelper) { this.dataSpec = new DataSpec( - uri, /* absoluteStreamPosition= */ 0, C.LENGTH_UNSET, customCacheKey, /* flags= */ 0); + uri, + /* absoluteStreamPosition= */ 0, + C.LENGTH_UNSET, + customCacheKey, + /* flags= */ DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION); this.cache = constructorHelper.getCache(); this.dataSource = constructorHelper.createCacheDataSource(); this.cacheKeyFactory = constructorHelper.getCacheKeyFactory(); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java index 21cb4aa226..40fac19178 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/ExtractorMediaPeriod.java @@ -988,7 +988,9 @@ import org.checkerframework.checker.nullness.compatqual.NullableType; position, C.LENGTH_UNSET, customCacheKey, - DataSpec.FLAG_ALLOW_ICY_METADATA | DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN); + DataSpec.FLAG_ALLOW_ICY_METADATA + | DataSpec.FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN + | DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION); } private void setLoadPosition(long position, long timeUs) { diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java index 75af3e96df..2996de4527 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DataSpec.java @@ -32,14 +32,19 @@ public final class DataSpec { /** * The flags that apply to any request for data. Possible flag values are {@link - * #FLAG_ALLOW_GZIP}, {@link #FLAG_ALLOW_ICY_METADATA} and {@link - * #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN}. + * #FLAG_ALLOW_GZIP}, {@link #FLAG_ALLOW_ICY_METADATA}, {@link #FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN} + * and {@link #FLAG_ALLOW_CACHE_FRAGMENTATION}. */ @Documented @Retention(RetentionPolicy.SOURCE) @IntDef( flag = true, - value = {FLAG_ALLOW_GZIP, FLAG_ALLOW_ICY_METADATA, FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN}) + value = { + FLAG_ALLOW_GZIP, + FLAG_ALLOW_ICY_METADATA, + FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN, + FLAG_ALLOW_CACHE_FRAGMENTATION + }) public @interface Flags {} /** * Allows an underlying network stack to request that the server use gzip compression. @@ -53,12 +58,17 @@ public final class DataSpec { * DataSource#read(byte[], int, int)} will be the decompressed data. */ public static final int FLAG_ALLOW_GZIP = 1; - /** Allows an underlying network stack to request that the stream contain ICY metadata. */ public static final int FLAG_ALLOW_ICY_METADATA = 1 << 1; // 2 - /** Prevents caching if the length cannot be resolved when the {@link DataSource} is opened. */ public static final int FLAG_DONT_CACHE_IF_LENGTH_UNKNOWN = 1 << 2; // 4 + /** + * Allows fragmentation of this request into multiple cache files, meaning a cache eviction policy + * will be able to evict individual fragments of the data. Depending on the cache implementation, + * setting this flag may also enable more concurrent access to the data (e.g. reading one fragment + * whilst writing another). + */ + public static final int FLAG_ALLOW_CACHE_FRAGMENTATION = 1 << 4; // 8 /** * The set of HTTP methods that are supported by ExoPlayer {@link HttpDataSource}s. One of {@link diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSink.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSink.java index 1da2c3f636..d527805120 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSink.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSink.java @@ -37,20 +37,22 @@ import java.io.OutputStream; */ public final class CacheDataSink implements DataSink { - /** Default {@code maxCacheFileSize} recommended for caching use cases. */ - public static final long DEFAULT_MAX_CACHE_FILE_SIZE = 5 * 1024 * 1024; + /** Default {@code fragmentSize} recommended for caching use cases. */ + public static final long DEFAULT_FRAGMENT_SIZE = 5 * 1024 * 1024; /** Default buffer size in bytes. */ public static final int DEFAULT_BUFFER_SIZE = 20 * 1024; - private static final long MIN_RECOMMENDED_MAX_CACHE_FILE_SIZE = 2 * 1024 * 1024; + private static final long MIN_RECOMMENDED_FRAGMENT_SIZE = 2 * 1024 * 1024; private static final String TAG = "CacheDataSink"; private final Cache cache; - private final long maxCacheFileSize; + private final long fragmentSize; private final int bufferSize; private boolean syncFileDescriptor; + private boolean respectCacheFragmentationFlag; private DataSpec dataSpec; + private long dataSpecFragmentSize; private File file; private OutputStream outputStream; private FileOutputStream underlyingFileOutputStream; @@ -73,42 +75,39 @@ public final class CacheDataSink implements DataSink { * Constructs an instance using {@link #DEFAULT_BUFFER_SIZE}. * * @param cache The cache into which data should be written. - * @param maxCacheFileSize The maximum size of a cache file, in bytes. If a request results in - * data being written whose size exceeds this value, then the data will be fragmented into - * multiple cache files. If set to {@link C#LENGTH_UNSET} then no fragmentation will occur. - * Using a small value allows for finer-grained cache eviction policies, at the cost of - * increased overhead both on the cache implementation and the file system. Values under - * {@code (2 * 1024 * 1024)} are not recommended. + * @param fragmentSize For requests that should be fragmented into multiple cache files, this is + * the maximum size of a cache file in bytes. If set to {@link C#LENGTH_UNSET} then no + * fragmentation will occur. Using a small value allows for finer-grained cache eviction + * policies, at the cost of increased overhead both on the cache implementation and the file + * system. Values under {@code (2 * 1024 * 1024)} are not recommended. */ - public CacheDataSink(Cache cache, long maxCacheFileSize) { - this(cache, maxCacheFileSize, DEFAULT_BUFFER_SIZE); + public CacheDataSink(Cache cache, long fragmentSize) { + this(cache, fragmentSize, DEFAULT_BUFFER_SIZE); } /** * @param cache The cache into which data should be written. - * @param maxCacheFileSize The maximum size of a cache file, in bytes. If a request results in - * data being written whose size exceeds this value, then the data will be fragmented into - * multiple cache files. If set to {@link C#LENGTH_UNSET} then no fragmentation will occur. - * Using a small value allows for finer-grained cache eviction policies, at the cost of - * increased overhead both on the cache implementation and the file system. Values under - * {@code (2 * 1024 * 1024)} are not recommended. + * @param fragmentSize For requests that should be fragmented into multiple cache files, this is + * the maximum size of a cache file in bytes. If set to {@link C#LENGTH_UNSET} then no + * fragmentation will occur. Using a small value allows for finer-grained cache eviction + * policies, at the cost of increased overhead both on the cache implementation and the file + * system. Values under {@code (2 * 1024 * 1024)} are not recommended. * @param bufferSize The buffer size in bytes for writing to a cache file. A zero or negative * value disables buffering. */ - public CacheDataSink(Cache cache, long maxCacheFileSize, int bufferSize) { + public CacheDataSink(Cache cache, long fragmentSize, int bufferSize) { Assertions.checkState( - maxCacheFileSize > 0 || maxCacheFileSize == C.LENGTH_UNSET, - "maxCacheFileSize must be positive or C.LENGTH_UNSET."); - if (maxCacheFileSize != C.LENGTH_UNSET - && maxCacheFileSize < MIN_RECOMMENDED_MAX_CACHE_FILE_SIZE) { + fragmentSize > 0 || fragmentSize == C.LENGTH_UNSET, + "fragmentSize must be positive or C.LENGTH_UNSET."); + if (fragmentSize != C.LENGTH_UNSET && fragmentSize < MIN_RECOMMENDED_FRAGMENT_SIZE) { Log.w( TAG, - "maxCacheFileSize is below the minimum recommended value of " - + MIN_RECOMMENDED_MAX_CACHE_FILE_SIZE + "fragmentSize is below the minimum recommended value of " + + MIN_RECOMMENDED_FRAGMENT_SIZE + ". This may cause poor cache performance."); } this.cache = Assertions.checkNotNull(cache); - this.maxCacheFileSize = maxCacheFileSize == C.LENGTH_UNSET ? Long.MAX_VALUE : maxCacheFileSize; + this.fragmentSize = fragmentSize == C.LENGTH_UNSET ? Long.MAX_VALUE : fragmentSize; this.bufferSize = bufferSize; syncFileDescriptor = true; } @@ -116,8 +115,7 @@ public final class CacheDataSink implements DataSink { /** * Sets whether file descriptors are synced when closing output streams. * - *

This method is experimental, and will be renamed or removed in a future release. It should - * only be called before the renderer is used. + *

This method is experimental, and will be renamed or removed in a future release. * * @param syncFileDescriptor Whether file descriptors are synced when closing output streams. */ @@ -125,6 +123,20 @@ public final class CacheDataSink implements DataSink { this.syncFileDescriptor = syncFileDescriptor; } + /** + * Sets whether this instance respects the {@link DataSpec#FLAG_ALLOW_CACHE_FRAGMENTATION} flag. + * If set to {@code false} requests will always be fragmented. If set to {@code true} requests + * will be fragmented only if the flag is set. + * + *

This method is experimental, and will be renamed or removed in a future release. + * + * @param respectCacheFragmentationFlag Whether to respect the {@link + * DataSpec#FLAG_ALLOW_CACHE_FRAGMENTATION} flag. + */ + public void experimental_setRespectCacheFragmentationFlag(boolean respectCacheFragmentationFlag) { + this.respectCacheFragmentationFlag = respectCacheFragmentationFlag; + } + @Override public void open(DataSpec dataSpec) throws CacheDataSinkException { if (dataSpec.length == C.LENGTH_UNSET @@ -133,6 +145,11 @@ public final class CacheDataSink implements DataSink { return; } this.dataSpec = dataSpec; + this.dataSpecFragmentSize = + !respectCacheFragmentationFlag + || dataSpec.isFlagSet(DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION) + ? fragmentSize + : Long.MAX_VALUE; dataSpecBytesWritten = 0; try { openNextOutputStream(); @@ -149,12 +166,12 @@ public final class CacheDataSink implements DataSink { try { int bytesWritten = 0; while (bytesWritten < length) { - if (outputStreamBytesWritten == maxCacheFileSize) { + if (outputStreamBytesWritten == dataSpecFragmentSize) { closeCurrentOutputStream(); openNextOutputStream(); } - int bytesToWrite = (int) Math.min(length - bytesWritten, - maxCacheFileSize - outputStreamBytesWritten); + int bytesToWrite = + (int) Math.min(length - bytesWritten, dataSpecFragmentSize - outputStreamBytesWritten); outputStream.write(buffer, offset + bytesWritten, bytesToWrite); bytesWritten += bytesToWrite; outputStreamBytesWritten += bytesToWrite; @@ -181,7 +198,7 @@ public final class CacheDataSink implements DataSink { long length = dataSpec.length == C.LENGTH_UNSET ? C.LENGTH_UNSET - : Math.min(dataSpec.length - dataSpecBytesWritten, maxCacheFileSize); + : Math.min(dataSpec.length - dataSpecBytesWritten, dataSpecFragmentSize); file = cache.startFile( dataSpec.key, dataSpec.absoluteStreamPosition + dataSpecBytesWritten, length); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSinkFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSinkFactory.java index 6dcb14f5fb..9540597c2e 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSinkFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSinkFactory.java @@ -23,27 +23,50 @@ import com.google.android.exoplayer2.upstream.DataSink; public final class CacheDataSinkFactory implements DataSink.Factory { private final Cache cache; - private final long maxCacheFileSize; + private final long fragmentSize; private final int bufferSize; - /** - * @see CacheDataSink#CacheDataSink(Cache, long) - */ - public CacheDataSinkFactory(Cache cache, long maxCacheFileSize) { - this(cache, maxCacheFileSize, CacheDataSink.DEFAULT_BUFFER_SIZE); + private boolean syncFileDescriptor; + private boolean respectCacheFragmentationFlag; + + /** @see CacheDataSink#CacheDataSink(Cache, long) */ + public CacheDataSinkFactory(Cache cache, long fragmentSize) { + this(cache, fragmentSize, CacheDataSink.DEFAULT_BUFFER_SIZE); + } + + /** @see CacheDataSink#CacheDataSink(Cache, long, int) */ + public CacheDataSinkFactory(Cache cache, long fragmentSize, int bufferSize) { + this.cache = cache; + this.fragmentSize = fragmentSize; + this.bufferSize = bufferSize; } /** - * @see CacheDataSink#CacheDataSink(Cache, long, int) + * See {@link CacheDataSink#experimental_setSyncFileDescriptor(boolean)}. + * + *

This method is experimental, and will be renamed or removed in a future release. */ - public CacheDataSinkFactory(Cache cache, long maxCacheFileSize, int bufferSize) { - this.cache = cache; - this.maxCacheFileSize = maxCacheFileSize; - this.bufferSize = bufferSize; + public CacheDataSinkFactory experimental_setSyncFileDescriptor(boolean syncFileDescriptor) { + this.syncFileDescriptor = syncFileDescriptor; + return this; + } + + /** + * See {@link CacheDataSink#experimental_setRespectCacheFragmentationFlag(boolean)}. + * + *

This method is experimental, and will be renamed or removed in a future release. + */ + public CacheDataSinkFactory experimental_setRespectCacheFragmentationFlag( + boolean respectCacheFragmentationFlag) { + this.respectCacheFragmentationFlag = respectCacheFragmentationFlag; + return this; } @Override public DataSink createDataSink() { - return new CacheDataSink(cache, maxCacheFileSize, bufferSize); + CacheDataSink dataSink = new CacheDataSink(cache, fragmentSize, bufferSize); + dataSink.experimental_setSyncFileDescriptor(syncFileDescriptor); + dataSink.experimental_setRespectCacheFragmentationFlag(respectCacheFragmentationFlag); + return dataSink; } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java index 579f5d05e9..909bd40023 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSource.java @@ -172,7 +172,7 @@ public final class CacheDataSource implements DataSource { cache, upstream, new FileDataSource(), - new CacheDataSink(cache, CacheDataSink.DEFAULT_MAX_CACHE_FILE_SIZE), + new CacheDataSink(cache, CacheDataSink.DEFAULT_FRAGMENT_SIZE), flags, /* eventListener= */ null); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceFactory.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceFactory.java index e25c3d7a4a..9675aa1762 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceFactory.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceFactory.java @@ -50,7 +50,7 @@ public final class CacheDataSourceFactory implements DataSource.Factory { cache, upstreamFactory, new FileDataSourceFactory(), - new CacheDataSinkFactory(cache, CacheDataSink.DEFAULT_MAX_CACHE_FILE_SIZE), + new CacheDataSinkFactory(cache, CacheDataSink.DEFAULT_FRAGMENT_SIZE), flags, /* eventListener= */ null); } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java index 666fa87e9e..f8c417499f 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheDataSourceTest.java @@ -47,7 +47,7 @@ import org.robolectric.RuntimeEnvironment; public final class CacheDataSourceTest { private static final byte[] TEST_DATA = new byte[] {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}; - private static final int MAX_CACHE_FILE_SIZE = 3; + private static final int CACHE_FRAGMENT_SIZE = 3; private static final String DATASPEC_KEY = "dataSpecKey"; private Uri testDataUri; @@ -81,13 +81,13 @@ public final class CacheDataSourceTest { } @Test - public void testMaxCacheFileSize() throws Exception { + public void testFragmentSize() throws Exception { CacheDataSource cacheDataSource = createCacheDataSource(false, false); assertReadDataContentLength(cacheDataSource, boundedDataSpec, false, false); for (String key : cache.getKeys()) { for (CacheSpan cacheSpan : cache.getCachedSpans(key)) { - assertThat(cacheSpan.length <= MAX_CACHE_FILE_SIZE).isTrue(); - assertThat(cacheSpan.file.length() <= MAX_CACHE_FILE_SIZE).isTrue(); + assertThat(cacheSpan.length <= CACHE_FRAGMENT_SIZE).isTrue(); + assertThat(cacheSpan.file.length() <= CACHE_FRAGMENT_SIZE).isTrue(); } } } @@ -548,14 +548,14 @@ public final class CacheDataSourceTest { setReadException, unknownLength, CacheDataSource.FLAG_BLOCK_ON_CACHE, - new CacheDataSink(cache, MAX_CACHE_FILE_SIZE), + new CacheDataSink(cache, CACHE_FRAGMENT_SIZE), cacheKeyFactory); } private CacheDataSource createCacheDataSource( boolean setReadException, boolean unknownLength, @CacheDataSource.Flags int flags) { return createCacheDataSource( - setReadException, unknownLength, flags, new CacheDataSink(cache, MAX_CACHE_FILE_SIZE)); + setReadException, unknownLength, flags, new CacheDataSink(cache, CACHE_FRAGMENT_SIZE)); } private CacheDataSource createCacheDataSource( @@ -602,6 +602,7 @@ public final class CacheDataSourceTest { } private DataSpec buildDataSpec(long position, long length, @Nullable String key) { - return new DataSpec(testDataUri, position, length, key); + return new DataSpec( + testDataUri, position, length, key, DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION); } } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java index d1bf734a98..228567a4bc 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/cache/CacheUtilTest.java @@ -336,17 +336,17 @@ public final class CacheUtilTest { FakeDataSource dataSource = new FakeDataSource(fakeDataSet); Uri uri = Uri.parse("test_data"); - DataSpec dataSpec = new DataSpec(uri); + DataSpec dataSpec = new DataSpec(uri, DataSpec.FLAG_ALLOW_CACHE_FRAGMENTATION); CacheUtil.cache( dataSpec, cache, /* cacheKeyFactory= */ null, - // Set maxCacheFileSize to 10 to make sure there are multiple spans. + // Set fragmentSize to 10 to make sure there are multiple spans. new CacheDataSource( cache, dataSource, new FileDataSource(), - new CacheDataSink(cache, /* maxCacheFileSize= */ 10), + new CacheDataSink(cache, /* fragmentSize= */ 10), /* flags= */ 0, /* eventListener= */ null), new byte[CacheUtil.DEFAULT_BUFFER_SIZE_BYTES],