Skip to content

Commit 72800b2

Browse files
committed
Update base for Update on "[Executorch] Make module constructors uniform across"
Existing constructors dont compose well such that if you want data loader or data files constructor then you cannot get to override memory allocator. Fix that. Differential Revision: [D86120037](https://our.internmc.facebook.com/intern/diff/D86120037/) [ghstack-poisoned]
1 parent 7bcaa43 commit 72800b2

File tree

6 files changed

+66
-23
lines changed

6 files changed

+66
-23
lines changed

extension/memory_allocator/cpu_caching_malloc_allocator.cpp

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include <cstdlib>
22

33
#include <executorch/extension/memory_allocator/cpu_caching_malloc_allocator.h>
4+
#include <executorch/extension/memory_allocator/memory_allocator_utils.h>
45

56
namespace executorch::extension {
67

@@ -18,6 +19,11 @@ void* CPUCachingAllocator::allocate(size_t size, size_t alignment) {
1819
return nullptr;
1920
}
2021
alignment = std::max(alignment, kCachingAllocatorDefaultAlignment);
22+
size_t adjusted_size = executorch::extension::utils::get_aligned_size(size, alignment);
23+
if (adjusted_size == 0) {
24+
return nullptr;
25+
}
26+
size = adjusted_size;
2127

2228
std::lock_guard<std::mutex> guard(mutex_);
2329
const auto& it = available_map_.find(size);
@@ -26,14 +32,7 @@ void* CPUCachingAllocator::allocate(size_t size, size_t alignment) {
2632
// 2. Allocate new memory
2733
// 2 can lead to current_size > max_size_
2834
if (it == available_map_.end() || it->second.empty()) {
29-
void* ptr = nullptr;
30-
#if defined(__ANDROID__)
31-
ptr = memalign(alignment, size);
32-
#elif defined(_MSC_VER)
33-
ptr = _aligned_malloc(size, alignment);
34-
#else
35-
ptr = std::aligned_alloc(alignment, size);
36-
#endif
35+
void* ptr = std::malloc(size);
3736
if (ptr == nullptr) {
3837
ET_LOG(Error, "Failed to allocate memory");
3938
return nullptr;

extension/memory_allocator/cpu_caching_malloc_allocator.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,13 @@ class CPUCachingAllocator : public executorch::runtime::MemoryAllocator {
7070
/*
7171
max_size: Maximum size of memory to cache. Never cache more than that.
7272
*/
73-
CPUCachingAllocator(uint32_t max_size);
73+
explicit CPUCachingAllocator(uint32_t max_size);
74+
// No copies allowed
75+
CPUCachingAllocator(const CPUCachingAllocator&) = delete;
76+
CPUCachingAllocator& operator=(const CPUCachingAllocator&) = delete;
77+
// No moves allowed
78+
CPUCachingAllocator(CPUCachingAllocator&&) = delete;
79+
CPUCachingAllocator& operator=(CPUCachingAllocator&&) = delete;
7480
// Checks the cache to see if allocation of size bytes can be found.
7581
// If so return cached memory, else
7682
// allocates memory, records it for caching and returns.

extension/memory_allocator/malloc_memory_allocator.h

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include <cstdlib>
1414
#include <vector>
1515

16+
#include <executorch/extension/memory_allocator/memory_allocator_utils.h>
1617
#include <executorch/runtime/core/memory_allocator.h>
1718

1819
namespace executorch {
@@ -51,20 +52,11 @@ class MallocMemoryAllocator : public executorch::runtime::MemoryAllocator {
5152
return nullptr;
5253
}
5354

54-
// The minimum alignment that malloc() is guaranteed to provide.
55-
static constexpr size_t kMallocAlignment = alignof(std::max_align_t);
56-
if (alignment > kMallocAlignment) {
57-
// To get higher alignments, allocate extra and then align the returned
58-
// pointer. This will waste an extra `alignment - 1` bytes every time, but
59-
// this is the only portable way to get aligned memory from the heap.
60-
const size_t extra = alignment - 1;
61-
if ET_UNLIKELY (extra >= SIZE_MAX - size) {
62-
ET_LOG(
63-
Error, "Malloc size overflow: size=%zu + extra=%zu", size, extra);
64-
return nullptr;
65-
}
66-
size += extra;
55+
size_t adjusted_size = executorch::extension::utils::get_aligned_size(size, alignment);
56+
if (adjusted_size == 0) {
57+
return nullptr;
6758
}
59+
size = adjusted_size;
6860
void* mem_ptr = std::malloc(size);
6961
if (!mem_ptr) {
7062
ET_LOG(Error, "Malloc failed to allocate %zu bytes", size);
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
#pragma once
10+
11+
#include <cstddef>
12+
#include <cstdint>
13+
#include <cstdlib>
14+
15+
#include <executorch/runtime/core/error.h>
16+
#include <executorch/runtime/platform/compiler.h>
17+
18+
namespace executorch::extension::utils {
19+
20+
// Util to get alighment adjusted allocation size
21+
inline size_t get_aligned_size(size_t size, size_t alignment) {
22+
// The minimum alignment that malloc() is guaranteed to provide.
23+
static constexpr size_t kMallocAlignment = alignof(std::max_align_t);
24+
if (alignment > kMallocAlignment) {
25+
// To get higher alignments, allocate extra and then align the returned
26+
// pointer. This will waste an extra `alignment - 1` bytes every time, but
27+
// this is the only portable way to get aligned memory from the heap.
28+
const size_t extra = alignment - 1;
29+
if ET_UNLIKELY (extra >= SIZE_MAX - size) {
30+
ET_LOG(
31+
Error, "Malloc size overflow: size=%zu + extra=%zu", size, extra);
32+
return 0;
33+
}
34+
size += extra;
35+
}
36+
return size;
37+
}
38+
39+
} // namespace executorch::extension::utils

extension/memory_allocator/targets.bzl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ def define_common_targets():
99

1010
runtime.cxx_library(
1111
name = "malloc_memory_allocator",
12+
headers = [
13+
"memory_allocator_utils.h",
14+
],
1215
exported_headers = [
1316
"malloc_memory_allocator.h",
1417
],
@@ -23,6 +26,9 @@ def define_common_targets():
2326

2427
runtime.cxx_library(
2528
name = "cpu_caching_allocator",
29+
headers = [
30+
"memory_allocator_utils.h",
31+
],
2632
srcs = [
2733
"cpu_caching_malloc_allocator.cpp",
2834
],

extension/memory_allocator/test/cpu_caching_malloc_allocator_test.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ TEST_F(CPUCachingAllocatorTest, ThreadSafety) {
241241
};
242242

243243
// Create threads
244+
threads.reserve(num_threads);
244245
for (int i = 0; i < num_threads; ++i) {
245246
threads.emplace_back(thread_work);
246247
}
@@ -284,7 +285,7 @@ TEST_F(CPUCachingAllocatorTest, SizeAlignmentAdjustment) {
284285
EXPECT_NE(p1, nullptr);
285286
EXPECT_ALIGNED(p1, 256);
286287

287-
auto p2 = allocator.allocate(100, 256);
288+
allocator.allocate(100, 256);
288289
// Should not get cached pointer since size was adjusted during first
289290
// allocation
290291
allocator.reset();

0 commit comments

Comments
 (0)