[GitHub] [flink] lirui-apache commented on a change in pull request #10620: [FLINK-15239][table-planner-blink] TM Metaspace memory leak
lirui-apache commented on a change in pull request #10620: [FLINK-15239][table-planner-blink] TM Metaspace memory leak URL: https://github.com/apache/flink/pull/10620#discussion_r361248052 ## File path: flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/runtime/generated/CompileUtils.java ## @@ -45,7 +48,7 @@ * number of Meta zone GC (class unloading), resulting in performance bottlenecks. So we add * a cache to avoid this problem. */ - protected static final Cache, Class> COMPILED_CACHE = CacheBuilder + protected static final Cache> COMPILED_CACHE = CacheBuilder .newBuilder() .maximumSize(100) // estimated cache size Review comment: The origin cache uses tuple of class loader and class name as key. So yes, I guess theoretically there can be multiple class instances for the same name, otherwise we don't need to involve class loaders at all. Perhaps it can happen if we run multiple queries concurrently in one TM. @JingsongLi any insights about this? 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] [flink] lirui-apache commented on a change in pull request #10620: [FLINK-15239][table-planner-blink] TM Metaspace memory leak
lirui-apache commented on a change in pull request #10620: [FLINK-15239][table-planner-blink] TM Metaspace memory leak URL: https://github.com/apache/flink/pull/10620#discussion_r361088455 ## File path: flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/runtime/generated/CompileUtils.java ## @@ -45,7 +48,7 @@ * number of Meta zone GC (class unloading), resulting in performance bottlenecks. So we add * a cache to avoid this problem. */ - protected static final Cache, Class> COMPILED_CACHE = CacheBuilder + protected static final Cache> COMPILED_CACHE = CacheBuilder .newBuilder() .maximumSize(100) // estimated cache size Review comment: My hunch is that using soft values in the inner cache gives us fine grained garbage collection. Instead of removing the whole inner cache, we may want to only remove the least recently used class instances. According to this [reading](http://jeremymanson.blogspot.com/2009/07/how-hotspot-decides-to-clear_07.html), the timestamp that a soft value was lastly accessed plays a part when JVM decides whether to clear the reference. 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
