Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class BidirectionalFitDecoderBufferAllocatorStrategy
public:
BidirectionalFitDecoderBufferAllocatorStrategy(size_t initial_capacity,
size_t allocation_increment)
: fallback_allocator_(/*enable_decommit_on_idle=*/false),
: fallback_allocator_(/*enable_decommit_on_idle=*/false,
/*enable_page_alignment=*/false),
bidirectional_fit_allocator_(&fallback_allocator_,
initial_capacity,
kSmallAllocationThreshold,
Expand All @@ -50,8 +51,10 @@ class BidirectionalFitDecoderBufferAllocatorStrategy
bool enable_decommit_on_idle,
size_t retain_blocks,
size_t conservative_decommit_blocks,
bool aggressive_decommit_on_suspend = false)
: fallback_allocator_(enable_decommit_on_idle),
bool aggressive_decommit_on_suspend = false,
bool allocate_with_page_alignment = true)
: fallback_allocator_(enable_decommit_on_idle,
allocate_with_page_alignment),
bidirectional_fit_allocator_(&fallback_allocator_,
initial_capacity,
kSmallAllocationThreshold,
Expand Down
23 changes: 17 additions & 6 deletions media/starboard/decoder_buffer_allocator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ void DecoderBufferAllocator::ReleaseIdleMemory() {
}

void DecoderBufferAllocator::DecommitAllDecommitableBlocks() {
if (!decommit_on_suspend_enabled_.load(std::memory_order_relaxed)) {
return;
}
base::AutoLock scoped_lock(mutex_);
if (strategy_) {
strategy_->DecommitAllDecommitableBlocks();
Expand Down Expand Up @@ -238,13 +241,19 @@ void DecoderBufferAllocator::EnableConfigurableDecommitStrategy(
int block_size,
int retain_blocks,
int conservative_decommit_blocks,
bool aggressive_decommit_on_suspend) {
bool aggressive_decommit_on_suspend,
bool allocate_with_page_alignment) {
auto* allocator = Get();
CHECK(allocator);
if (aggressive_decommit_on_suspend) {
// This optimization is enable only, and isn't expected to be disabled.
allocator->decommit_on_suspend_enabled_.store(true,
std::memory_order_relaxed);
}
Comment thread
xiaomings marked this conversation as resolved.
allocator->UpdateAllocatorStrategy(base::BindRepeating(
[](int block_size, int retain_blocks, int conservative_decommit_blocks,
bool aggressive_decommit_on_suspend, int initial_capacity,
int allocation_unit)
bool aggressive_decommit_on_suspend, bool allocate_with_page_alignment,
int initial_capacity, int allocation_unit)
-> std::unique_ptr<DecoderBufferAllocator::Strategy> {
LOG(INFO)
<< "DecoderBufferAllocator is using "
Expand All @@ -256,15 +265,17 @@ void DecoderBufferAllocator::EnableConfigurableDecommitStrategy(
<< "retain_blocks: " << retain_blocks << ", "
<< "conservative_decommit_blocks: " << conservative_decommit_blocks
<< ", aggressive_decommit_on_suspend: "
<< ToString(aggressive_decommit_on_suspend);
<< ToString(aggressive_decommit_on_suspend)
<< ", allocate_with_page_alignment: "
<< ToString(allocate_with_page_alignment);

return std::make_unique<DefaultReuseAllocatorStrategy>(
block_size, block_size, /*enable_decommit_on_idle=*/true,
retain_blocks, conservative_decommit_blocks,
aggressive_decommit_on_suspend);
aggressive_decommit_on_suspend, allocate_with_page_alignment);
},
block_size, retain_blocks, conservative_decommit_blocks,
aggressive_decommit_on_suspend));
aggressive_decommit_on_suspend, allocate_with_page_alignment));
}

// static
Expand Down
5 changes: 4 additions & 1 deletion media/starboard/decoder_buffer_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#ifndef MEDIA_STARBOARD_DECODER_BUFFER_ALLOCATOR_H_
#define MEDIA_STARBOARD_DECODER_BUFFER_ALLOCATOR_H_

#include <atomic>
#include <memory>
#include <sstream>

Expand Down Expand Up @@ -86,7 +87,8 @@ class DecoderBufferAllocator : public DecoderBuffer::Allocator,
int block_size,
int retain_blocks,
int conservative_decommit_blocks,
bool aggressive_decommit_on_suspend);
bool aggressive_decommit_on_suspend,
bool allocate_with_page_alignment);
static void EnableMediaBufferPoolStrategy();
static void EnableReleaseIdleMemory();

