From c3b470f308487d28d132fa6e23be56c37e5ce91f Mon Sep 17 00:00:00 2001 From: aquilescanta Date: Mon, 10 Jan 2022 22:35:28 +0000 Subject: [PATCH] Remove most allocations in SampleQueue.release SampleQueues may be released in the context of a finally block after an out of memory error. Allocating in that scenario can throw yet a new OutOfMemoryError. By safely releasing SampleQueue memory, we increase the possibility of handling the error gracefully. PiperOrigin-RevId: 420859022 --- .../exoplayer2/source/SampleDataQueue.java | 38 ++++++++++++------- .../exoplayer2/upstream/Allocator.java | 25 +++++++++++- .../exoplayer2/upstream/DefaultAllocator.java | 11 ++++++ 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/library/core/src/main/java/com/google/android/exoplayer2/source/SampleDataQueue.java b/library/core/src/main/java/com/google/android/exoplayer2/source/SampleDataQueue.java index d9e7ca95d2..5cf5660b02 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/source/SampleDataQueue.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/source/SampleDataQueue.java @@ -27,6 +27,7 @@ import com.google.android.exoplayer2.source.SampleQueue.SampleExtrasHolder; import com.google.android.exoplayer2.upstream.Allocation; import com.google.android.exoplayer2.upstream.Allocator; import com.google.android.exoplayer2.upstream.DataReader; +import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.ParsableByteArray; import com.google.android.exoplayer2.util.Util; import java.io.EOFException; @@ -79,6 +80,7 @@ import java.util.Arrays; * discarded, or 0 if the queue is now empty. */ public void discardUpstreamSampleBytes(long totalBytesWritten) { + Assertions.checkArgument(totalBytesWritten <= this.totalBytesWritten); this.totalBytesWritten = totalBytesWritten; if (this.totalBytesWritten == 0 || this.totalBytesWritten == firstAllocationNode.startPosition) { @@ -92,8 +94,8 @@ import java.util.Arrays; while (this.totalBytesWritten > lastNodeToKeep.endPosition) { lastNodeToKeep = lastNodeToKeep.next; } - // Discard all subsequent nodes. - AllocationNode firstNodeToDiscard = lastNodeToKeep.next; + // Discard all subsequent nodes. lastNodeToKeep is initialized, therefore next cannot be null. + AllocationNode firstNodeToDiscard = Assertions.checkNotNull(lastNodeToKeep.next); clearAllocationNodes(firstNodeToDiscard); // Reset the successor of the last node to be an uninitialized node. lastNodeToKeep.next = new AllocationNode(lastNodeToKeep.endPosition, allocationLength); @@ -213,17 +215,8 @@ import java.util.Arrays; // Bulk release allocations for performance (it's significantly faster when using // DefaultAllocator because the allocator's lock only needs to be acquired and released once) // [Internal: See b/29542039]. - int allocationCount = - (writeAllocationNode.allocation != null ? 1 : 0) - + ((int) (writeAllocationNode.startPosition - fromNode.startPosition) - / allocationLength); - Allocation[] allocationsToRelease = new Allocation[allocationCount]; - AllocationNode currentNode = fromNode; - for (int i = 0; i < allocationsToRelease.length; i++) { - allocationsToRelease[i] = currentNode.allocation; - currentNode = currentNode.clear(); - } - allocator.release(allocationsToRelease); + allocator.release(fromNode); + fromNode.clear(); } /** @@ -466,7 +459,7 @@ import java.util.Arrays; } /** A node in a linked list of {@link Allocation}s held by the output. */ - private static final class AllocationNode { + private static final class AllocationNode implements Allocator.AllocationNode { /** The absolute position of the start of the data (inclusive). */ public final long startPosition; @@ -525,5 +518,22 @@ import java.util.Arrays; next = null; return temp; } + + // AllocationChainNode implementation. + + @Override + public Allocation getAllocation() { + return Assertions.checkNotNull(allocation); + } + + @Override + @Nullable + public Allocator.AllocationNode next() { + if (next == null || next.allocation == null) { + return null; + } else { + return next; + } + } } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/Allocator.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/Allocator.java index 6b9ddcc1da..22d132dfed 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/Allocator.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/Allocator.java @@ -15,9 +15,22 @@ */ package com.google.android.exoplayer2.upstream; +import androidx.annotation.Nullable; + /** A source of allocations. */ public interface Allocator { + /** A node in a chain of {@link Allocation Allocations}. */ + interface AllocationNode { + + /** Returns the {@link Allocation} associated to this chain node. */ + Allocation getAllocation(); + + /** Returns the next chain node, or {@code null} if this is the last node in the chain. */ + @Nullable + AllocationNode next(); + } + /** * Obtain an {@link Allocation}. * @@ -36,15 +49,23 @@ public interface Allocator { void release(Allocation allocation); /** - * Releases an array of {@link Allocation}s back to the allocator. + * Releases an array of {@link Allocation Allocations} back to the allocator. * * @param allocations The array of {@link Allocation}s being released. */ void release(Allocation[] allocations); + /** + * Releases all {@link Allocation Allocations} in the chain starting at the given {@link + * AllocationNode}. + * + *

Implementations must not make memory allocations. + */ + void release(AllocationNode allocationNode); + /** * Hints to the allocator that it should make a best effort to release any excess {@link - * Allocation}s. + * Allocation Allocations}. */ void trim(); diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java index 47a77beb38..7797883d20 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultAllocator.java @@ -128,6 +128,17 @@ public final class DefaultAllocator implements Allocator { notifyAll(); } + @Override + public synchronized void release(@Nullable AllocationNode allocationNode) { + while (allocationNode != null) { + availableAllocations[availableCount++] = allocationNode.getAllocation(); + allocatedCount--; + allocationNode = allocationNode.next(); + } + // Wake up threads waiting for the allocated size to drop. + notifyAll(); + } + @Override public synchronized void trim() { int targetAllocationCount = Util.ceilDivide(targetBufferSize, individualAllocationSize);