Patch is attached. Once again, please note that this is based on Tiles as of
2001-09-10. I would send my sources to the list, but I have only just joined
struts-dev today and I don't know the etiquette in use here with regards to
attachments. FWIW, my modified sources fit in a 17k zip file
(non-uuencoded).
If you are not interested in my patch and prefer to implement it your way,
here is my advice:
1. go to ComponentDefinition and ComponentContext and delete the following
'evil' methods
public Map getAttributes()
public ComponentContext(Map attributes)
public void addAll(Map newAttributes)
public void addMissing(Map defaultAttributes)
2. then refactor/recompile/refactor the rest of Tiles until you have
something usable (pass along and keep track of ComponentDefinition objects
as much as you can -- never access the HashMap directly).
Regards,
-- Christophe
-----Original Message-----
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]
Sent: Monday, March 10, 2003 6:22 PM
To: Struts Developers List
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: Re: Tiles excessive memory usage
Chris,
I'm just now getting to profiling our struts/tiles app and have always had
some concern about the efficiency of the tiles. Not that I'm a suspecting
type of person but I just didn't have any information and that caused me to
wonder.
Regardles when or if this fix gets placed in a release, would you be willing
to offer sufficient code snippets that will give me enough understanding to
implement your fix in my app? I'm now getting deeper into tiles and getting
a good feel for what's going on so this would be a perfect time for me
before we enter client phase. Also, I'd be willing to contribute this
efficiency gain to the project(with your help or ok) if Cedric and the other
powers agree this is something that should be done.
But in the meantime, could you supply me with some more detailed information
and snippets. This could potentially kill my project since performance is
the major hot word amoung manager types these days where I work. You can
reply to [EMAIL PROTECTED] with details that don't pertain to the
developer's group if you like.
Thanks for posting this!!!!!!!
Chris Willingham
----- Original Message -----
From: Christophe Warland
To: '[EMAIL PROTECTED]'
Cc: '[EMAIL PROTECTED]' ; '[EMAIL PROTECTED]'
Sent: Monday, March 10, 2003 4:47 PM
Subject: Tiles excessive memory usage
At my company, we have been recently forced to patch Tiles to solve some
major memory problems where Tiles was eating a lot of memory for no apparent
reasons. We wish to share our findings with you. And we will be happy to
send our code change to Cedric if he wishes so.
Here are some quick numbers about our J2EE runtime after complete bootup
(appserver and EAR file are up, deployed and ready to serve HTTP requests):
- With original Tiles code: 79 MB of RAM is used
- After our custom code change: 28 MB of RAM is used
Note that the Tiles version that we use is an old one (our source zip
shows 09/10/2001). However, a quick analysis of the more recent Tiles source
found in Struts 1.1 RC1 shows that most the code where the defect lies is
still in there. But we haven't actually been able to confirm the problem at
runtime since our app is not compatible with the latest Struts and Tiles
development.
Here are a few excerpt of our internal analysis so that you can understand
the issue.
--
Our application does not actually make use of the Tiles template mechanism
but instead builds on top of Tiles' i18n concept to offer localized Web
pages based on an individual's language, country, personality and channel.
We make extensive use of Tiles Component definitions and inheritance in XML
files, and 667 out of the 668 definitions used by our app inherit from
another one.
With the help of a Java profiler, we showed that 64% of the 79 MB of
memory was used by HashMap entries created by Tiles. This corresponds to
50.5 MB of RAM. After close investigation, it turns out that the very useful
Tiles' component inheritance has been mis-implemented.
Our application has only one definition that doesn't extend another one:
the root "pageDefinition", which contains common information such as
"pageCopyrightLink" for example. Other Component Definitions that extend
"pageDefinition", such as "disbursementCB", do not need to repeat this
information. When queried for the "pageCopyrightLink" value, the
"disbursementCB" component will delegate the processing to its parent
component, in this case "pageDefinition".
Unfortunately, this handy conceptual delegation is actually not
implemented in a similar way in the Tiles source code.
In our example, when the "disbursementCB" component is instantiated in
memory, the Tiles source code does not pass it a reference to the parent
"pageDefiniton" component. Instead, Tiles forces the new "disbursementCB"
component to make a deep copy of all values defined in its parent(s). This
means that, ultimately, our application ends up with 668 copies of the
"pageCopyrightLink" value in memory, instead of one.
After modification of the Tiles source code so that true delegation is
actually happening in memory at runtime, new heap allocation statistics
showed that the problem with excessive usage of HashMap$Entry has been
completely solved. The most in-use object in the JVM is now of the type
"[C", a common and normal trend in typical Java applications.
The total amount of memory was also down to 28 MB. This is a pleasant 51
MB gain over the previous result.
--
Finally, without being too demanding, we would appreciate if a patched
version of Tiles could be made available for for both the upcoming Struts
1.1 release and the current stabe Strust 1.0.2 one.
Best Regards,
Christophe Warland
----------------------------------------------------------------------------
--
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
diff -rub original/org/apache/struts/taglib/tiles/InsertTag.java
updated/org/apache/struts/taglib/tiles/InsertTag.java
--- original/org/apache/struts/taglib/tiles/InsertTag.java 2001-09-10
15:09:18.000000000 -0400
+++ updated/org/apache/struts/taglib/tiles/InsertTag.java 2003-02-28
18:31:24.000000000 -0500
@@ -525,7 +525,7 @@
if( page == null )
page = definition.getTemplate();
// Can check if page is set
- return new InsertHandler( definition.getAttributes(), page, role );
+ return new InsertHandler( definition, page, role );
}
/**
@@ -664,11 +664,11 @@
* Constructor.
* Create insert handler using Component definition.
*/
- public InsertHandler( Map attributes, String page, String role )
+ public InsertHandler( ComponentDefinition definition, String page, String role )
{
this.page = page;
this.role = role;
- subCompContext = new ComponentContext( attributes );
+ subCompContext = new ComponentContext( definition );
}
/**
diff -rub original/org/apache/struts/taglib/tiles/util/TagUtils.java
updated/org/apache/struts/taglib/tiles/util/TagUtils.java
--- original/org/apache/struts/taglib/tiles/util/TagUtils.java 2001-09-10
15:09:18.000000000 -0400
+++ updated/org/apache/struts/taglib/tiles/util/TagUtils.java 2003-02-28
18:29:46.000000000 -0500
@@ -8,7 +8,7 @@
package org.apache.struts.taglib.tiles.util;
-import org.apache.commons.beanutils.PropertyUtils;
+import org.apache.struts.util.PropertyUtils;
import org.apache.struts.taglib.tiles.ComponentConstants;
import org.apache.struts.tiles.ComponentContext;
diff -rub original/org/apache/struts/tiles/ActionComponentServlet.java
updated/org/apache/struts/tiles/ActionComponentServlet.java
--- original/org/apache/struts/tiles/ActionComponentServlet.java 2001-09-10
15:09:18.000000000 -0400
+++ updated/org/apache/struts/tiles/ActionComponentServlet.java 2003-02-28
17:28:46.000000000 -0500
@@ -210,11 +210,11 @@
uri = definition.getPath();
if( tileContext == null )
{
- tileContext = new ComponentContext( definition.getAttributes() );
+ tileContext = new ComponentContext(definition);
ComponentContext.setContext( tileContext, request);
}
else
- tileContext.addMissing( definition.getAttributes() );
+ tileContext.addMissing(definition);
} // end if
} // end if
// Check if there is a definition set in jsp context.
@@ -226,11 +226,11 @@
uri = definition.getPath();
if( tileContext == null )
{
- tileContext = new ComponentContext( definition.getAttributes() );
+ tileContext = new ComponentContext(definition);
ComponentContext.setContext( tileContext, request);
}
else
- tileContext.addMissing( definition.getAttributes() );
+ tileContext.addMissing(definition);
} // end if
}
diff -rub original/org/apache/struts/tiles/ComponentContext.java
updated/org/apache/struts/tiles/ComponentContext.java
--- original/org/apache/struts/tiles/ComponentContext.java 2001-09-10
15:09:18.000000000 -0400
+++ updated/org/apache/struts/tiles/ComponentContext.java 2003-02-28
19:40:38.000000000 -0500
@@ -9,6 +9,8 @@
package org.apache.struts.tiles;
+import java.util.ArrayList;
+import java.util.List;
import java.util.Map;
import java.util.HashMap;
import java.util.Iterator;
@@ -23,11 +25,65 @@
*/
public class ComponentContext
{
+ /** Compound iterator
+ */
+
+ class NameIteratorAggregate implements Iterator {
+
+ Iterator chain;
+ Iterator active;
+ Iterator lastActive;
+
+ NameIteratorAggregate() {
+ List iterators = new ArrayList();
+ iterators.add(delegate.attributes.keySet().iterator());
+ if(missingDefinitions != null) {
+ for (Iterator it = missingDefinitions.iterator(); it.hasNext();) {
+ ComponentDefinition elt =
(ComponentDefinition) it.next();
+
iterators.add(elt.attributes.keySet().iterator());
+ }
+ }
+ chain = iterators.iterator();
+ active = (Iterator) chain.next();
+ }
+
+ public boolean hasNext() {
+ while(! active.hasNext() && chain.hasNext()) {
+ active = (Iterator) chain.next();
+ }
+
+ return active.hasNext();
+ }
+
+ public Object next() {
+ while(! active.hasNext() && chain.hasNext()) {
+ active = (Iterator) chain.next();
+ }
+
+ lastActive = active;
+ return active.next();
+ }
+
+ public void remove() {
+ if(lastActive == null) {
+ throw new IllegalStateException();
+ }
+
+ lastActive.remove();
+ lastActive = null;
+ }
+
+ }
/**
- * Component attributes.
+ * underlying ComponentDefinition.
+ */
+ private ComponentDefinition delegate;
+
+ /**
+ * underlying missing ComponentDefinitions.
*/
- private Map attributes;
+ private List missingDefinitions;
/**
* Component attributes.
@@ -39,51 +95,17 @@
*/
public ComponentContext()
{
+ this(new ComponentDefinition());
}
/**
* Constructor.
- * @deprecated Use ComponentContext( Map attributes ) instead.
*/
public ComponentContext( ComponentDefinition instance )
{
-// try
-// {
- // instance's attributes map is never null.
- attributes = new HashMap(instance.getAttributes());
-// }
-// catch( NullPointerException ex )
-// { // no attributes in instance : silently fail.
-// }
+ delegate = instance;
}
- /**
- * Constructor.
- * Create a context and set specified attributes.
- * @param attributes Attributes to initialize context
- */
- public ComponentContext( Map attributes )
- {
- if( attributes != null )
- this.attributes = new HashMap(attributes);
- }
-
- /**
- * Add all attributes to this context.
- * Copies all of the mappings from the specified attribute map to this context.
- * New attribute mappings will replace any mappings that this context had for any
of the keys
- * currently in the specified attribute map.
- * @param attributes to add.
- */
- public void addAll(Map newAttributes)
- {
- if( attributes == null )
- {
- attributes = new HashMap(newAttributes);
- return;
- }
- attributes.putAll( newAttributes );
- }
/**
* Add all missing attributes to this context.
@@ -92,26 +114,12 @@
* this context.
* @param attributes to add.
*/
- public void addMissing(Map defaultAttributes)
- {
- if( defaultAttributes == null )
- return;
- if( attributes == null )
+ public void addMissing(ComponentDefinition missing)
{
- attributes = new HashMap(defaultAttributes);
- return;
+ if(missingDefinitions == null) {
+ missingDefinitions = new ArrayList();
}
-
- Set entries = defaultAttributes.entrySet();
- Iterator iterator = entries.iterator();
- while( iterator.hasNext() )
- {
- Map.Entry entry = (Map.Entry)iterator.next();
- if( !attributes.containsKey( entry.getKey()) )
- {
- attributes.put(entry.getKey(), entry.getValue());
- } // end if
- } // end loop
+ missingDefinitions.add(missing);
}
/**
@@ -121,9 +129,19 @@
*/
public Object getAttribute(String name)
{
- if( attributes == null )
- return null;
- return attributes.get( name );
+ Object value = delegate.getAttribute(name);
+ if(value == null && missingDefinitions != null) {
+ for (Iterator it = missingDefinitions.iterator(); it.hasNext();) {
+ ComponentDefinition missing = (ComponentDefinition) it.next();
+ if( (value = missing.getAttribute(name)) != null) {
+ break;
+ }
+ }
+ }
+
+ // if(value == null) System.out.println("[ComponentContext] null value for: " +
name + " in " + delegate.getPath());
+
+ return value;
}
/**
@@ -133,9 +151,7 @@
*/
public Iterator getAttributeNames()
{
- if( attributes == null )
- return EMPTY_ITERATOR;
- return attributes.keySet().iterator();
+ return new NameIteratorAggregate();
}
/**
@@ -147,10 +163,7 @@
*/
public void putAttribute(String name, Object value)
{
- if( attributes == null )
- attributes = new HashMap();
-
- attributes.put( name, value );
+ delegate.put(name, value);
}
/**
diff -rub original/org/apache/struts/tiles/ComponentDefinition.java
updated/org/apache/struts/tiles/ComponentDefinition.java
--- original/org/apache/struts/tiles/ComponentDefinition.java 2001-09-10
15:09:20.000000000 -0400
+++ updated/org/apache/struts/tiles/ComponentDefinition.java 2003-02-28
18:45:36.000000000 -0500
@@ -37,6 +37,10 @@
protected Map attributes;
/** role associated to definition */
protected String role;
+ /** parentDefinition used to implement inheritance*/
+ protected ComponentDefinition parentDefinition;
+ /** overloadDefinition used to implement overload child relationships*/
+ protected ComponentDefinition overloadDefinition;
/**
* @return void
@@ -66,7 +70,8 @@
*/
public ComponentDefinition( ComponentDefinition definition )
{
- attributes = new HashMap( definition.getAttributes() );
+ parentDefinition = definition;
+ attributes = new HashMap();
this.name = definition.getName();
this.path = definition.getPath();
this.role = definition.getRole();
@@ -163,16 +168,6 @@
}
/**
- * Access method for the attributes property.
- * If there is no attributes, return an empty map.
- * @return the current value of the attributes property
- */
- public Map getAttributes()
- {
- return attributes;
- }
-
- /**
* Returns the value of the named attribute as an Object, or null if no
* attribute of the given name exists.
*
@@ -180,7 +175,21 @@
*/
public Object getAttribute(String key)
{
- return attributes.get( key);
+ // 1. get from overload -- 2. get from attributes -- 3. get from parent
+ Object value = null;
+ // 1.
+ if(overloadDefinition != null) {
+ value = overloadDefinition.getAttribute(key);
+ }
+ // 2.
+ if(value == null) {
+ value = attributes.get(key);
+ }
+ // 3.
+ if(value == null && parentDefinition != null) {
+ value = parentDefinition.getAttribute(key);
+ }
+ return value;
}
/**
diff -rub original/org/apache/struts/tiles/xmlDefinition/DefinitionsFactory.java
updated/org/apache/struts/tiles/xmlDefinition/DefinitionsFactory.java
--- original/org/apache/struts/tiles/xmlDefinition/DefinitionsFactory.java
2001-09-10 15:09:18.000000000 -0400
+++ updated/org/apache/struts/tiles/xmlDefinition/DefinitionsFactory.java
2003-02-28 12:40:44.000000000 -0500
@@ -67,7 +67,7 @@
while( i.hasNext() )
{
XmlDefinition xmlDefinition = (XmlDefinition)i.next();
- putDefinition( new ComponentDefinition( xmlDefinition) );
+ putDefinition(xmlDefinition);
} // end loop
}
diff -rub original/org/apache/struts/tiles/xmlDefinition/XmlDefinition.java
updated/org/apache/struts/tiles/xmlDefinition/XmlDefinition.java
--- original/org/apache/struts/tiles/xmlDefinition/XmlDefinition.java 2001-09-10
15:09:18.000000000 -0400
+++ updated/org/apache/struts/tiles/xmlDefinition/XmlDefinition.java 2003-02-28
18:46:24.000000000 -0500
@@ -13,7 +13,7 @@
public class XmlDefinition extends ComponentDefinition
{
/** Debug flag */
- static public final boolean debug = true;
+ static public final boolean debug = false;
/**
* Extends attribute value.
*/
@@ -119,17 +119,18 @@
parent.resolveInheritance( definitionsSet );
- // Iterate on each parent's attribute, and add it if not defined in child.
- Iterator parentAttributes = parent.getAttributes().keySet().iterator();
- while( parentAttributes.hasNext() )
- {
- String name = (String)parentAttributes.next();
- if( !getAttributes().containsKey(name) )
- putAttribute( name, parent.getAttribute(name) );
- }
- // Set path
+ // add parentDefinition
+ parentDefinition = parent;
+
+ /** Set path - change by padma ginnaram
+ * bug in tiles, the parent definition path
+ * should not be used if the child has the path
+ */
+
+ if (this.path == null) {
setPath( parent.getPath() );
}
+ }
/**
* Overload this definition with passed child.
@@ -152,7 +153,7 @@
{
role = child.getRole();
}
- // put all child attributes in parent.
- attributes.putAll( child.getAttributes());
+ // put child as overload def.
+ overloadDefinition = child;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
