polx 2004/12/27 17:39:23
Modified: jelly/src/java/org/apache/commons/jelly/impl
StaticTagScript.java TagScript.java
jelly/src/java/org/apache/commons/jelly JellyContext.java
jelly/jelly-tags/xml/src/test/org/apache/commons/jelly/tags/xml
TestParser.java
jelly/jelly-tags/xml/src/java/org/apache/commons/jelly/tags/xml
TransformTag.java
jelly/src/java/org/apache/commons/jelly/util TagUtils.java
Added: jelly/src/test/org/apache/commons/jelly/core
TestCoreMemoryLeak.java BaseMemoryLeakTest.java
Removed: jelly/src/java/org/apache/commons/jelly/impl
WeakReferenceWrapperScript.java
Log:
Tag caching is now done in the JellyContext.
The TestCoreMemoryLeak shows that, at least with some usage of
JellyContext.clear(), one avoids leaks.
Removing Tag-caching flag. Also, removing the WeakReferenceWrapperScript,
both were in the era of ThreadLocal which is to be finished.
Have tested these with an amount of the tag-libs... had to change the XML one
but otherwise apparently none.
Feedback and testing eagerly welcome!
paul
Revision Changes Path
1.25 +2 -2
jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/StaticTagScript.java
Index: StaticTagScript.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/StaticTagScript.java,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -r1.24 -r1.25
--- StaticTagScript.java 16 Sep 2004 01:12:11 -0000 1.24
+++ StaticTagScript.java 28 Dec 2004 01:39:22 -0000 1.25
@@ -59,14 +59,14 @@
Tag tag = null;
try {
- tag = getTag();
+ tag = getTag(context);
// lets see if we have a dynamic tag
if (tag instanceof StaticTag) {
tag = findDynamicTag(context, (StaticTag) tag);
}
- setTag(tag);
+ setTag(tag,context);
} catch (JellyException e) {
throw new JellyTagException(e);
}
1.47 +12 -31
jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/TagScript.java
Index: TagScript.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/TagScript.java,v
retrieving revision 1.46
retrieving revision 1.47
diff -u -r1.46 -r1.47
--- TagScript.java 27 Oct 2004 21:50:53 -0000 1.46
+++ TagScript.java 28 Dec 2004 01:39:22 -0000 1.47
@@ -61,15 +61,6 @@
/** The Log to which logging calls will be made. */
private static final Log log = LogFactory.getLog(TagScript.class);
- /**
- * Thread local storage for the tag used by the current thread.
- * This allows us to pool tag instances, per thread to reduce object
construction
- * over head, if we need it.
- *
- * Note that we could use the stack and create a new tag for each
invocation
- * if we made a slight change to the Script API to pass in the parent
tag.
- */
- private ThreadLocal tagHolder = new ThreadLocal();
/** The attribute expressions that are created */
protected Map attributes = new Hashtable();
@@ -194,11 +185,8 @@
public void run(JellyContext context, XMLOutput output) throws
JellyTagException {
URL rootURL = context.getRootURL();
URL currentURL = context.getCurrentURL();
- if ( ! context.isCacheTags() ) {
- clearTag();
- }
try {
- Tag tag = getTag();
+ Tag tag = getTag(context);
if ( tag == null ) {
return;
}
@@ -295,15 +283,15 @@
/**
* @return the tag to be evaluated, creating it lazily if required.
*/
- public Tag getTag() throws JellyException {
- Tag tag = (Tag) tagHolder.get();
+ public Tag getTag(JellyContext context) throws JellyException {
+ Tag tag = context.getTagOfTagScript(this);
if ( tag == null ) {
tag = createTag();
if ( tag != null ) {
- tagHolder.set(tag);
+ context.setTagForScript(this,tag);
}
}
- configureTag(tag);
+ configureTag(tag,context);
return tag;
}
@@ -494,21 +482,20 @@
return null;
}
-
+
/**
* Compiles a newly created tag if required, sets its parent and body.
*/
- protected void configureTag(Tag tag) throws JellyException {
+ protected void configureTag(Tag tag, JellyContext context) throws
JellyException {
if (tag instanceof CompilableTag) {
((CompilableTag) tag).compile();
}
Tag parentTag = null;
if ( parent != null ) {
- parentTag = parent.getTag();
+ parentTag = parent.getTag(context);
}
tag.setParent( parentTag );
- tag.setBody( new WeakReferenceWrapperScript(tagBody) );
- //tag.setBody( tagBody );
+ tag.setBody( tagBody );
if (tag instanceof NamespaceAwareTag) {
NamespaceAwareTag naTag = (NamespaceAwareTag) tag;
@@ -519,19 +506,13 @@
}
}
- /**
- * Flushes the current cached tag so that it will be created, lazily,
next invocation
- */
- protected void clearTag() {
- tagHolder.set(null);
- }
/**
* Allows the script to set the tag instance to be used, such as in a
StaticTagScript
* when a StaticTag is switched with a DynamicTag
*/
- protected void setTag(Tag tag) {
- tagHolder.set(tag);
+ protected void setTag(Tag tag, JellyContext context) {
+ context.setTagForScript(this,tag);
}
/**
1.62 +39 -27
jakarta-commons/jelly/src/java/org/apache/commons/jelly/JellyContext.java
Index: JellyContext.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/JellyContext.java,v
retrieving revision 1.61
retrieving revision 1.62
diff -u -r1.61 -r1.62
--- JellyContext.java 9 Sep 2004 15:10:03 -0000 1.61
+++ JellyContext.java 28 Dec 2004 01:39:23 -0000 1.62
@@ -25,6 +25,7 @@
import java.util.Map;
import org.apache.commons.jelly.parser.XMLParser;
+import org.apache.commons.jelly.impl.TagScript;
import org.apache.commons.jelly.util.ClassLoaderUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -90,8 +91,15 @@
/** Should we export tag libraries to our parents context */
private boolean exportLibraries = true;
- /** Should we cache Tag instances, per thread, to reduce object
contruction overhead? */
- private boolean cacheTags = false;
+ /** A map that associates TagScript objects to Tag objects.
+ * Is made a WeakHashMap so that the storage is discarded if the
+ * Script is discarded.
+ */
+ private Map tagHolderMap = new java.util.WeakHashMap(5);
+ // THINKME: Script objects are like Object (for equals and hashCode) I
think.
+ // It should be asy to optimize hash-map distribution, e.g. by
+ // shifting the hashcode return value (presuming
Object.hashcode()
+ // is something like an address)
/**
* Create a new context with the currentURL set to the rootURL
@@ -133,7 +141,6 @@
this.rootURL = parent.rootURL;
this.currentURL = parent.currentURL;
this.variables.put("parentScope", parent.variables);
- this.cacheTags = parent.cacheTags;
init();
}
@@ -377,7 +384,36 @@
public JellyContext newJellyContext() {
return createChildContext();
}
+
+ /** @return the tag associated to the given TagScript for the current
run.
+ */
+ public Tag getTagOfTagScript(TagScript script) {
+ if( script == null )
+ return null;
+ return (Tag) tagHolderMap.get(script);
+ }
+
+ /** @return the Map that associates the the Tags to Scripts */
+ public Map getTagHolderMap() {
+ return tagHolderMap;
+ }
+
+ /** Associates, for the run of this context, the tag of this TagScript.
+ */
+ public void setTagForScript(TagScript script, Tag tag) {
+ tagHolderMap.put(script,tag);
+ }
+
+ /** Clears both the script-to-tags association and the variables
association.
+ */
+ public void clear() {
+ tagHolderMap.clear();
+ variables.clear();
+ }
+
+
+
/** Registers the given tag library against the given namespace URI.
* This should be called before the parser is used.
*/
@@ -787,30 +823,6 @@
*/
public void setCurrentURL(URL currentURL) {
this.currentURL = currentURL;
- }
-
- /**
- * Returns whether caching of Tag instances, per thread, is enabled.
- * Caching Tags can boost performance, on some JVMs, by reducing the
cost of
- * object construction when running Jelly inside a multi-threaded
application server
- * such as a Servlet engine.
- *
- * @return whether caching of Tag instances is enabled.
- */
- public boolean isCacheTags() {
- return cacheTags;
- }
-
- /**
- * Sets whether caching of Tag instances, per thread, is enabled.
- * Caching Tags can boost performance, on some JVMs, by reducing the
cost of
- * object construction when running Jelly inside a multi-threaded
application server
- * such as a Servlet engine.
- *
- * @param cacheTags Whether caching should be enabled or disabled.
- */
- public void setCacheTags(boolean cacheTags) {
- this.cacheTags = cacheTags;
}
/**
1.6 +9 -6
jakarta-commons/jelly/jelly-tags/xml/src/test/org/apache/commons/jelly/tags/xml/TestParser.java
Index: TestParser.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/jelly-tags/xml/src/test/org/apache/commons/jelly/tags/xml/TestParser.java,v
retrieving revision 1.5
retrieving revision 1.6
diff -u -r1.5 -r1.6
--- TestParser.java 9 Sep 2004 12:24:41 -0000 1.5
+++ TestParser.java 28 Dec 2004 01:39:23 -0000 1.6
@@ -26,6 +26,7 @@
import org.apache.commons.jelly.Script;
import org.apache.commons.jelly.Tag;
+import org.apache.commons.jelly.JellyContext;
import org.apache.commons.jelly.impl.ScriptBlock;
import org.apache.commons.jelly.impl.TagScript;
import org.apache.commons.jelly.parser.XMLParser;
@@ -66,26 +67,28 @@
log.debug("Found: " + script);
- assertTagsHaveParent( script, null );
+ assertTagsHaveParent( script, null, null );
}
/**
* Tests that the Tag in the TagScript has the given parent and then
* recurse to check its children has the correct parent and so forth.
*/
- protected void assertTagsHaveParent(Script script, Tag parent) throws
Exception {
+ protected void assertTagsHaveParent(Script script, Tag parent,
JellyContext context) throws Exception {
+ if ( context == null )
+ context = new JellyContext();
if ( script instanceof TagScript ) {
TagScript tagScript = (TagScript) script;
- Tag tag = tagScript.getTag();
+ Tag tag = tagScript.getTag(context);
assertEquals( "Tag: " + tag + " has the incorrect parent",
parent, tag.getParent() );
- assertTagsHaveParent( tag.getBody(), tag );
+ assertTagsHaveParent( tag.getBody(), tag, context );
}
else if ( script instanceof ScriptBlock ) {
ScriptBlock block = (ScriptBlock) script;
for ( Iterator iter = block.getScriptList().iterator();
iter.hasNext(); ) {
- assertTagsHaveParent( (Script) iter.next(), parent );
+ assertTagsHaveParent( (Script) iter.next(), parent, context
);
}
}
}
1.7 +4 -4
jakarta-commons/jelly/jelly-tags/xml/src/java/org/apache/commons/jelly/tags/xml/TransformTag.java
Index: TransformTag.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/jelly-tags/xml/src/java/org/apache/commons/jelly/tags/xml/TransformTag.java,v
retrieving revision 1.6
retrieving revision 1.7
diff -u -r1.6 -r1.7
--- TransformTag.java 27 Oct 2004 21:51:13 -0000 1.6
+++ TransformTag.java 28 Dec 2004 01:39:23 -0000 1.7
@@ -372,7 +372,7 @@
Tag tag = null;
try {
- tag = ((TagScript) script).getTag();
+ tag = ((TagScript) script).getTag(getContext());
} catch (JellyException e) {
throw new JellyTagException(e);
}
@@ -414,7 +414,7 @@
Tag tag = null;
try {
- tag = ((TagScript) script).getTag();
+ tag = ((TagScript) script).getTag(getContext());
} catch (JellyException e) {
throw new JellyTagException(e);
}
1.1
jakarta-commons/jelly/src/test/org/apache/commons/jelly/core/TestCoreMemoryLeak.java
Index: TestCoreMemoryLeak.java
===================================================================
/*
* Copyright 2002,2004 The Apache Software Foundation.
*
* 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.apache.commons.jelly.core;
/** Tests for basic memory leaking in core tags. Runs a few test scripts many
times.
* @author Hans Gilde
*
*/
public class TestCoreMemoryLeak extends BaseMemoryLeakTest {
/** The JUnit constructor.
* @param name
*/
public TestCoreMemoryLeak(String name) {
super(name);
}
public void testBasicScriptForLeak() throws Exception {
assertTrue(runScriptManyTimes("c.jelly", 10000) < 200000);
}
}
1.1
jakarta-commons/jelly/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java
Index: BaseMemoryLeakTest.java
===================================================================
/*
* Copyright 2002,2004 The Apache Software Foundation.
*
* 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.apache.commons.jelly.core;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import junit.framework.TestCase;
import org.apache.commons.jelly.JellyContext;
import org.apache.commons.jelly.JellyException;
import org.apache.commons.jelly.Script;
import org.apache.commons.jelly.XMLOutput;
import org.apache.commons.jelly.parser.XMLParser;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
/**
* Automates the basic process of testing a tag library for a memory leak.
* <p>
* To use it, extend it. Use the [EMAIL PROTECTED] runScriptManyTimes(String,
int)}
* method in your unit tests.
*
* @author Hans Gilde
*
*/
public class BaseMemoryLeakTest extends TestCase {
private final static Log log =
LogFactory.getLog(BaseMemoryLeakTest.class);
/**
* The JUnit constructor
*
* @param name
*/
public BaseMemoryLeakTest(String name) {
super(name);
}
/** Runs a script count times and reports the number of bytes "leaked".
* Note that "leaked" means "not collected by the GC"
* and can easily be different between JVM's. This is because all
* freed references may not be available for GC in the short time
* between their freeing and the completion of this test.
* <p/>
* However, running a
* script 10,000 or 100,000 times should be a pretty good test
* for a memory leak. If there's not too much memory "leaked",
* you're probably OK.
* @param scriptName The path to the script, from the classloader of the
current class.
* @param count The number of times to run the script.
* @return The number of bytes "leaked"
* @throws IOException
* @throws SAXException
* @throws JellyException
*/
public long runScriptManyTimes(String scriptName, int count)
throws IOException, SAXException, JellyException {
Runtime rt = Runtime.getRuntime();
JellyContext jc = new JellyContext();
jc.setClassLoader(getClass().getClassLoader());
XMLOutput output = XMLOutput.createDummyXMLOutput();
URL url = this.getClass().getResource(scriptName);
String exturl = url.toExternalForm();
int lastSlash = exturl.lastIndexOf("/");
String extBase = exturl.substring(0,lastSlash+1);
URL baseurl = new URL(extBase);
jc.setCurrentURL(baseurl);
InputStream is = url.openStream();
byte[] bytes = new byte[is.available()];
is.read(bytes);
InputStream scriptIStream = new ByteArrayInputStream(bytes);
InputSource scriptISource = new InputSource(scriptIStream);
is.close();
is = null;
bytes = null;
rt.runFinalization();
rt.gc();
long start = rt.totalMemory() - rt.freeMemory();
log.info("Starting memory test with used memory of " + start);
XMLParser parser;
Script script;
int outputEveryXIterations = outputEveryXIterations();
for (int i = 0; i < count; i++) {
scriptIStream.reset();
parser = new XMLParser();
script = parser.parse(scriptISource);
script.run(jc, output);
log.info("TagHolderMap has " +
jc.getTagHolderMap().size() + " entries.");
// PL: I don't see why but removing the clear here
// does make the test fail!
// As if the WeakHashMap wasn't weak enough...
jc.clear();
if (outputEveryXIterations != 0 && i % outputEveryXIterations ==
0) {
parser = null;
script = null;
rt.runFinalization();
rt.gc();
long middle = rt.totalMemory() - rt.freeMemory();
log.info("Memory test after " + i + " runs: "
+ (middle - start));
}
}
rt.gc();
jc = null;
output = null;
parser = null;
script = null;
scriptIStream = null;
scriptISource = null;
rt.runFinalization();
rt.gc();
long nullsDone = rt.totalMemory() - rt.freeMemory();
log.info("Memory test completed, memory \"leaked\": " + (nullsDone -
start));
return nullsDone - start;
}
protected int outputEveryXIterations() {
return 1000;
}
}
1.2 +1 -10
jakarta-commons/jelly/src/java/org/apache/commons/jelly/util/TagUtils.java
Index: TagUtils.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/util/TagUtils.java,v
retrieving revision 1.1
retrieving revision 1.2
diff -u -r1.1 -r1.2
--- TagUtils.java 27 Oct 2004 21:49:28 -0000 1.1
+++ TagUtils.java 28 Dec 2004 01:39:23 -0000 1.2
@@ -20,7 +20,6 @@
import org.apache.commons.jelly.impl.CompositeTextScriptBlock;
import org.apache.commons.jelly.impl.ScriptBlock;
import org.apache.commons.jelly.impl.TextScript;
-import org.apache.commons.jelly.impl.WeakReferenceWrapperScript;
/** Contains static methods to help tag developers.
* @author Hans Gilde
@@ -36,15 +35,7 @@
*/
public static void trimScript(Script body) {
synchronized(body) {
- if (body instanceof WeakReferenceWrapperScript) {
- WeakReferenceWrapperScript wrrs =
(WeakReferenceWrapperScript) body;
- try {
- wrrs.trimWhitespace();
- } catch (JellyTagException e) {
- //TODO handle this exception once the Tag interface
allows JellyTagException to be thrown
- return;
- }
- } else if ( body instanceof CompositeTextScriptBlock ) {
+ if ( body instanceof CompositeTextScriptBlock ) {
CompositeTextScriptBlock block = (CompositeTextScriptBlock)
body;
block.trimWhitespace();
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
