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") {