[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for
ReflectiveVisitDispatcher implementation
URL: https://github.com/apache/calcite/pull/1875#issuecomment-610846372
> it describes "Weak values will be garbage collected once they are weakly
reachable. This makes them a poor candidate for caching; consider {@link
#softValues} instead."
The keys can still reference classes and retain them from garbage collecting.
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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-607143698 >I see, how about using weak or soft values? Weak would probably work. Soft references are almost always a bad choice. 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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-607120781 >I would rather call it a global caching, not memory leak, because we might use them repeatedly. What I mean is GLOBAL_CACHE keeps a strong reference to the class objects, thus effectively prevents class unloading even in the case the class is not used anymore. 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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-607098114 It looks good, except the fact that GLOBAL_METHOD_CACHE might result in a memory leak. It would retain `Method` (and thus `Class`) objects preventing them from being garbage-collected. 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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-605098579 Have you seen https://dzone.com/articles/hacking-lambda-expressions-in-java ? > The invocation of functional interfaces with dynamically generated implementations using LambdaMetafactory is significantly faster than invocation through the Java Reflection API It does not look like your branch is using `LambdaMetafactory` :) 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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-604937327 @neoremind , https://github.com/apache/calcite/pull/1880 makes `.main` classes runnable for me. Please try it in your own branch (or even include 1880 changes to this PR). I don't commit it yet because there's a pending discussion (hey, @danny0405 , no rush, but, your input is very welcome ) re https://github.com/apache/calcite/pull/1872 (see https://issues.apache.org/jira/browse/CALCITE-3871). 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
[GitHub] [calcite] vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation
vlsi commented on issue #1875: [CALCITE-3873] Use global caching for ReflectiveVisitDispatcher implementation URL: https://github.com/apache/calcite/pull/1875#issuecomment-604506267 @neoremind , do you think you could add the jmh test to Calcite as well (see https://github.com/apache/calcite/tree/master/ubenchmark )? PS Have you considered MethodHandle-based approach? 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
