From 3b4100772c979e705e5b08e1bedd2c58c6ccb2e8 Mon Sep 17 00:00:00 2001 From: Xiaoming Shi Date: Sat, 25 Jul 2026 17:20:10 -0700 Subject: [PATCH] media: Add page alignment flag and lockless suspend decommit in DecoderBufferAllocator Add support for the allocate_with_page_alignment flag to the configurable decommit strategy. This flag is bound to bit 25 (the second bit of the highest byte) of the setting and controls whether the fallback block allocator forces page alignment and page-rounded size. Additionally, implement a lockless suspend decommit optimization by checking the decommit_on_suspend_enabled_ atomic flag before acquiring the mutex. This prevents UI thread blocking on suspend when decommits are disabled. Also simplify StarboardMemoryAllocator pointer arithmetic inside Decommit() using uint8_t*, clean up redundant constructors, and standardize parameter documentation. Bug: 454441375 TAG=agy CONV=92545d5a-4b8a-4031-bbd7-3775e458e496 --- ...al_fit_decoder_buffer_allocator_strategy.h | 9 ++-- media/starboard/decoder_buffer_allocator.cc | 23 ++++++--- media/starboard/decoder_buffer_allocator.h | 5 +- ..._pool_decoder_buffer_allocator_strategy.cc | 3 +- media/starboard/starboard_memory_allocator.h | 28 ++++++----- .../starboard_memory_allocator_test.cc | 8 ++-- .../cobalt/h5vcc_settings/h_5_vcc_settings.cc | 47 ++++++++++++------- 7 files changed, 79 insertions(+), 44 deletions(-) diff --git a/media/starboard/bidirectional_fit_decoder_buffer_allocator_strategy.h b/media/starboard/bidirectional_fit_decoder_buffer_allocator_strategy.h index 7678881bea44..9b45f38147ce 100644 --- a/media/starboard/bidirectional_fit_decoder_buffer_allocator_strategy.h +++ b/media/starboard/bidirectional_fit_decoder_buffer_allocator_strategy.h @@ -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, @@ -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, diff --git a/media/starboard/decoder_buffer_allocator.cc b/media/starboard/decoder_buffer_allocator.cc index 6ef18a033b02..2b7cb6c2848c 100644 --- a/media/starboard/decoder_buffer_allocator.cc +++ b/media/starboard/decoder_buffer_allocator.cc @@ -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(); @@ -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); + } 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 { LOG(INFO) << "DecoderBufferAllocator is using " @@ -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( 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 diff --git a/media/starboard/decoder_buffer_allocator.h b/media/starboard/decoder_buffer_allocator.h index d28a7edb0ffa..1a3c2b3ef288 100644 --- a/media/starboard/decoder_buffer_allocator.h +++ b/media/starboard/decoder_buffer_allocator.h @@ -15,6 +15,7 @@ #ifndef MEDIA_STARBOARD_DECODER_BUFFER_ALLOCATOR_H_ #define MEDIA_STARBOARD_DECODER_BUFFER_ALLOCATOR_H_ +#include #include #include @@ -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(); @@ -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 decommit_on_suspend_enabled_{false}; }; } // namespace media diff --git a/media/starboard/media_buffer_pool_decoder_buffer_allocator_strategy.cc b/media/starboard/media_buffer_pool_decoder_buffer_allocator_strategy.cc index 147bd43edf5b..350602a70b65 100644 --- a/media/starboard/media_buffer_pool_decoder_buffer_allocator_strategy.cc +++ b/media/starboard/media_buffer_pool_decoder_buffer_allocator_strategy.cc @@ -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 diff --git a/media/starboard/starboard_memory_allocator.h b/media/starboard/starboard_memory_allocator.h index 1442bb40e919..af29c10bc4e7 100644 --- a/media/starboard/starboard_memory_allocator.h +++ b/media/starboard/starboard_memory_allocator.h @@ -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(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 { @@ -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_); } @@ -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(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(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); } } @@ -102,6 +105,7 @@ class StarboardMemoryAllocator : public starboard::Allocator { private: const bool enable_decommit_; + const bool enable_page_alignment_; const size_t page_size_; }; diff --git a/media/starboard/starboard_memory_allocator_test.cc b/media/starboard/starboard_memory_allocator_test.cc index 29ee21b9aea3..add5e6de4714 100644 --- a/media/starboard/starboard_memory_allocator_test.cc +++ b/media/starboard/starboard_memory_allocator_test.cc @@ -24,7 +24,7 @@ namespace { class StarboardMemoryAllocatorTest : public ::testing::TestWithParam { protected: - StarboardMemoryAllocatorTest() : allocator_(GetParam()) {} + StarboardMemoryAllocatorTest() : allocator_(GetParam(), GetParam()) {} StarboardMemoryAllocator allocator_; }; @@ -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 @@ -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; diff --git a/third_party/blink/renderer/modules/cobalt/h5vcc_settings/h_5_vcc_settings.cc b/third_party/blink/renderer/modules/cobalt/h5vcc_settings/h_5_vcc_settings.cc index 6bd719736f83..71d98bebde47 100644 --- a/third_party/blink/renderer/modules/cobalt/h5vcc_settings/h_5_vcc_settings.cc +++ b/third_party/blink/renderer/modules/cobalt/h5vcc_settings/h_5_vcc_settings.cc @@ -149,23 +149,27 @@ ScriptPromise 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. + // - 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 @@ -179,18 +183,25 @@ ScriptPromise 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( 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; 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") {