Expand Down Expand Up @@ -118,6 +120,7 @@ class DecoderBufferAllocator : public DecoderBuffer::Allocator,
int pending_allocation_operations_count_ GUARDED_BY(mutex_) = 0;
int allocation_operation_index_ GUARDED_BY(mutex_) = 0;
#endif // !BUILDFLAG(COBALT_IS_RELEASE_BUILD)
std::atomic<bool> decommit_on_suspend_enabled_{false};
};

} // namespace media
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ MediaBufferPoolDecoderBufferAllocatorStrategy::
: media_buffer_pool_(media_buffer_pool),
video_buffer_initial_capacity_(video_buffer_initial_capacity),
video_buffer_allocation_increment_(video_buffer_allocation_increment),
audio_fallback_allocator_(/*enable_decommit_on_idle=*/false),
audio_fallback_allocator_(/*enable_decommit_on_idle=*/false,
/*enable_page_alignment=*/false),
audio_allocator_(
&audio_fallback_allocator_,
// Pre-allocate sufficient capacity for audio buffers to
Expand Down
28 changes: 16 additions & 12 deletions media/starboard/starboard_memory_allocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,14 @@ using starboard::AlignUp;
// using posix_memalign() and free().
class StarboardMemoryAllocator : public starboard::Allocator {
public:
explicit StarboardMemoryAllocator(bool enable_decommit)
StarboardMemoryAllocator(bool enable_decommit, bool enable_page_alignment)
: enable_decommit_(enable_decommit),
enable_page_alignment_(enable_page_alignment),
page_size_(static_cast<size_t>(sysconf(_SC_PAGESIZE))) {
LOG(INFO) << "StarboardMemoryAllocator: decommit is "
<< (enable_decommit_ ? "enabled" : "disabled") << ".";
<< (enable_decommit_ ? "enabled" : "disabled")
<< ", page alignment is "
<< (enable_page_alignment_ ? "enabled" : "disabled") << ".";
}

void* Allocate(size_t size) override {
Expand All @@ -57,7 +60,7 @@ class StarboardMemoryAllocator : public starboard::Allocator {
}

void* AllocateForAlignment(size_t* size, size_t alignment) override {
if (enable_decommit_) {
if (enable_page_alignment_) {
alignment = std::max(alignment, page_size_);
*size = AlignUp(*size, page_size_);
}
Expand All @@ -69,23 +72,23 @@ class StarboardMemoryAllocator : public starboard::Allocator {

void Decommit(void* memory, size_t size, bool conservative) override {
#if !BUILDFLAG(COBALT_IS_RELEASE_BUILD)
uintptr_t start = reinterpret_cast<uintptr_t>(memory);
CHECK(enable_decommit_);
CHECK_EQ(start % page_size_, 0U);
CHECK_EQ(size % page_size_, 0U);
#endif // !BUILDFLAG(COBALT_IS_RELEASE_BUILD)

// Align down the size to prevent decommitting past the end just in case
// the user explicitly passes an altered, unaligned size block.
size_t aligned_size = AlignDown(size, page_size_);
uint8_t* start = static_cast<uint8_t*>(memory);
uint8_t* end = start + size;
uint8_t* aligned_start = AlignUp(start, page_size_);
uint8_t* aligned_end = AlignDown(end, page_size_);

if (aligned_size > 0) {
if (aligned_start < aligned_end) {
size_t aligned_size = aligned_end - aligned_start;
// MADV_FREE is not supported on all kernel versions/configurations. If it
// fails, fallback to MADV_DONTNEED to ensure memory is still decommitted.
if (conservative && madvise(memory, aligned_size, MADV_FREE) == 0) {
if (conservative &&
madvise(aligned_start, aligned_size, MADV_FREE) == 0) {
return;
}
madvise(memory, aligned_size, MADV_DONTNEED);
madvise(aligned_start, aligned_size, MADV_DONTNEED);
}
Comment thread
xiaomings marked this conversation as resolved.
Comment thread
xiaomings marked this conversation as resolved.
}

Expand All @@ -102,6 +105,7 @@ class StarboardMemoryAllocator : public starboard::Allocator {

private:
const bool enable_decommit_;
const bool enable_page_alignment_;
const size_t page_size_;
};

Expand Down
8 changes: 5 additions & 3 deletions media/starboard/starboard_memory_allocator_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ namespace {

class StarboardMemoryAllocatorTest : public ::testing::TestWithParam<bool> {
protected:
StarboardMemoryAllocatorTest() : allocator_(GetParam()) {}
StarboardMemoryAllocatorTest() : allocator_(GetParam(), GetParam()) {}
StarboardMemoryAllocator allocator_;
};

Expand Down Expand Up @@ -78,7 +78,8 @@ INSTANTIATE_TEST_SUITE_P(EnableDecommit,

TEST(StarboardMemoryAllocatorDecommitSpecificTest,
AllocateForAlignmentUpdatesSizeWhenDecommitEnabled) {
StarboardMemoryAllocator allocator(/*enable_decommit=*/true);
StarboardMemoryAllocator allocator(/*enable_decommit=*/true,
/*enable_page_alignment=*/true);

const size_t page_size = sysconf(_SC_PAGESIZE);
const size_t initial_size = page_size / 2; // A size smaller than page size
Expand All @@ -95,7 +96,8 @@ TEST(StarboardMemoryAllocatorDecommitSpecificTest,

TEST(StarboardMemoryAllocatorDecommitSpecificTest,
AllocateForAlignmentDoesNotUpdateSizeWhenDecommitDisabled) {
StarboardMemoryAllocator allocator(/*enable_decommit=*/false);
StarboardMemoryAllocator allocator(/*enable_decommit=*/false,
/*enable_page_alignment=*/false);

const size_t page_size = sysconf(_SC_PAGESIZE);
const size_t initial_size = page_size / 2;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,23 +149,27 @@ ScriptPromise<IDLUndefined> H5vccSettings::set(

if (name == "DecoderBuffer.EnableConfigurableDecommitStrategy") {
// The value is a 32-bit integer encoding four parts:
// aggressive_decommit_on_suspend (flag), block_size (in MB), retain_blocks
// (count), and conservative_decommit_blocks (count). For example,
// flags (8 bits), block_size (in MB), retain_blocks (count), and
// conservative_decommit_blocks (count). For example,
// 0x01040402 sets aggressive_decommit_on_suspend to true, 4 MB block size,
// 4 retain blocks, and 2 conservative decommit blocks respectively.
// Passing multiple parameters encoded within a single integer is not
// ideal, but it simplifies experiment setup in the current framework.
//
// - aggressive_decommit_on_suspend: When non-zero, enables aggressive
// MADV_DONTNEED decommit on all idle
// blocks when app suspends/hides.
// - block_size: Specifies both the initial pool capacity and the fallback
// allocation increment.
// - retain_blocks: The first `retain_blocks` blocks of the pool are kept
// fully committed.
// - conservative_decommit_blocks: The next `conservative_decommit_blocks`
// blocks are conservatively decommitted
// (e.g. using MADV_FREE).
// - flags (8 bits):
// - aggressive_decommit_on_suspend (bit 24): When set (LSB is non-zero),
// enables aggressive MADV_DONTNEED decommit on all idle blocks when
// app suspends/hides.
// - allocate_with_page_alignment (bit 25): When set (second bit is
// non-zero), forces new OS block allocations to be page-aligned and
// rounded up to page sizes. Default is true.
Comment thread
xiaomings marked this conversation as resolved.
// - block_size (8 bits): Specifies both the initial pool capacity and the
// fallback allocation increment.
// - retain_blocks (8 bits): Specifies the first `retain_blocks` blocks of
// the pool are kept fully committed.
// - conservative_decommit_blocks (8 bits): Specifies the next
// `conservative_decommit_blocks` blocks are conservatively decommitted
// (e.g. using MADV_FREE).
// Any remaining memory beyond these two windows is aggressively decommitted
// (e.g. MADV_DONTNEED).
// For example, if 128 MB is allocated for the memory pool, and
Expand All @@ -179,18 +183,25 @@ ScriptPromise<IDLUndefined> H5vccSettings::set(
// (returned
// to the OS immediately, with virtual memory address range retained).
//
// Note: The values passed in are assumed to be valid positive integers. A
// value of 0 will not enable this feature as it is not a positive integer.
return ProcessSettingAsPositiveInt(
// Note: A value of 0 or less will not enable this feature (handled as a
// placebo).
return ProcessSettingAs<int>(
script_state, exception_context, name, *value, [](int value) {
bool aggressive_decommit_on_suspend = ((value >> 24) & 0xFF) != 0;
if (value <= 0) {
// Explicitly allow non-positive values as placebo.
return base::ok();
}

bool aggressive_decommit_on_suspend = ((value >> 24) & 0x01) != 0;
bool allocate_with_page_alignment = ((value >> 24) & 0x02) != 0;
Comment thread
xiaomings marked this conversation as resolved.
Comment thread
xiaomings marked this conversation as resolved.
int block_size_mb = (value >> 16) & 0xFF;
int retain_blocks = (value >> 8) & 0xFF;
int conservative_decommit_blocks = value & 0xFF;
::media::DecoderBufferAllocator::EnableConfigurableDecommitStrategy(
block_size_mb * 1024 * 1024, retain_blocks,
conservative_decommit_blocks, aggressive_decommit_on_suspend);
return true;
conservative_decommit_blocks, aggressive_decommit_on_suspend,
allocate_with_page_alignment);
return base::ok();
});
}
if (name == "DecoderBuffer.EnableMediaBufferPoolAllocatorStrategy") {
Expand Down
Loading