Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve support for Java 9+ #64

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+728 −993
Draft

Improve support for Java 9+ #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions findbugs-exclude-filter.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?xml version="1.0"?>
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Copyright Terracotta, Inc.
~
Expand All @@ -15,16 +15,19 @@
~ limitations under the License.
-->

<FindBugsFilter>

<FindBugsFilter>
<FindBugsFilter xmlns="https://github.com/spotbugs/filter/3.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="https://github.com/spotbugs/filter/3.0.0 https://raw.githubusercontent.com/spotbugs/spotbugs/4.7.0/spotbugs/etc/findbugsfilter.xsd">
<Match>
<Class name="~org\.ehcache\.sizeof\.FlyweightType.*"/>
<Class name="~org\.ehcache\.sizeof\.FlyweightType.*"/>
</Match>
<Match>
<Class name="org.ehcache.sizeof.Configuration"/>
<Method name="getFilters" />
<Class name="org.ehcache.sizeof.Configuration"/>
<Method name="getFilters"/>
</Match>
<Match>
<Class name="org.ehcache.sizeof.util.UnsafeAccess"/>
<Method name="getUnsafe"/>
<Bug pattern="MS_EXPOSE_REP"/>
</Match>
</FindBugsFilter>

</FindBugsFilter>
89 changes: 40 additions & 49 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

<groupId>org.ehcache</groupId>
<artifactId>sizeof</artifactId>
<version>0.4.1-SNAPSHOT</version>
<version>0.5-SNAPSHOT</version>
<packaging>jar</packaging>

<name>Ehcache SizeOf Engine</name>
Expand Down Expand Up @@ -40,6 +40,11 @@
</properties>

<dependencies>
<dependency>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy-agent</artifactId>
<version>1.12.9</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
Expand All @@ -64,9 +69,9 @@
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
<version>6.0</version>
<groupId>net.bytebuddy</groupId>
<artifactId>byte-buddy</artifactId>
<version>1.12.9</version>
<scope>test</scope>
</dependency>
</dependencies>
Expand All @@ -92,7 +97,7 @@
</plugin>
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<version>2.20.1</version>
<version>2.22.2</version>
</plugin>
<plugin>
<artifactId>maven-gpg-plugin</artifactId>
Expand Down Expand Up @@ -129,9 +134,9 @@
</plugin>

<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<version>3.0.5</version>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<version>4.7.0.0</version>
</plugin>
<plugin>
<groupId>org.jacoco</groupId>
Expand Down Expand Up @@ -188,52 +193,13 @@
</goals>
<configuration>
<excludes>
<exclude>src/hidden/**</exclude>
<exclude>README.adoc</exclude>
<exclude>**/*.txt</exclude>
</excludes>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.gmavenplus</groupId>
<artifactId>gmavenplus-plugin</artifactId>
<executions>
<execution>
<id>create-agent-jar</id>
<phase>process-classes</phase>
<goals>
<goal>execute</goal>
</goals>
<configuration>
<scripts>
<script>
def jarFile = new File(project.build.directory, "/classes/org/ehcache/sizeof/impl/sizeof-agent.jar")
def agentClass = "/org/ehcache/sizeof/impl/SizeOfAgent.class"
def agentDir = project.build.directory + "/agent-jar"
def manifestDir = project.basedir.getAbsolutePath() + "/src/hidden/resources"
ant.move(file: new File(project.build.outputDirectory, agentClass),
tofile: new File(agentDir, agentClass))

ant.jar(destfile: jarFile, basedir: new File(agentDir).getAbsolutePath(), manifest: new File(manifestDir, "/META-INF/MANIFEST.MF"))

ant.delete(dir: new File(agentDir))
</script>
</scripts>
</configuration>
</execution>
</executions>
<dependencies>
<dependency>
<groupId>org.codehaus.groovy</groupId>
<artifactId>groovy-all</artifactId>
<!-- any version of Groovy \>= 1.5.0 should work here -->
<version>2.4.12</version>
<scope>runtime</scope>
</dependency>
</dependencies>
</plugin>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
Expand Down Expand Up @@ -275,8 +241,8 @@
</executions>
</plugin>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>findbugs-maven-plugin</artifactId>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
<configuration>
<excludeFilterFile>${basedir}/findbugs-exclude-filter.xml</excludeFilterFile>
</configuration>
Expand Down Expand Up @@ -309,6 +275,31 @@
</plugins>
</build>

<profiles>
<profile>
<id>java9</id>
<activation>
<jdk>[9,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>-Xms64m -Xmx64m --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED</argLine>
</configuration>
</plugin>
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<argLine>-Xms64m -Xmx64m --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>

<scm>
<url>https://github.com/ehcache/sizeof</url>
<connection>scm:git:https://github.com/ehcache/sizeof.git</connection>
Expand Down
3 changes: 0 additions & 3 deletions src/hidden/resources/META-INF/MANIFEST.MF
View file
Open in desktop

This file was deleted.

83 changes: 57 additions & 26 deletions src/main/java/org/ehcache/sizeof/ObjectGraphWalker.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/
package org.ehcache.sizeof;

import org.ehcache.sizeof.util.UnsafeAccess;
import sun.misc.Unsafe;
import org.ehcache.sizeof.filters.SizeOfFilter;
import org.ehcache.sizeof.util.WeakIdentityConcurrentMap;
import org.slf4j.Logger;
Expand All @@ -30,6 +32,7 @@
import java.util.Deque;
import java.util.IdentityHashMap;
import java.util.Set;
import java.util.function.Function;

import static java.util.Collections.newSetFromMap;

Expand All @@ -44,7 +47,7 @@ final class ObjectGraphWalker {
private static final String VERBOSE_DEBUG_LOGGING = "org.ehcache.sizeof.verboseDebugLogging";
private static final boolean USE_VERBOSE_DEBUG_LOGGING;

private final WeakIdentityConcurrentMap<Class<?>, SoftReference<Collection<Field>>> fieldCache =
private final WeakIdentityConcurrentMap<Class<?>, SoftReference<Collection<Function<Object, Object>>>> fieldCache =
new WeakIdentityConcurrentMap<>();
private final WeakIdentityConcurrentMap<Class<?>, Boolean> classCache =
new WeakIdentityConcurrentMap<>();
Expand Down Expand Up @@ -156,12 +159,8 @@ long walk(VisitorListener visitorListener, Object... root) {
nullSafeAdd(toVisit, Array.get(ref, i));
}
} else {
for (Field field : getFilteredFields(refClass)) {
try {
nullSafeAdd(toVisit, field.get(ref));
} catch (IllegalAccessException ex) {
throw new RuntimeException(ex);
}
for (Function<Object, Object> accessor : getFilteredFields(refClass)) {
nullSafeAdd(toVisit, accessor.apply(ref));
}
}

Expand Down Expand Up @@ -194,23 +193,66 @@ long walk(VisitorListener visitorListener, Object... root) {
* @param refClass the type
* @return A collection of fields to be visited
*/
private Collection<Field> getFilteredFields(Class<?> refClass) {
SoftReference<Collection<Field>> ref = fieldCache.get(refClass);
Collection<Field> fieldList = ref != null ? ref.get() : null;
private Collection<Function<Object, Object>> getFilteredFields(Class<?> refClass) {
SoftReference<Collection<Function<Object, Object>>> ref = fieldCache.get(refClass);
Collection<Function<Object, Object>> fieldList = ref != null ? ref.get() : null;
if (fieldList != null) {
return fieldList;
} else {
Collection<Field> result;
result = sizeOfFilter.filterFields(refClass, getAllFields(refClass));
Collection<Field> fields = sizeOfFilter.filterFields(refClass, getAllFields(refClass));
Collection<Function<Object, Object>> accessors = new ArrayList<>(fields.size());
if (USE_VERBOSE_DEBUG_LOGGING && LOG.isDebugEnabled()) {
for (Field field : result) {
for (Field field : fields) {
if (Modifier.isTransient(field.getModifiers())) {
LOG.debug("SizeOf engine walking transient field '{}' of class {}", field.getName(), refClass.getName());
}
}
}
fieldCache.put(refClass, new SoftReference<>(result));
return result;

for (Field field : fields) {
try {
try {
accessors.add(readUsingReflection(field));
} catch (Throwable t) {
Function<Object, Object> unsafeRead = readUsingUnsafe(field);
if (unsafeRead == null) {
throw t;
} else {
accessors.add(unsafeRead);
}
}
} catch (SecurityException e) {
LOG.error("Security settings prevent Ehcache from accessing the subgraph beneath '{}'" +
" - cache sizes may be underestimated as a result", field, e);
} catch (Throwable e) {
LOG.warn("The JVM is preventing Ehcache from accessing the subgraph beneath '{}'" +
" - cache sizes may be underestimated as a result", field, e);
Copy link

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.

Copy link

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?

}
}

fieldCache.put(refClass, new SoftReference<>(accessors));
return accessors;
}
}

private static Function<Object, Object> readUsingReflection(Field field) throws SecurityException {
field.setAccessible(true);
return ref -> {
try {
return field.get(ref);
} catch (IllegalAccessException e) {
throw new AssertionError("IllegalAccessException after setting accessible!", e);
}
};
}

private static Function<Object, Object> readUsingUnsafe(Field field) {
Unsafe unsafe = UnsafeAccess.getUnsafe();
if (unsafe == null) {
return null;
} else {
long offset = unsafe.objectFieldOffset(field);
return ref -> unsafe.getObject(ref, offset);
}
}

Expand Down Expand Up @@ -241,17 +283,6 @@ private static Collection<Field> getAllFields(Class<?> refClass) {
for (Field field : klazz.getDeclaredFields()) {
if (!Modifier.isStatic(field.getModifiers()) &&
!field.getType().isPrimitive()) {
try {
field.setAccessible(true);
} catch (SecurityException e) {
LOG.error("Security settings prevent Ehcache from accessing the subgraph beneath '{}'" +
" - cache sizes may be underestimated as a result", field, e);
continue;
} 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);
continue;
}
fields.add(field);
}
}
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/org/ehcache/sizeof/ObjectSizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Copyright Terracotta, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.ehcache.sizeof;

public interface ObjectSizer {

long sizeOf(Object obj);
}
Loading