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