You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2016/02/19 20:30:54 UTC
svn commit: r1731291 - in /tomcat/trunk:
java/org/apache/catalina/startup/ContextConfig.java
test/org/apache/catalina/startup/TestContextConfigAnnotation.java
webapps/docs/changelog.xml
Author: markt
Date: Fri Feb 19 19:30:53 2016
New Revision: 1731291
URL: http://svn.apache.org/viewvc?rev=1731291&view=rev
Log:
Refactor to use a local variable rather than a field.
Even after the cache is cleared it uses significant memory because of the size the Map has grown to.
Modified:
tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java?rev=1731291&r1=1731290&r2=1731291&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java (original)
+++ tomcat/trunk/java/org/apache/catalina/startup/ContextConfig.java Fri Feb 19 19:30:53 2016
@@ -222,14 +222,6 @@ public class ContextConfig implements Li
new HashMap<>();
/**
- * Cache of JavaClass objects (byte code) by fully qualified class name.
- * Only populated if it is necessary to scan the super types and interfaces
- * as part of the processing for {@link HandlesTypes}.
- */
- protected final Map<String,JavaClassCacheEntry> javaClassCache =
- new HashMap<>();
-
- /**
* Flag that indicates if at least one {@link HandlesTypes} entry is present
* that represents an annotation.
*/
@@ -1141,13 +1133,15 @@ public class ContextConfig implements Li
if (!webXml.isMetadataComplete() || typeInitializerMap.size() > 0) {
// Step 4. Process /WEB-INF/classes for annotations and
// @HandlesTypes matches
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
+
if (ok) {
WebResource[] webResources =
context.getResources().listResources("/WEB-INF/classes");
for (WebResource webResource : webResources) {
processAnnotationsWebResource(webResource, webXml,
- webXml.isMetadataComplete());
+ webXml.isMetadataComplete(), javaClassCache);
}
}
@@ -1157,7 +1151,7 @@ public class ContextConfig implements Li
// container fragments)
if (ok) {
processAnnotations(
- orderedFragments, webXml.isMetadataComplete());
+ orderedFragments, webXml.isMetadataComplete(), javaClassCache);
}
// Cache, if used, is no longer required so clear it
@@ -1899,7 +1893,7 @@ public class ContextConfig implements Li
}
protected void processAnnotations(Set<WebXml> fragments,
- boolean handlesTypesOnly) {
+ boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
for(WebXml fragment : fragments) {
// Only need to scan for @HandlesTypes matches if any of the
// following are true:
@@ -1914,7 +1908,7 @@ public class ContextConfig implements Li
// no impact on distributable
annotations.setDistributable(true);
URL url = fragment.getURL();
- processAnnotationsUrl(url, annotations, htOnly);
+ processAnnotationsUrl(url, annotations, htOnly, javaClassCache);
Set<WebXml> set = new HashSet<>();
set.add(annotations);
// Merge annotations into fragment - fragment takes priority
@@ -1923,7 +1917,8 @@ public class ContextConfig implements Li
}
protected void processAnnotationsWebResource(WebResource webResource,
- WebXml fragment, boolean handlesTypesOnly) {
+ WebXml fragment, boolean handlesTypesOnly,
+ Map<String,JavaClassCacheEntry> javaClassCache) {
if (webResource.isDirectory()) {
WebResource[] webResources =
@@ -1936,13 +1931,13 @@ public class ContextConfig implements Li
webResource.getURL()));
}
for (WebResource r : webResources) {
- processAnnotationsWebResource(r, fragment, handlesTypesOnly);
+ processAnnotationsWebResource(r, fragment, handlesTypesOnly, javaClassCache);
}
}
} else if (webResource.isFile() &&
webResource.getName().endsWith(".class")) {
try (InputStream is = webResource.getInputStream()) {
- processAnnotationsStream(is, fragment, handlesTypesOnly);
+ processAnnotationsStream(is, fragment, handlesTypesOnly, javaClassCache);
} catch (IOException e) {
log.error(sm.getString("contextConfig.inputStreamWebResource",
webResource.getWebappPath()),e);
@@ -1955,16 +1950,16 @@ public class ContextConfig implements Li
protected void processAnnotationsUrl(URL url, WebXml fragment,
- boolean handlesTypesOnly) {
+ boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
if (url == null) {
// Nothing to do.
return;
} else if ("jar".equals(url.getProtocol())) {
- processAnnotationsJar(url, fragment, handlesTypesOnly);
+ processAnnotationsJar(url, fragment, handlesTypesOnly, javaClassCache);
} else if ("file".equals(url.getProtocol())) {
try {
processAnnotationsFile(
- new File(url.toURI()), fragment, handlesTypesOnly);
+ new File(url.toURI()), fragment, handlesTypesOnly, javaClassCache);
} catch (URISyntaxException e) {
log.error(sm.getString("contextConfig.fileUrl", url), e);
}
@@ -1977,7 +1972,7 @@ public class ContextConfig implements Li
protected void processAnnotationsJar(URL url, WebXml fragment,
- boolean handlesTypesOnly) {
+ boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
try (Jar jar = JarFactory.newInstance(url)) {
if (log.isDebugEnabled()) {
@@ -1990,8 +1985,7 @@ public class ContextConfig implements Li
while (entryName != null) {
if (entryName.endsWith(".class")) {
try (InputStream is = jar.getEntryInputStream()) {
- processAnnotationsStream(
- is, fragment, handlesTypesOnly);
+ processAnnotationsStream(is, fragment, handlesTypesOnly, javaClassCache);
} catch (IOException e) {
log.error(sm.getString("contextConfig.inputStreamJar",
entryName, url),e);
@@ -2010,7 +2004,7 @@ public class ContextConfig implements Li
protected void processAnnotationsFile(File file, WebXml fragment,
- boolean handlesTypesOnly) {
+ boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache) {
if (file.isDirectory()) {
// Returns null if directory is not readable
@@ -2022,12 +2016,12 @@ public class ContextConfig implements Li
}
for (String dir : dirs) {
processAnnotationsFile(
- new File(file,dir), fragment, handlesTypesOnly);
+ new File(file,dir), fragment, handlesTypesOnly, javaClassCache);
}
}
} else if (file.getName().endsWith(".class") && file.canRead()) {
try (FileInputStream fis = new FileInputStream(file)) {
- processAnnotationsStream(fis, fragment, handlesTypesOnly);
+ processAnnotationsStream(fis, fragment, handlesTypesOnly, javaClassCache);
} catch (IOException e) {
log.error(sm.getString("contextConfig.inputStreamFile",
file.getAbsolutePath()),e);
@@ -2040,12 +2034,12 @@ public class ContextConfig implements Li
protected void processAnnotationsStream(InputStream is, WebXml fragment,
- boolean handlesTypesOnly)
+ boolean handlesTypesOnly, Map<String,JavaClassCacheEntry> javaClassCache)
throws ClassFormatException, IOException {
ClassParser parser = new ClassParser(is);
JavaClass clazz = parser.parse();
- checkHandlesTypes(clazz);
+ checkHandlesTypes(clazz, javaClassCache);
if (handlesTypesOnly) {
return;
@@ -2075,7 +2069,8 @@ public class ContextConfig implements Li
* for an annotation that matches {@link HandlesTypes}.
* @param javaClass the class to check
*/
- protected void checkHandlesTypes(JavaClass javaClass) {
+ protected void checkHandlesTypes(JavaClass javaClass,
+ Map<String,JavaClassCacheEntry> javaClassCache) {
// Skip this if we can
if (typeInitializerMap.size() == 0) {
@@ -2093,16 +2088,16 @@ public class ContextConfig implements Li
Class<?> clazz = null;
if (handlesTypesNonAnnotations) {
// This *might* be match for a HandlesType.
- populateJavaClassCache(className, javaClass);
+ populateJavaClassCache(className, javaClass, javaClassCache);
JavaClassCacheEntry entry = javaClassCache.get(className);
if (entry.getSciSet() == null) {
try {
- populateSCIsForCacheEntry(entry);
+ populateSCIsForCacheEntry(entry, javaClassCache);
} catch (StackOverflowError soe) {
throw new IllegalStateException(sm.getString(
"contextConfig.annotationsStackOverflow",
context.getName(),
- classHierarchyToString(className, entry)));
+ classHierarchyToString(className, entry, javaClassCache)));
}
}
if (!entry.getSciSet().isEmpty()) {
@@ -2157,7 +2152,7 @@ public class ContextConfig implements Li
private String classHierarchyToString(String className,
- JavaClassCacheEntry entry) {
+ JavaClassCacheEntry entry, Map<String,JavaClassCacheEntry> javaClassCache) {
JavaClassCacheEntry start = entry;
StringBuilder msg = new StringBuilder(className);
msg.append("->");
@@ -2180,7 +2175,8 @@ public class ContextConfig implements Li
return msg.toString();
}
- private void populateJavaClassCache(String className, JavaClass javaClass) {
+ private void populateJavaClassCache(String className, JavaClass javaClass,
+ Map<String,JavaClassCacheEntry> javaClassCache) {
if (javaClassCache.containsKey(className)) {
return;
}
@@ -2188,14 +2184,15 @@ public class ContextConfig implements Li
// Add this class to the cache
javaClassCache.put(className, new JavaClassCacheEntry(javaClass));
- populateJavaClassCache(javaClass.getSuperclassName());
+ populateJavaClassCache(javaClass.getSuperclassName(), javaClassCache);
for (String iterface : javaClass.getInterfaceNames()) {
- populateJavaClassCache(iterface);
+ populateJavaClassCache(iterface, javaClassCache);
}
}
- private void populateJavaClassCache(String className) {
+ private void populateJavaClassCache(String className,
+ Map<String,JavaClassCacheEntry> javaClassCache) {
if (!javaClassCache.containsKey(className)) {
String name = className.replace('.', '/') + ".class";
try (InputStream is = context.getLoader().getClassLoader().getResourceAsStream(name)) {
@@ -2204,7 +2201,7 @@ public class ContextConfig implements Li
}
ClassParser parser = new ClassParser(is);
JavaClass clazz = parser.parse();
- populateJavaClassCache(clazz.getClassName(), clazz);
+ populateJavaClassCache(clazz.getClassName(), clazz, javaClassCache);
} catch (ClassFormatException e) {
log.debug(sm.getString("contextConfig.invalidSciHandlesTypes",
className), e);
@@ -2215,7 +2212,8 @@ public class ContextConfig implements Li
}
}
- private void populateSCIsForCacheEntry(JavaClassCacheEntry cacheEntry) {
+ private void populateSCIsForCacheEntry(JavaClassCacheEntry cacheEntry,
+ Map<String,JavaClassCacheEntry> javaClassCache) {
Set<ServletContainerInitializer> result = new HashSet<>();
// Super class
@@ -2232,7 +2230,7 @@ public class ContextConfig implements Li
// May be null of the class is not present or could not be loaded.
if (superClassCacheEntry != null) {
if (superClassCacheEntry.getSciSet() == null) {
- populateSCIsForCacheEntry(superClassCacheEntry);
+ populateSCIsForCacheEntry(superClassCacheEntry, javaClassCache);
}
result.addAll(superClassCacheEntry.getSciSet());
}
@@ -2247,7 +2245,7 @@ public class ContextConfig implements Li
// so move along
if (interfaceEntry != null) {
if (interfaceEntry.getSciSet() == null) {
- populateSCIsForCacheEntry(interfaceEntry);
+ populateSCIsForCacheEntry(interfaceEntry, javaClassCache);
}
result.addAll(interfaceEntry.getSciSet());
}
@@ -2598,7 +2596,7 @@ public class ContextConfig implements Li
}
}
- private static class JavaClassCacheEntry {
+ static class JavaClassCacheEntry {
public final String superclassName;
public final String[] interfaceNames;
Modified: tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java?rev=1731291&r1=1731290&r2=1731291&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java (original)
+++ tomcat/trunk/test/org/apache/catalina/startup/TestContextConfigAnnotation.java Fri Feb 19 19:30:53 2016
@@ -19,7 +19,9 @@ package org.apache.catalina.startup;
import java.beans.PropertyChangeListener;
import java.io.File;
import java.net.URL;
+import java.util.HashMap;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import javax.servlet.DispatcherType;
@@ -39,6 +41,7 @@ import org.junit.Test;
import org.apache.catalina.Context;
import org.apache.catalina.Loader;
import org.apache.catalina.core.StandardContext;
+import org.apache.catalina.startup.ContextConfig.JavaClassCacheEntry;
import org.apache.tomcat.util.descriptor.web.FilterDef;
import org.apache.tomcat.util.descriptor.web.FilterMap;
import org.apache.tomcat.util.descriptor.web.ServletDef;
@@ -55,11 +58,12 @@ public class TestContextConfigAnnotation
@Test
public void testAnnotation() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
File pFile = paramClassResource(
"org/apache/catalina/startup/ParamServlet");
assertTrue(pFile.exists());
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
ServletDef servletDef = webxml.getServlets().get("param");
assertNotNull(servletDef);
assertEquals("Hello", servletDef.getParameterMap().get("foo"));
@@ -81,6 +85,7 @@ public class TestContextConfigAnnotation
@Test
public void testOverwriteAnnotation() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ServletDef servletDef = new ServletDef();
servletDef.setServletName("param");
servletDef.setServletClass("org.apache.catalina.startup.ParamServlet");
@@ -98,7 +103,7 @@ public class TestContextConfigAnnotation
File pFile = paramClassResource(
"org/apache/catalina/startup/ParamServlet");
assertTrue(pFile.exists());
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
assertEquals(servletDef, webxml.getServlets().get("param"));
@@ -121,16 +126,17 @@ public class TestContextConfigAnnotation
@Test
public void testNoMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
File pFile = paramClassResource(
"org/apache/catalina/startup/NoMappingParamServlet");
assertTrue(pFile.exists());
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
ServletDef servletDef = webxml.getServlets().get("param1");
assertNull(servletDef);
webxml.addServletMapping("/param", "param1");
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
servletDef = webxml.getServlets().get("param1");
assertNull(servletDef);
@@ -139,6 +145,7 @@ public class TestContextConfigAnnotation
@Test
public void testSetupWebXMLNoMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ServletDef servletDef = new ServletDef();
servletDef.setServletName("param1");
servletDef.setServletClass(
@@ -151,7 +158,7 @@ public class TestContextConfigAnnotation
File pFile = paramClassResource(
"org/apache/catalina/startup/NoMappingParamServlet");
assertTrue(pFile.exists());
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
assertEquals("tomcat", servletDef.getParameterMap().get("foo"));
assertEquals("World!", servletDef.getParameterMap().get("bar"));
ServletDef servletDef1 = webxml.getServlets().get("param1");
@@ -162,12 +169,13 @@ public class TestContextConfigAnnotation
@Test
public void testDuplicateMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
File pFile = paramClassResource(
"org/apache/catalina/startup/DuplicateMappingParamServlet");
assertTrue(pFile.exists());
try {
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
fail();
} catch (IllegalArgumentException ex) {
// ignore
@@ -179,13 +187,14 @@ public class TestContextConfigAnnotation
@Test
public void testFilterMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
File sFile = paramClassResource(
"org/apache/catalina/startup/ParamServlet");
- config.processAnnotationsFile(sFile, webxml, false);
+ config.processAnnotationsFile(sFile, webxml, false, javaClassCache);
File fFile = paramClassResource(
"org/apache/catalina/startup/ParamFilter");
- config.processAnnotationsFile(fFile, webxml, false);
+ config.processAnnotationsFile(fFile, webxml, false, javaClassCache);
FilterDef fdef = webxml.getFilters().get("paramFilter");
assertNotNull(fdef);
assertEquals("Servlet says: ",fdef.getParameterMap().get("message"));
@@ -194,6 +203,7 @@ public class TestContextConfigAnnotation
@Test
public void testOverwriteFilterMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
FilterDef filterDef = new FilterDef();
filterDef.setFilterName("paramFilter");
filterDef.setFilterClass("org.apache.catalina.startup.ParamFilter");
@@ -214,10 +224,10 @@ public class TestContextConfigAnnotation
ContextConfig config = new ContextConfig();
File sFile = paramClassResource(
"org/apache/catalina/startup/ParamServlet");
- config.processAnnotationsFile(sFile, webxml, false);
+ config.processAnnotationsFile(sFile, webxml, false, javaClassCache);
File fFile = paramClassResource(
"org/apache/catalina/startup/ParamFilter");
- config.processAnnotationsFile(fFile, webxml, false);
+ config.processAnnotationsFile(fFile, webxml, false, javaClassCache);
FilterDef fdef = webxml.getFilters().get("paramFilter");
assertNotNull(fdef);
assertEquals(filterDef,fdef);
@@ -249,12 +259,13 @@ public class TestContextConfigAnnotation
@Test
public void testDuplicateFilterMapping() throws Exception {
WebXml webxml = new WebXml();
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
File pFile = paramClassResource(
"org/apache/catalina/startup/DuplicateMappingParamFilter");
assertTrue(pFile.exists());
try {
- config.processAnnotationsFile(pFile, webxml, false);
+ config.processAnnotationsFile(pFile, webxml, false, javaClassCache);
fail();
} catch (IllegalArgumentException ex) {
// ignore
@@ -265,6 +276,7 @@ public class TestContextConfigAnnotation
@Test
public void testCheckHandleTypes() throws Exception {
+ Map<String,JavaClassCacheEntry> javaClassCache = new HashMap<>();
ContextConfig config = new ContextConfig();
config.handlesTypesAnnotations = true;
config.handlesTypesNonAnnotations = true;
@@ -296,13 +308,13 @@ public class TestContextConfigAnnotation
WebXml ignore = new WebXml();
File file = paramClassResource(
"org/apache/catalina/startup/ParamServlet");
- config.processAnnotationsFile(file, ignore, false);
+ config.processAnnotationsFile(file, ignore, false, javaClassCache);
file = paramClassResource("org/apache/catalina/startup/ParamFilter");
- config.processAnnotationsFile(file, ignore, false);
+ config.processAnnotationsFile(file, ignore, false, javaClassCache);
file = paramClassResource("org/apache/catalina/startup/TesterServlet");
- config.processAnnotationsFile(file, ignore, false);
+ config.processAnnotationsFile(file, ignore, false, javaClassCache);
file = paramClassResource("org/apache/catalina/startup/TestListener");
- config.processAnnotationsFile(file, ignore, false);
+ config.processAnnotationsFile(file, ignore, false, javaClassCache);
// Check right number of classes were noted to be handled
assertEquals(0, config.initializerClassMap.get(sciNone).size());
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1731291&r1=1731290&r2=1731291&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Fri Feb 19 19:30:53 2016
@@ -108,6 +108,9 @@
effective web.xml. Components needing access to configuration
information may access it via the Servlet API. (markt)
</fix>
+ <fix>
+ Refactor JAR scanning to reduce memory footprint. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1731291 - in /tomcat/trunk:
java/org/apache/catalina/startup/ContextConfig.java
test/org/apache/catalina/startup/TestContextConfigAnnotation.java
webapps/docs/changelog.xml
Posted by Mark Thomas <ma...@apache.org>.
On 19/02/2016 19:30, markt@apache.org wrote:
> Author: markt
> Date: Fri Feb 19 19:30:53 2016
> New Revision: 1731291
>
> URL: http://svn.apache.org/viewvc?rev=1731291&view=rev
> Log:
> Refactor to use a local variable rather than a field.
> Even after the cache is cleared it uses significant memory because of the size the Map has grown to.
The next biggest target is the cached Manifests. I'm not (yet) convinced
we need to go that far. If we did, caching them the same way we cache
the JarEntries is probably the way to go.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org