Skip to content

Commit f2e1ee5

Browse files
[ExecuTorch] Separate extension.Module Namespaces from Aten and non-Aten
Pull Request resolved: pytorch/executorch#10576 # Context Separate `extension.Module` Namespaces to be `executorch::extension::module` and `executorch::extension::module::aten`, otherwise if a package relies on both aten and non-aten of the same implementation and the namespace is the same, there will be duplicate symbol issue like: ``` ld.lld: error: duplicate symbol: vtable for executorch::extension::Module >>> defined at {redacted}/executorch/extension/module/__module__/__stripped__/module.cpp.pic.stripped.o:(vtable for executorch::extension::Module) >>> defined at {redacted}/executorch/extension/module/__module_aten__/__stripped__/module.cpp.pic.stripped.o: ``` # Proposal Doing something similar to what we already did for `bundled_program` in pytorch/executorch#10307 Since `extension.Module` is a public API, we introduce a namespace alias, so that existing use cases won't get affected. Since namespace alias doesn't create additional symbols, there won't be duplicate symbol issue. ghstack-source-id: 282066740 @exported-using-ghexport Differential Revision: [D73903870](https://our.internmc.facebook.com/intern/diff/D73903870/)
1 parent 6a9ccf1 commit f2e1ee5

File tree

4 files changed

+31
-2
lines changed

4 files changed

+31
-2
lines changed

extension/module/bundled_module.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
namespace executorch {
1717
namespace extension {
18+
namespace ET_BUNDLED_MODULE_NAMESPACE {
1819

1920
namespace {
2021
std::unique_ptr<BufferDataLoader> program_data_loader(
@@ -108,5 +109,6 @@ runtime::Error BundledModule::verify_method_outputs(
108109
*method, bundled_program_ptr_, testset_idx, rtol, atol);
109110
}
110111

112+
} // namespace ET_BUNDLED_MODULE_NAMESPACE
111113
} // namespace extension
112114
} // namespace executorch

extension/module/bundled_module.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,15 @@
1010

1111
#include <executorch/extension/module/module.h>
1212

13+
#ifdef USE_ATEN_LIB
14+
#define ET_BUNDLED_MODULE_NAMESPACE bundled_module::aten
15+
#else // !USE_ATEN_LIB
16+
#define ET_BUNDLED_MODULE_NAMESPACE bundled_module
17+
#endif // USE_ATEN_LIB
18+
1319
namespace executorch {
1420
namespace extension {
21+
namespace ET_BUNDLED_MODULE_NAMESPACE {
1522

1623
using executorch::extension::ET_MODULE_NAMESPACE::Module;
1724

@@ -121,5 +128,6 @@ class BundledModule : public Module {
121128
bool is_loaded_from_file_ = false;
122129
};
123130

131+
} // namespace ET_BUNDLED_MODULE_NAMESPACE
124132
} // namespace extension
125133
} // namespace executorch

extension/module/module.h

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616

1717
#include <executorch/runtime/executor/program.h>
1818

19+
#ifdef USE_ATEN_LIB
20+
#define ET_MODULE_NAMESPACE module::aten
21+
#else // !USE_ATEN_LIB
22+
#define ET_MODULE_NAMESPACE module
23+
#endif // USE_ATEN_LIB
24+
1925
namespace executorch {
2026
namespace extension {
2127

@@ -24,6 +30,10 @@ using ET_RUNTIME_NAMESPACE::MethodMeta;
2430
using ET_RUNTIME_NAMESPACE::NamedDataMap;
2531
using ET_RUNTIME_NAMESPACE::Program;
2632

33+
class ExecuTorchJni;
34+
35+
namespace ET_MODULE_NAMESPACE {
36+
2737
/**
2838
* A facade class for loading programs and executing methods within them.
2939
*/
@@ -493,16 +503,24 @@ class Module {
493503
protected:
494504
std::unordered_map<std::string, MethodHolder> methods_;
495505

496-
friend class ExecuTorchJni;
506+
friend class executorch::extension::ExecuTorchJni;
497507
};
498508

509+
} // namespace ET_MODULE_NAMESPACE
499510
} // namespace extension
500511
} // namespace executorch
501512

502513
namespace torch {
503514
namespace executor {
504515
// TODO(T197294990): Remove these deprecated aliases once all users have moved
505516
// to the new `::executorch` namespaces.
506-
using ::executorch::extension::Module;
517+
using ::executorch::extension::ET_MODULE_NAMESPACE::Module;
507518
} // namespace executor
508519
} // namespace torch
520+
521+
namespace executorch {
522+
namespace extension {
523+
// backward compatible namespace alias
524+
using ::executorch::extension::ET_MODULE_NAMESPACE::Module;
525+
} // namespace extension
526+
} // namespace executorch

extension/module/test/bundled_module_test.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
using namespace ::executorch::extension;
1313
using namespace ::executorch::runtime;
14+
using namespace ::executorch::extension::ET_BUNDLED_MODULE_NAMESPACE;
1415

1516
class BundledModuleTest : public ::testing::Test {
1617
protected:

0 commit comments

Comments
 (0)