tqchen commented on code in PR #334:
URL: https://github.com/apache/tvm-ffi/pull/334#discussion_r2612562769


##########
src/ffi/extra/module.cc:
##########
@@ -30,6 +31,32 @@
 namespace tvm {
 namespace ffi {
 
+/*!
+ * \brief Global modules, i.e. modules that are owned by the runtime and 
should not be unloaded.
+ * On the frontend, a module is added to the registry if `keep_alive=True` 
when `load_module` is
+ * called.
+ */
+struct ModuleGlobals {
+  void Add(const Module& m) {
+    std::scoped_lock<std::mutex> lock(mutex);
+    modules.insert(m);
+  }
+
+  void Remove(const Module& m) {
+    std::scoped_lock<std::mutex> lock(mutex);
+    modules.erase(m);
+  }
+
+  static ModuleGlobals* Get() {
+    static ModuleGlobals instance;
+    return &instance;
+  }
+
+ private:
+  std::unordered_set<Module, ObjectPtrHash, ObjectPtrEqual> modules;

Review Comment:
   Google c style `modules_`, we can actually consider use Map<Module, int>, 
which might help reduce ABI bloat as we don't need to have `unordered_set`



##########
src/ffi/extra/module.cc:
##########
@@ -30,6 +31,32 @@
 namespace tvm {
 namespace ffi {
 
+/*!
+ * \brief Global modules, i.e. modules that are owned by the runtime and 
should not be unloaded.
+ * On the frontend, a module is added to the registry if `keep_alive=True` 
when `load_module` is
+ * called.
+ */
+struct ModuleGlobals {

Review Comment:
   ```
   class ModuleGlobals {
     public:
   ```



##########
src/ffi/extra/module.cc:
##########
@@ -30,6 +31,32 @@
 namespace tvm {
 namespace ffi {
 
+/*!
+ * \brief Global modules, i.e. modules that are owned by the runtime and 
should not be unloaded.
+ * On the frontend, a module is added to the registry if `keep_alive=True` 
when `load_module` is
+ * called.
+ */
+struct ModuleGlobals {
+  void Add(const Module& m) {
+    std::scoped_lock<std::mutex> lock(mutex);
+    modules.insert(m);
+  }
+
+  void Remove(const Module& m) {
+    std::scoped_lock<std::mutex> lock(mutex);
+    modules.erase(m);
+  }
+
+  static ModuleGlobals* Get() {
+    static ModuleGlobals instance;
+    return &instance;
+  }
+
+ private:
+  std::unordered_set<Module, ObjectPtrHash, ObjectPtrEqual> modules;
+  std::mutex mutex;

Review Comment:
   ``std::mutex mutex_``



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to