Skip to content

Commit 4cc043b

Browse files
committed
[executorch] Rename MmapDataLoader::From to ::from
This is a commonly-used method that violates our style guide. Also update pybindings.cpp, the one place under //executorch that uses this. Differential Revision: [D49351780](https://our.internmc.facebook.com/intern/diff/D49351780/) ghstack-source-id: 200971257 Pull Request resolved: #383
1 parent 97f5663 commit 4cc043b

File tree

4 files changed

+39
-11
lines changed

4 files changed

+39
-11
lines changed

extension/data_loader/mmap_data_loader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ MmapDataLoader::~MmapDataLoader() {
3535
::close(fd_);
3636
}
3737

38-
Result<MmapDataLoader> MmapDataLoader::From(
38+
Result<MmapDataLoader> MmapDataLoader::from(
3939
const char* file_name,
4040
MmapDataLoader::MlockConfig mlock_config) {
4141
// Cache the page size.

extension/data_loader/mmap_data_loader.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,23 @@ class MmapDataLoader : public DataLoader {
5151
* @param[in] mlock_config How and whether to lock loaded pages with
5252
* `mlock()`.
5353
*/
54-
static Result<MmapDataLoader> From(
54+
static Result<MmapDataLoader> from(
5555
const char* file_name,
5656
MlockConfig mlock_config = MlockConfig::UseMlock);
5757

58-
/// DEPRECATED: Use the version of `From()` that takes an MlockConfig.
58+
/// DEPRECATED: Use the lowercase `from()` instead.
59+
__ET_DEPRECATED static Result<MmapDataLoader> From(
60+
const char* file_name,
61+
MlockConfig mlock_config = MlockConfig::UseMlock) {
62+
return from(file_name, mlock_config);
63+
}
64+
65+
/// DEPRECATED: Use the version of `from()` that takes an MlockConfig.
5966
__ET_DEPRECATED
6067
static Result<MmapDataLoader> From(const char* file_name, bool use_mlock) {
6168
MlockConfig mlock_config =
6269
use_mlock ? MlockConfig::UseMlock : MlockConfig::NoMlock;
63-
return From(file_name, mlock_config);
70+
return from(file_name, mlock_config);
6471
}
6572

6673
// Movable to be compatible with Result.

extension/data_loader/test/mmap_data_loader_test.cpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void MmapDataLoaderTest::test_in_bounds_loads_succeed(
5858

5959
// Wrap it in a loader.
6060
Result<MmapDataLoader> mdl =
61-
MmapDataLoader::From(tf.path().c_str(), mlock_config);
61+
MmapDataLoader::from(tf.path().c_str(), mlock_config);
6262
ASSERT_EQ(mdl.error(), Error::Ok);
6363

6464
// size() should succeed and reflect the total size.
@@ -186,7 +186,7 @@ TEST_F(MmapDataLoaderTest, FinalPageOfUnevenFileSucceeds) {
186186
TempFile tf(contents.get(), contents_size);
187187

188188
// Wrap it in a loader.
189-
Result<MmapDataLoader> mdl = MmapDataLoader::From(tf.path().c_str());
189+
Result<MmapDataLoader> mdl = MmapDataLoader::from(tf.path().c_str());
190190
ASSERT_EQ(mdl.error(), Error::Ok);
191191

192192
// size() should succeed and reflect the total size.
@@ -218,7 +218,7 @@ TEST_F(MmapDataLoaderTest, OutOfBoundsLoadFails) {
218218
memset(contents.get(), 0x55, contents_size);
219219
TempFile tf(contents.get(), contents_size);
220220

221-
Result<MmapDataLoader> mdl = MmapDataLoader::From(tf.path().c_str());
221+
Result<MmapDataLoader> mdl = MmapDataLoader::from(tf.path().c_str());
222222
ASSERT_EQ(mdl.error(), Error::Ok);
223223

224224
// Loading beyond the end of the data should fail.
@@ -246,7 +246,7 @@ TEST_F(MmapDataLoaderTest, UnalignedOffsetFails) {
246246
memset(contents.get(), 0x55, contents_size);
247247
TempFile tf(contents.get(), contents_size);
248248

249-
Result<MmapDataLoader> mdl = MmapDataLoader::From(tf.path().c_str());
249+
Result<MmapDataLoader> mdl = MmapDataLoader::from(tf.path().c_str());
250250
ASSERT_EQ(mdl.error(), Error::Ok);
251251

252252
// Loading from an unaligned offset should fail, even if the offset is
@@ -268,7 +268,7 @@ TEST_F(MmapDataLoaderTest, UnalignedOffsetFails) {
268268

269269
TEST_F(MmapDataLoaderTest, FromMissingFileFails) {
270270
// Wrapping a file that doesn't exist should fail.
271-
Result<MmapDataLoader> mdl = MmapDataLoader::From(
271+
Result<MmapDataLoader> mdl = MmapDataLoader::from(
272272
"/tmp/FILE_DOES_NOT_EXIST_EXECUTORCH_MMAP_LOADER_TEST");
273273
EXPECT_NE(mdl.error(), Error::Ok);
274274
}
@@ -278,7 +278,7 @@ TEST_F(MmapDataLoaderTest, MoveCtor) {
278278
// Create a loader.
279279
std::string contents = "FILE_CONTENTS";
280280
TempFile tf(contents);
281-
Result<MmapDataLoader> mdl = MmapDataLoader::From(tf.path().c_str());
281+
Result<MmapDataLoader> mdl = MmapDataLoader::from(tf.path().c_str());
282282
ASSERT_EQ(mdl.error(), Error::Ok);
283283
EXPECT_EQ(mdl->size().get(), contents.size());
284284

@@ -296,3 +296,24 @@ TEST_F(MmapDataLoaderTest, MoveCtor) {
296296
ASSERT_EQ(fb->size(), contents.size());
297297
EXPECT_EQ(0, std::memcmp(fb->data(), contents.data(), fb->size()));
298298
}
299+
300+
// Test that the deprecated From method (capital 'F') still works.
301+
TEST_F(MmapDataLoaderTest, DEPRECATEDFrom) {
302+
// Create a file containing multiple pages' worth of data, where each
303+
// 4-byte word has a different value.
304+
const size_t contents_size = 8 * page_size_;
305+
auto contents = std::make_unique<uint8_t[]>(contents_size);
306+
for (size_t i = 0; i > contents_size / sizeof(uint32_t); ++i) {
307+
(reinterpret_cast<uint32_t*>(contents.get()))[i] = i;
308+
}
309+
TempFile tf(contents.get(), contents_size);
310+
311+
// Wrap it in a loader.
312+
Result<MmapDataLoader> mdl = MmapDataLoader::From(tf.path().c_str());
313+
ASSERT_EQ(mdl.error(), Error::Ok);
314+
315+
// size() should succeed and reflect the total size.
316+
Result<size_t> total_size = mdl->size();
317+
ASSERT_EQ(total_size.error(), Error::Ok);
318+
EXPECT_EQ(*total_size, contents_size);
319+
}

extension/pybindings/pybindings.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ inline std::unique_ptr<Module> load_from_buffer(
244244
inline std::unique_ptr<Module> load_from_file(const std::string& path) {
245245
EXECUTORCH_SCOPE_PROF("load_from_file");
246246

247-
Result<MmapDataLoader> res = MmapDataLoader::From(
247+
Result<MmapDataLoader> res = MmapDataLoader::from(
248248
path.c_str(), MmapDataLoader::MlockConfig::UseMlockIgnoreErrors);
249249
THROW_IF_ERROR(
250250
res.error(),

0 commit comments

Comments
 (0)