Skip to content

GitLab

T
tangled
Commit 55b5a578 authored by pliard@chromium.org's avatar pliard@chromium.org
Browse files
Options

Align ashmem region sizes to page size after ashmem creation failed. ... 

Align ashmem region sizes to page size after ashmem creation failed.                                                                                                                                                                                                                                                           

When ashmem creation failed the new size (twice smaller) used to retry wasn't
page-aligned.

This also moves the page alignment function to the allocator's anonymous
namespace since it's now an implementation detail (since the allocator is
always used).

BUG=331832

Review URL: https://codereview.chromium.org/136723005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245201 0039d316-1c4b-4281-b951-d872f2087c98
parent 058548ba
Hide whitespace changes
Inline Side-by-side
Showing with 48 additions and 25 deletions
base/memory/discardable_memory_allocator_android.cc
View file @ 55b5a578
......@@ -46,6 +46,16 @@ const size_t kMaxChunkFragmentationBytes = 4096 - 1;
const size_t kMinAshmemRegionSize = 32 * 1024 * 1024;
// Returns 0 if the provided size is too high to be aligned.
size_t AlignToNextPage(size_t size) {
const size_t kPageSize = 4096;
DCHECK_EQ(static_cast<int>(kPageSize), getpagesize());
if (size > std::numeric_limits<size_t>::max() - kPageSize + 1)
return 0;
const size_t mask = ~(kPageSize - 1);
return (size + kPageSize - 1) & mask;
}
} // namespace
namespace internal {
......@@ -106,7 +116,7 @@ class DiscardableMemoryAllocator::AshmemRegion {
size_t size,
const std::string& name,
DiscardableMemoryAllocator* allocator) {
DCHECK_EQ(size, internal::AlignToNextPage(size));
DCHECK_EQ(size, AlignToNextPage(size));
int fd;
void* base;
if (!internal::CreateAshmemRegion(name.c_str(), size, &fd, &base))
......@@ -371,7 +381,9 @@ DiscardableMemoryAllocator::DiscardableMemoryAllocator(
const std::string& name,
size_t ashmem_region_size)
: name_(name),
ashmem_region_size_(std::max(kMinAshmemRegionSize, ashmem_region_size)) {
ashmem_region_size_(
std::max(kMinAshmemRegionSize, AlignToNextPage(ashmem_region_size))),
last_ashmem_region_size_(0) {
DCHECK_GE(ashmem_region_size_, kMinAshmemRegionSize);
}
......@@ -382,7 +394,7 @@ DiscardableMemoryAllocator::~DiscardableMemoryAllocator() {
scoped_ptr<DiscardableMemory> DiscardableMemoryAllocator::Allocate(
size_t size) {
const size_t aligned_size = internal::AlignToNextPage(size);
const size_t aligned_size = AlignToNextPage(size);
if (!aligned_size)
return scoped_ptr<DiscardableMemory>();
// TODO(pliard): make this function less naive by e.g. moving the free chunks
......@@ -403,11 +415,13 @@ scoped_ptr<DiscardableMemory> DiscardableMemoryAllocator::Allocate(
// repetitively dividing the size by 2.
const size_t min_region_size = std::max(kMinAshmemRegionSize, aligned_size);
for (size_t region_size = std::max(ashmem_region_size_, aligned_size);
region_size >= min_region_size; region_size /= 2) {
region_size >= min_region_size;
region_size = AlignToNextPage(region_size / 2)) {
scoped_ptr<AshmemRegion> new_region(
AshmemRegion::Create(region_size, name_.c_str(), this));
if (!new_region)
continue;
last_ashmem_region_size_ = region_size;
ashmem_regions_.push_back(new_region.release());
return ashmem_regions_.back()->Allocate_Locked(size, aligned_size);
}
......@@ -415,6 +429,11 @@ scoped_ptr<DiscardableMemory> DiscardableMemoryAllocator::Allocate(
return scoped_ptr<DiscardableMemory>();
}
size_t DiscardableMemoryAllocator::last_ashmem_region_size() const {
AutoLock auto_lock(lock_);
return last_ashmem_region_size_;
}
void DiscardableMemoryAllocator::DeleteAshmemRegion_Locked(
AshmemRegion* region) {
lock_.AssertAcquired();
......
base/memory/discardable_memory_allocator_android.h
View file @ 55b5a578
......@@ -44,16 +44,21 @@ class BASE_EXPORT_PRIVATE DiscardableMemoryAllocator {
// instance.
scoped_ptr<DiscardableMemory> Allocate(size_t size);
// Returns the size of the last ashmem region which was created. This is used
// for testing only.
size_t last_ashmem_region_size() const;
private:
class AshmemRegion;
class DiscardableAshmemChunk;
void DeleteAshmemRegion_Locked(AshmemRegion* region);
base::ThreadChecker thread_checker_;
ThreadChecker thread_checker_;
const std::string name_;
const size_t ashmem_region_size_;
base::Lock lock_;
mutable Lock lock_;
size_t last_ashmem_region_size_;
ScopedVector<AshmemRegion> ashmem_regions_;
DISALLOW_COPY_AND_ASSIGN(DiscardableMemoryAllocator);
......
base/memory/discardable_memory_allocator_android_unittest.cc
View file @ 55b5a578
......@@ -23,6 +23,9 @@ const char kAllocatorName[] = "allocator-for-testing";
const size_t kAshmemRegionSizeForTesting = 32 * 1024 * 1024;
const size_t kPageSize = 4096;
const size_t kMaxAllowedAllocationSize =
std::numeric_limits<size_t>::max() - kPageSize + 1;
class DiscardableMemoryAllocatorTest : public testing::Test {
protected:
DiscardableMemoryAllocatorTest()
......@@ -52,10 +55,8 @@ TEST_F(DiscardableMemoryAllocatorTest, ZeroAllocationIsNotSupported) {
}
TEST_F(DiscardableMemoryAllocatorTest, TooLargeAllocationFails) {
const size_t max_allowed_allocation_size =
std::numeric_limits<size_t>::max() - kPageSize + 1;
scoped_ptr<DiscardableMemory> memory(
allocator_.Allocate(max_allowed_allocation_size + 1));
allocator_.Allocate(kMaxAllowedAllocationSize + 1));
// Page-alignment would have caused an overflow resulting in a small
// allocation if the input size wasn't checked correctly.
ASSERT_FALSE(memory);
......@@ -63,17 +64,28 @@ TEST_F(DiscardableMemoryAllocatorTest, TooLargeAllocationFails) {
TEST_F(DiscardableMemoryAllocatorTest,
AshmemRegionsAreNotSmallerThanRequestedSize) {
const size_t size = std::numeric_limits<size_t>::max() - kPageSize + 1;
// The creation of the underlying ashmem region is expected to fail since
// there should not be enough room in the address space. When ashmem creation
// fails, the allocator repetitively retries by dividing the size by 2. This
// size should not be smaller than the size the user requested so the
// allocation here should just fail (and not succeed with the minimum ashmem
// region size).
scoped_ptr<DiscardableMemory> memory(allocator_.Allocate(size));
scoped_ptr<DiscardableMemory> memory(
allocator_.Allocate(kMaxAllowedAllocationSize));
ASSERT_FALSE(memory);
}
TEST_F(DiscardableMemoryAllocatorTest, AshmemRegionsAreAlwaysPageAligned) {
// Use a separate allocator here so that we can override the ashmem region
// size.
DiscardableMemoryAllocator allocator(
kAllocatorName, kMaxAllowedAllocationSize);
scoped_ptr<DiscardableMemory> memory(allocator.Allocate(kPageSize));
ASSERT_TRUE(memory);
EXPECT_GT(kMaxAllowedAllocationSize, allocator.last_ashmem_region_size());
ASSERT_TRUE(allocator.last_ashmem_region_size() % kPageSize == 0);
}
TEST_F(DiscardableMemoryAllocatorTest, LargeAllocation) {
// Note that large allocations should just use DiscardableMemoryAndroidSimple
// instead.
......
base/memory/discardable_memory_android.cc
View file @ 55b5a578
......@@ -37,8 +37,7 @@ struct DiscardableMemoryAllocatorWrapper {
static size_t GetOptimalAshmemRegionSizeForAllocator() {
// Note that this may do some I/O (without hitting the disk though) so it
// should not be called on the critical path.
return internal::AlignToNextPage(
base::android::SysUtils::AmountOfPhysicalMemoryKB() * 1024 / 8);
return base::android::SysUtils::AmountOfPhysicalMemoryKB() * 1024 / 8;
}
};
......@@ -49,15 +48,6 @@ LazyInstance<DiscardableMemoryAllocatorWrapper>::Leaky g_context =
namespace internal {
size_t AlignToNextPage(size_t size) {
const size_t kPageSize = 4096;
DCHECK_EQ(static_cast<int>(kPageSize), getpagesize());
if (size > std::numeric_limits<size_t>::max() - kPageSize + 1)
return 0;
const size_t mask = ~(kPageSize - 1);
return (size + kPageSize - 1) & mask;
}
bool CreateAshmemRegion(const char* name,
size_t size,
int* out_fd,
......
base/memory/discardable_memory_android.h
View file @ 55b5a578
......@@ -15,9 +15,6 @@
namespace base {
namespace internal {
// Returns 0 if the provided size is too high to be aligned.
size_t AlignToNextPage(size_t size);
bool CreateAshmemRegion(const char* name, size_t size, int* fd, void** address);
bool CloseAshmemRegion(int fd, size_t size, void* address);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment