-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve support for Java 9+ #64
base: master
Are you sure you want to change the base?
Conversation
accessors.add(readUsingUnsafe(field)); | ||
} catch (RuntimeException e) { | ||
LOG.warn("The JVM is preventing Ehcache from accessing the subgraph beneath '{}'" + | ||
" - cache sizes may be underestimated as a result", field, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I test this PR and found the logs are full of this warning. I think we should only log this if unsafe fails. Otherwise, it's confusing since we are actually not understimating the size.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chrisdennis Could you please address this? I tested the latest version in Impala. The logs are still full of such warnings:
W1012 12:40:35.496349 2656 ObjectGraphWalker.java:220] 4a48697e1f87e06d:102ed89f00000000] The JVM is preventing Ehcache from accessing the subgraph beneath 'final jdk.internal.loader.URLClassPath jdk.internal.loader.ClassLoaders$AppClassLoader.ucp' - cache sizes may be underestimated as a result
Java exception follows:
java.lang.reflect.InaccessibleObjectException: Unable to make field final jdk.internal.loader.URLClassPath jdk.internal.loader.ClassLoaders$AppClassLoader.ucp accessible: module java.base does not "opens jdk.internal.loader" to unnamed module @10823d72
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:340)
at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:280)
at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:176)
at java.base/java.lang.reflect.Field.setAccessible(Field.java:170)
at org.ehcache.sizeof.ObjectGraphWalker.readUsingReflection(ObjectGraphWalker.java:232)
at org.ehcache.sizeof.ObjectGraphWalker.getFilteredFields(ObjectGraphWalker.java:214)
at org.ehcache.sizeof.ObjectGraphWalker.walk(ObjectGraphWalker.java:162)
at org.ehcache.sizeof.SizeOf.deepSizeOf(SizeOf.java:83)
at org.apache.impala.catalog.local.CatalogdMetaProvider$SizeOfWeigher.weigh(CatalogdMetaProvider.java:2014)
at com.google.common.cache.LocalCache$Segment.setValue(LocalCache.java:2010)
at com.google.common.cache.LocalCache$Segment.replace(LocalCache.java:2956)
at com.google.common.cache.LocalCache.replace(LocalCache.java:4258)
at org.apache.impala.catalog.local.CatalogdMetaProvider.loadWithCaching(CatalogdMetaProvider.java:542)
at org.apache.impala.catalog.local.CatalogdMetaProvider.loadIcebergApiTable(CatalogdMetaProvider.java:1058)
at org.apache.impala.catalog.local.LocalIcebergTable.loadIcebergTableViaMetaProvider(LocalIcebergTable.java:90)
at org.apache.impala.catalog.local.LocalTable.load(LocalTable.java:116)
at org.apache.impala.catalog.local.LocalDb.getTable(LocalDb.java:127)
at org.apache.impala.analysis.StmtMetadataLoader.getMissingTables(StmtMetadataLoader.java:310)
at org.apache.impala.analysis.StmtMetadataLoader.loadTables(StmtMetadataLoader.java:165)
at org.apache.impala.analysis.StmtMetadataLoader.loadTables(StmtMetadataLoader.java:141)
at org.apache.impala.service.Frontend.doCreateExecRequest(Frontend.java:2014)
at org.apache.impala.service.Frontend.getTExecRequest(Frontend.java:1926)
at org.apache.impala.service.Frontend.createExecRequest(Frontend.java:1748)
at org.apache.impala.service.JniFrontend.createExecRequest(JniFrontend.java:164)
Can we simply remove this log since the following accessor got from readUsingUnsafe() will work?
Can we reopen #65 and merge it for 0.4.1 release? It's much simpler than this PR so I think can be merged easier than this. |
@chrisdennis do you see a chance, to get this merged and to solve the java-9+ problems? questioning, if ehcache is still using this library and - if not - what else? |
This is a PoC for Java 9+ (up to current Java 18) support. We don't have to use the ByteBuddy agent implementation verbatim, but it proves that things can be made to work.