You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "js8544 (via GitHub)" <gi...@apache.org> on 2023/09/17 06:47:33 UTC

[GitHub] [arrow] js8544 commented on a diff in pull request #37752: [GH-37751] [C++] [Gandiva] Avoid registering exported functions multiple times in gandiva

js8544 commented on code in PR #37752:
URL: https://github.com/apache/arrow/pull/37752#discussion_r1328052541


##########
cpp/src/gandiva/exported_funcs_registry.h:
##########
@@ -36,19 +36,11 @@ class ExportedFuncsRegistry {
   static void AddMappings(Engine* engine);
 
   static bool Register(std::shared_ptr<ExportedFuncsBase> entry) {
-    registered().push_back(entry);
+    registered().emplace_back(std::move(entry));
     return true;
   }
 
  private:
-  static list_type& registered() {
-    static list_type registered_list;
-    return registered_list;
-  }
+  static list_type& registered();
 };
-
-#define REGISTER_EXPORTED_FUNCS(classname) \
-  static bool _registered_##classname =    \
-      ExportedFuncsRegistry::Register(std::make_shared<classname>())

Review Comment:
   Can this macro be kept? I'm afraid this may be a breaking change for some users, if they have used this macro in their.code.



##########
cpp/src/gandiva/engine.cc:
##########
@@ -88,6 +89,7 @@ std::once_flag llvm_init_once_flag;
 static bool llvm_init = false;
 static llvm::StringRef cpu_name;
 static llvm::SmallVector<std::string, 10> cpu_attrs;
+std::once_flag register_exported_funcs_flag;

Review Comment:
   If you use the `REGISTER_EXPORTED_FUNCS` macro in `exported_funcs.cc`, can the changes to this file be skipped?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org