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
