Skip to content

Commit 5762802

Browse files
dbortfacebook-github-bot
authored andcommitted
Adjust MethodMeta's non-const indices by 1, and update all users (#315)
Summary: Pull Request resolved: #315 First step in hiding the off-by-one non-const buffer indices from users. Since fewer users use `MethodMeta::num_non_const_buffers` today (relative to the version on Program), update the behavior here, along with all of its users. After this, we can migrate the users of `Program::get_non_const_buffer_size` to this new API, fixing the off-by-one as we go. I found all of these uses with the search ``` cd fbsource xbgs -sl "non_const_buffer_size" | xargs grep -le '\<non_const_buffer_size\>' ``` Luckily, `Program`'s version of this is called `get_non_const_buffer_size` (with the extra `get_` prefix) so it was straightforward to find code that used `MethodMeta`'s methods. Reviewed By: JacobSzwejbka Differential Revision: D48762563 fbshipit-source-id: bd5b1b7dc7702cf0192261b535d59e8ba80b96e4
1 parent 1f558c7 commit 5762802

File tree

4 files changed

+15
-12
lines changed

4 files changed

+15
-12
lines changed

exir/backend/test/demos/rpc/ExecutorBackend.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,19 @@ class ExecutorBackend final : public PyTorchBackendInterface {
7777
runtime_allocator, MemoryAllocator);
7878
new (client_const_allocator) MemoryAllocator(0, nullptr);
7979

80-
auto num_buffers = method_meta->num_non_const_buffers();
81-
size_t num_non_const_buffers = num_buffers - 1;
80+
auto num_non_const_buffers = method_meta->num_non_const_buffers();
8281

8382
uint8_t** non_const_buffers = ET_ALLOCATE_LIST_OR_RETURN_ERROR(
8483
runtime_allocator, uint8_t*, num_non_const_buffers);
8584
MemoryAllocator* non_const_allocators = ET_ALLOCATE_LIST_OR_RETURN_ERROR(
8685
runtime_allocator, MemoryAllocator, num_non_const_buffers);
8786

88-
for (size_t id = 1; id < num_buffers; ++id) {
87+
for (size_t id = 0; id < num_non_const_buffers; ++id) {
8988
auto buffer_size = method_meta->non_const_buffer_size(id);
9089
uint8_t* buffer_i = ET_ALLOCATE_LIST_OR_RETURN_ERROR(
9190
runtime_allocator, uint8_t, buffer_size.get());
92-
non_const_buffers[id - 1] = buffer_i;
93-
new (&non_const_allocators[id - 1])
91+
non_const_buffers[id] = buffer_i;
92+
new (&non_const_allocators[id])
9493
MemoryAllocator(static_cast<uint32_t>(buffer_size.get()), buffer_i);
9594
}
9695

extension/pybindings/pybindings.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,7 @@ class Module final {
8383
for (size_t i = 0; i < program_->num_methods(); ++i) {
8484
auto name = program_->get_method_name(i).get();
8585
auto method_meta = program_->method_meta(name).get();
86-
// 1 on purpose because non-const are 1 indexed
87-
for (size_t j = 1; j < method_meta.num_non_const_buffers(); j++) {
86+
for (size_t j = 0; j < method_meta.num_non_const_buffers(); j++) {
8887
int64_t buffer_size = method_meta.non_const_buffer_size(j).get();
8988
if (non_const_buffer_sizes.find(j) == non_const_buffer_sizes.end()) {
9089
non_const_buffer_sizes.insert({j, buffer_size});

runtime/executor/method_meta.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,10 @@ Result<TensorInfo> MethodMeta::output_tensor_meta(size_t index) const {
170170
}
171171

172172
size_t MethodMeta::num_non_const_buffers() const {
173-
return s_plan_->non_const_buffer_sizes()->size();
173+
// Index zero is reserved internally, and we hide it from users. The actual
174+
// number of buffers is one fewer than the actual size of this list in the
175+
// program.
176+
return s_plan_->non_const_buffer_sizes()->size() - 1;
174177
}
175178

176179
Result<int64_t> MethodMeta::non_const_buffer_size(size_t index) const {
@@ -181,7 +184,9 @@ Result<int64_t> MethodMeta::non_const_buffer_size(size_t index) const {
181184
"index %zu out of range. num_buffers: %zu",
182185
index,
183186
num_buffers);
184-
return s_plan_->non_const_buffer_sizes()->Get(index);
187+
// Index zero is reserved internally, and we hide it from users. Adjust the
188+
// provided index to point to one of the actual buffers.
189+
return s_plan_->non_const_buffer_sizes()->Get(index + 1);
185190
}
186191

187192
} // namespace executor

runtime/executor/test/method_meta_test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,14 @@ TEST_F(MethodMetaTest, MethodMetaApi) {
7676
EXPECT_EQ(method_meta->num_outputs(), 1);
7777

7878
// Appropriate amount of non_const_buffers
79-
EXPECT_EQ(method_meta->num_non_const_buffers(), 2);
79+
EXPECT_EQ(method_meta->num_non_const_buffers(), 1);
8080

8181
// Appropriate content of non_const_buffers
82-
EXPECT_EQ(method_meta->non_const_buffer_size(1).get(), 48);
82+
EXPECT_EQ(method_meta->non_const_buffer_size(0).get(), 48);
8383

8484
// Invalid index Errors
8585
EXPECT_EQ(
86-
method_meta->non_const_buffer_size(2).error(), Error::InvalidArgument);
86+
method_meta->non_const_buffer_size(1).error(), Error::InvalidArgument);
8787

8888
EXPECT_EQ(
8989
program_->method_meta("not_a_method").error(), Error::InvalidArgument);

0 commit comments

Comments
 (0)