neoremind commented on a change in pull request #1875: [CALCITE-3873] Use 
global caching for ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#discussion_r398291034
 
 

 ##########
 File path: core/src/main/java/org/apache/calcite/util/ReflectUtil.java
 ##########
 @@ -403,71 +403,40 @@ private static Method lookupVisitMethod(
   }
 
   /**
-   * Creates a dispatcher for calls to {@link #lookupVisitMethod}. The
-   * dispatcher caches methods between invocations.
+   * Creates a dispatcher for calls to {@link #lookupVisitMethod}. By default 
the
+   * dispatcher caches methods globally. If caching methods between invocations
+   * is preferred, use the overridden method {@link #createDispatcher(Class, 
Class, boolean)}.
    *
-   * @param visitorBaseClazz Visitor base class
-   * @param visiteeBaseClazz Visitee base class
+   * @param visitorBaseClazz        Visitor base class
+   * @param visiteeBaseClazz        Visitee base class
    * @return cache of methods
    */
   public static <R extends ReflectiveVisitor, E> ReflectiveVisitDispatcher<R, 
E> createDispatcher(
       final Class<R> visitorBaseClazz,
       final Class<E> visiteeBaseClazz) {
+    return createDispatcher(visitorBaseClazz, visiteeBaseClazz, true);
+  }
+
+  /**
+   * Creates a dispatcher for calls to {@link #lookupVisitMethod}.
+   *
+   * @param visitorBaseClazz        Visitor base class
+   * @param visiteeBaseClazz        Visitee base class
+   * @param useGlobalMethodCache    If set to true, the created dispatchers 
will
+   *                                share a globally cache to store methods to
+   *                                mitigating reflection invocation overhead
+   *                                when looking up method. If set to false, 
every
+   *                                dispatcher instance will only cache methods
+   *                                between invocations individually.
+   * @return cache of methods
+   */
+  public static <R extends ReflectiveVisitor, E> ReflectiveVisitDispatcher<R, 
E> createDispatcher(
+      final Class<R> visitorBaseClazz,
+      final Class<E> visiteeBaseClazz,
+      final boolean useGlobalMethodCache) {
 
 Review comment:
   To maintain backward compatibility, I add one param boolean 
`useGlobalMethodCache` to method `ReflectiveVisitDispatcher<R, E> 
createDispatcher(..)` and provide an overridden method to make 
`useGlobalMethodCache=true` by default, so anywhere else won't be changed, but 
the underlying behavior will enable global caching. In case there might be 
situations where `ReflectiveVisitDispatcher` is likely to cache per instance 
and does not want to bother the global cache, so to make that flexibility 
possible I made such change.

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


With regards,
Apache Git Services

Reply via email to