mcvsubbu commented on a change in pull request #5531:
URL: https://github.com/apache/incubator-pinot/pull/5531#discussion_r437729537



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java
##########
@@ -39,62 +38,63 @@
  */
 public class FunctionRegistry {
   private static final Logger LOGGER = 
LoggerFactory.getLogger(FunctionRegistry.class);
-  private static final Map<String, FunctionInfo> _functionInfoMap = new 
HashMap<>();
+  private static final Map<String, FunctionInfo> FUNCTION_INFO_MAP = new 
HashMap<>();
+
+  private static boolean _initialized = false;
 
   /**
-   * Given a function name, asserts that a corresponding function was 
registered during construction and returns it
+   * Initializes the FunctionRegistry by registering the scalar functions via 
reflection.
+   * NOTE: In order to plugin methods using reflection, the methods should be 
inside a class that includes ".function."
+   *       in its class path. This convention can significantly reduce the 
time of class scanning.
    */
-  public static FunctionInfo getFunctionByName(String functionName) {
-    
Preconditions.checkArgument(_functionInfoMap.containsKey(functionName.toLowerCase()));
-    return _functionInfoMap.get(functionName.toLowerCase());
+  public static synchronized void init() {
+    if (_initialized) {
+      LOGGER.info("FunctionRegistry is already initialized");
+      return;
+    }
+
+    long startTimeMs = System.currentTimeMillis();
+    Reflections reflections = new Reflections(
+        new 
ConfigurationBuilder().setUrls(ClasspathHelper.forPackage("org.apache.pinot"))
+            .filterInputsBy(new FilterBuilder.Include(".*\\.function\\..*"))
+            .setScanners(new MethodAnnotationsScanner()));
+    Set<Method> methodSet = 
reflections.getMethodsAnnotatedWith(ScalarFunction.class);
+    for (Method method : methodSet) {
+      ScalarFunction scalarFunction = 
method.getAnnotation(ScalarFunction.class);
+      if (scalarFunction.enabled()) {
+        if (!scalarFunction.name().isEmpty()) {
+          FunctionRegistry.registerFunction(scalarFunction.name(), method);
+        } else {
+          FunctionRegistry.registerFunction(method);
+        }
+      }
+    }
+    LOGGER.info("Initialized FunctionRegistry within {}ms", 
System.currentTimeMillis() - startTimeMs);

Review comment:
       why are we measuring the time to init functions? do we expect it to take 
a long time?
   Also, if we logging it may be useful to log the map size here, and also each 
function as they are registered




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to