Skip to content

Commit 14be5a9

Browse files
lucylqfacebook-github-bot
authored andcommitted
Back FreeableBuffer with int64_t (#14570)
Summary: This diff backs FreeableBuffer with an int64_t, instead of a void* ## Usecase PTE file lives on a 32-bit system, where void* is 4 bytes. PTD file lives on a 36-bit system, which requires int64_t to address it. We want to fetch addresses from the ptd file and pass them to accelerator. Executorch APIs return a FreeableBuffer when fetching data (data_loader.h, named_data_map.h). FreeableBuffer is currently backed by a void*, which could truncate a 36-bit address, when on a 32-bit system. Note that we still want existing void* behavior to load segments etc., and only want int64_t behavior when fetching weights from the named_data_map. ## Potential concerns * Increased memory usage; additional 4 bytes for each FreeableBuffer + some extra for the std::variant template. For the PTE file, this is order of the number of segments, which is usually small. * Increased runtime latency; calls to the existing void* API perform truncation checks now. ## Alternatives Why we choose this solution? Seems to be the least intrusive out of a number of solutions. 1. Compiler macro to switch the backing value of FreeableBuffer to int64_t for specific builds. However void* and int64_ are both required - void* for the existing use cases (fetching delegate blobs). Both APIs must exist. 2. Template FreeableBuffer on void* and int64_t. Messy, as the template is contagious; it will need to be applied to data_loader.h, named_data_map.h, and potentially program/method. Imagine this will bloat the core runtime with templating. 3. Store int64_t addresses in FreeableBuffer, and ask user to parse the address to load the data. This avoids changing FreeableBuffer, but is not semantically correct as the API is not returning a buffer of data, but an address that the user must parse and then load data from. 4. Add a specific API to named_data_map.h that returns an int64_t buffer. Not a good use of API surface. https://docs.google.com/document/d/11dMXh1N66rfY-8aO3N-ra2dDPx0RGCoc1rlCSN80Zf8/edit?tab=t.0 Reviewed By: swolchok Differential Revision: D83007972
1 parent 0e74a17 commit 14be5a9

File tree

2 files changed

+246
-21
lines changed

2 files changed

+246
-21
lines changed

runtime/core/freeable_buffer.h

Lines changed: 112 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
#pragma once
1010

1111
#include <cstddef>
12+
#include <cstdint>
13+
#include <variant>
14+
15+
#include <executorch/runtime/core/error.h>
16+
#include <executorch/runtime/core/result.h>
17+
#include <executorch/runtime/platform/assert.h>
1218

1319
namespace executorch {
1420
namespace runtime {
@@ -20,20 +26,35 @@ class FreeableBuffer final {
2026
public:
2127
// Callback signature for the function that does the freeing.
2228
using FreeFn = void (*)(void* context, void* data, size_t size);
29+
using FreeUInt64Fn =
30+
void (*)(void* context, uint64_t data_uint64, size_t size);
31+
32+
private:
33+
// Forward declare types.
34+
struct PointerData {
35+
const void* data_;
36+
FreeFn free_fn_;
37+
};
2338

39+
struct UInt64Data {
40+
// A pointer value cast to uint64_t.
41+
uint64_t data_;
42+
FreeUInt64Fn free_fn_;
43+
};
44+
45+
public:
2446
/**
2547
* Creates an empty FreeableBuffer with size zero and a null data pointer.
2648
*/
2749
FreeableBuffer()
28-
: free_fn_(nullptr),
50+
: data_(PointerData{nullptr, nullptr}),
2951
free_fn_context_(nullptr),
30-
data_(nullptr),
3152
size_(0) {}
3253

3354
/**
3455
* Creates a FreeableBuffer with an optional free function.
3556
*
36-
* @param[in] data The data of the segment.
57+
* @param[in] data The data of the segment, as a void*.
3758
* @param[in] size The size of the segment data, in bytes.
3859
* @param[in] free_fn Optional function to free the data. Guaranteed to be
3960
* called exactly once before the FreeableBuffer is destroyed. May be
@@ -47,23 +68,51 @@ class FreeableBuffer final {
4768
size_t size,
4869
FreeFn free_fn,
4970
void* free_fn_context = nullptr)
50-
: free_fn_(free_fn),
71+
: data_(PointerData{data, free_fn}),
72+
free_fn_context_(free_fn_context),
73+
size_(size) {}
74+
75+
/**
76+
* Creates a FreeableBuffer with an optional free function.
77+
*
78+
* NOTE: most users should use the other ctor with FreeFn.
79+
* This variant exists for situations where the FreeableBuffer points to
80+
* memory on a different core whose pointer value is larger than the local
81+
* core's void*.
82+
*
83+
* @param[in] data Pointer to the data of the segment, cast to a uint64_t
84+
* value.
85+
* @param[in] size The size of the segment data, in bytes.
86+
* @param[in] free_fn Optional function to free the data. Guaranteed to be
87+
* called exactly once before the FreeableBuffer is destroyed. May be
88+
* nullptr. NOTE: This function must be thread-safe. If it modifies common
89+
* state, the function must do its own locking.
90+
* @param[in] free_fn_context Opaque pointer to pass as the `context`
91+
* parameter of `free_fn`. May be nullptr.
92+
*/
93+
explicit FreeableBuffer(
94+
const uint64_t data_uint64,
95+
size_t size,
96+
FreeUInt64Fn free_fn,
97+
void* free_fn_context = nullptr)
98+
: data_(UInt64Data{data_uint64, free_fn}),
5199
free_fn_context_(free_fn_context),
52-
data_(data),
53100
size_(size) {}
54101

55102
/**
56103
* Move ctor. Takes the ownership of the data previously owned by `rhs`,
57104
* leaving `rhs` pointing to nullptr.
58105
*/
59106
FreeableBuffer(FreeableBuffer&& rhs) noexcept
60-
: free_fn_(rhs.free_fn_),
107+
: data_(rhs.data_),
61108
free_fn_context_(rhs.free_fn_context_),
62-
data_(rhs.data_),
63109
size_(rhs.size_) {
64-
rhs.free_fn_ = nullptr;
110+
if (std::holds_alternative<PointerData>(rhs.data_)) {
111+
rhs.data_ = PointerData{nullptr, nullptr};
112+
} else {
113+
rhs.data_ = UInt64Data{0, nullptr};
114+
}
65115
rhs.free_fn_context_ = nullptr;
66-
rhs.data_ = nullptr;
67116
rhs.size_ = 0;
68117
}
69118

@@ -75,11 +124,22 @@ class FreeableBuffer final {
75124
* Frees the data if not already free. Safe to call multiple times.
76125
*/
77126
void Free() {
78-
if (data_ != nullptr) {
79-
if (free_fn_ != nullptr) {
80-
free_fn_(free_fn_context_, const_cast<void*>(data_), size_);
127+
if (std::holds_alternative<PointerData>(data_)) {
128+
PointerData& ptr_data = std::get<PointerData>(data_);
129+
if (ptr_data.data_ != nullptr && ptr_data.free_fn_ != nullptr) {
130+
// Do not need to check for truncation here, as free_fn_ is only set
131+
// using the void* ctor.
132+
ptr_data.free_fn_(
133+
free_fn_context_, const_cast<void*>(ptr_data.data_), size_);
81134
}
82-
data_ = nullptr;
135+
ptr_data.data_ = nullptr;
136+
size_ = 0;
137+
} else {
138+
UInt64Data& int64_data = std::get<UInt64Data>(data_);
139+
if (int64_data.data_ != 0 && int64_data.free_fn_ != nullptr) {
140+
int64_data.free_fn_(free_fn_context_, int64_data.data_, size_);
141+
}
142+
int64_data.data_ = static_cast<uint64_t>(0);
83143
size_ = 0;
84144
}
85145
}
@@ -95,7 +155,37 @@ class FreeableBuffer final {
95155
* Pointer to the data. Returns nullptr if the data has been freed.
96156
*/
97157
const void* data() const {
98-
return data_;
158+
ET_CHECK_MSG(
159+
std::holds_alternative<PointerData>(data_),
160+
"FreeableBuffer is backed by an uint64_t, please use the data_uint64_type() API.");
161+
return std::get<PointerData>(data_).data_;
162+
}
163+
164+
/**
165+
* Pointer to the data. Returns nullptr if the data has been freed.
166+
* Safe version of data() API that returns an ERror if the data is
167+
* backed by int64_t instead of void*.
168+
*/
169+
Result<const void*> data_safe() const {
170+
ET_CHECK_OR_RETURN_ERROR(
171+
std::holds_alternative<PointerData>(data_),
172+
InvalidType,
173+
"FreeableBuffer is backed by an uint64_t, please use the data_uint64_type() API.");
174+
return std::get<PointerData>(data_).data_;
175+
}
176+
177+
/**
178+
* Data address as a uint64_t. Returns zero if the data has been freed.
179+
* Most users should use data(). data_uint64_type() is only helpful in
180+
* situations where the FreeableBuffer points to memory on a different core
181+
* whose pointer value is larger than the local core's void *.
182+
*/
183+
Result<uint64_t> data_uint64_type() const {
184+
ET_CHECK_OR_RETURN_ERROR(
185+
std::holds_alternative<UInt64Data>(data_),
186+
InvalidType,
187+
"FreeableBuffer is backed by a void*, please use the data() API.");
188+
return std::get<UInt64Data>(data_).data_;
99189
}
100190

101191
private:
@@ -104,9 +194,15 @@ class FreeableBuffer final {
104194
FreeableBuffer& operator=(FreeableBuffer&& rhs) noexcept = delete;
105195
FreeableBuffer& operator=(const FreeableBuffer& rhs) = delete;
106196

107-
FreeFn free_fn_;
197+
// This stores either a PointerData or a UInt64Data structure. Most users
198+
// should use the PointerData variant and the void* ctor. This creates a
199+
// FreeableBuffer backed by void*, accessed using the void* getter data().
200+
// The UInt64Data variant is only helpful in situations where the
201+
// FreeableBuffer points to memory on a different core whose pointer value
202+
// is larger than the local core's void*.
203+
std::variant<PointerData, UInt64Data> data_;
204+
108205
void* free_fn_context_;
109-
const void* data_;
110206
size_t size_;
111207
};
112208

0 commit comments

Comments
 (0)