commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brett Porter <br...@apache.org>
Subject Re: Memory leak patch
Date Sun, 27 Mar 2005 01:02:50 GMT
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I applied this, and it still has the same problems. It's very likely
Maven is assuming something about the context not in the public contract.

I'm guessing the Maven jelly tag line at fault is this:

    public WerkzProject getProject()
    {
        return (WerkzProject)
getContext().getVariable(MavenConstants.WERKZ_PROJECT);
    }

That's the only real context interaction and getting the wrong project
back would result in goals not being found.

It's voodoo, and something I haven't had to muck with in a year and
don't really care to again - but if you're certain that the patch is
doing the right thing, I should have time in 2-3 weeks to take a peek.
If you have any thoughts, or would like to play around with it yourself,
I'd be grateful... I believe I gave all the necessary instructions in my
last mail.

Thanks,
Brett

Hans Gilde wrote:

>Apparently I did something dumb. The fix that I'm really confident
>about isn't in either CVS or SVN and also isn't in the last patch I
>sent out.
>
>But does it even exist, or did I dream the whole thing? It does exist!
>I'm attaching 3 patches that should apply to the
>latest CSV and should also fix the memory leak in a very simple and
>non-intrusive way so as not to break Maven.
>
>Over the weekend, I'll figure out how these changes relate to what's
>in SVN, the patches may apply properly.
>
>Hans
>
>
>-------------------------
>
>Index: JellyContext.java
>===================================================================
>RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/JellyContext.java,v
>retrieving revision 1.66
>diff -u -r1.66 JellyContext.java
>--- JellyContext.java 27 Jan 2005 05:52:27 -0000 1.66
>+++ JellyContext.java 24 Mar 2005 05:31:20 -0000
>@@ -92,20 +92,6 @@
> /** Should we export tag libraries to our parents context */
> private boolean exportLibraries = true;
>
>- /** Maps a Thread to its local Script data cache. It's
>- * like a ThreadLocal, but it reclaims memory better
>- * when the JellyCointext goes out of scope.
>- * This isn't a ThreadLocal because of the typical usage scenario of
>- * JellyContext. ThreadLocal is meant to be sued as a static variable,
>- * we were using it as a local variable.
>- * {@link #setThreadLocalScriptData(Script,Object)}
>- */
>- private Map threadLocalScriptData = Collections.synchronizedMap(new
WeakHashMap());
>- // 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
> */
>@@ -390,93 +376,20 @@
> return createChildContext();
> }
>
>-
>- /** Gets the Script data item that may have previously been stored
>- * by the script, in this context, for the current thread.
>- *
>- * @return the tag associated with the current context and thread
>- */
>- public Object getThreadScriptData(Script script) {
>- if( script == null )
>- return null;
>- Tag tag = (Tag) getThreadScriptDataMap().get(script);
>- if( tag == null && getParent() != null) {
>- return getParent().getThreadScriptData(script);
>- } else {
>- return tag;
>- }
>- }
>-
>- /** Gets a per-thread (thread local) Map of data for use by
>- * Scripts.
>- * @return the thread local Map of Script data */
>- public Map getThreadScriptDataMap() {
>- Map rv;
>- Thread t = Thread.currentThread();
>- Map data = (Map) threadLocalScriptData.get(t);
>- if (data == null) {
>- rv = new HashMap();
>- threadLocalScriptData.put(t, rv);
>- } else {
>- rv = data;
>- }
>- return rv;
>- }
>-
>- /** Stores an object that lasts for the life of this context
>- * and is local to the current thread. This method is
>- * mainly intended to store Tag instances. However, any
>- * Script that wants to cache data can use this
>- * method.
>- */
>- public void setThreadScriptData(Script script, Object data) {
>- getThreadScriptDataMap().put(script,data);
>- }
>-
>- /** Clears variables set by Tags (basically, variables set in a Jelly
script)
>- * and data stored by {@link Script} instances.
>+ /** Clears variables set by Tags.
> * @see #clearVariables()
>- * @see #clearThreadScriptData()
>- * @see #clearScriptData()
> */
> public void clear() {
>- clearScriptData();
> clearVariables();
> }
>
> /** Clears variables set by Tags (variables set while running a Jelly
script)
> * @see #clear()
>- * @see #clearThreadScriptData()
>- * @see #clearScriptData()
> */
>- public void clearVariables() {
>+ protected void clearVariables() {
> variables.clear();
> }
>
>- /** Clears data cached by {@link Script} instances,
>- * for this context, <strong>for the current thread</strong>.
>- * The data cleared could be cached Tag instances or other data
>- * saved by Script classes.
>- * @see #clear()
>- * @see #clearVariables()
>- * @see #clearScriptData()
>- */
>- public void clearThreadScriptData() {
>- getThreadScriptDataMap().clear();
>- }
>-
>- /** Clears data cached by {@link Script} instances,
>- * for this context, <strong>for all threads</strong>.
>- * The data cleared could be cached Tag instances or other data
>- * saved by Script classes.
>- * @see #clear()
>- * @see #clearThreadScriptData()
>- * @see #clearVariables()
>- */
>- public void clearScriptData() {
>- threadLocalScriptData.clear();
>- }
>-
> /** Registers the given tag library against the given namespace URI.
> * This should be called before the parser is used.
> */
>
>
>-------------------------
>
>Index: TagScript.java
>===================================================================
>RCS file:
/home/cvs/jakarta-commons/jelly/src/java/org/apache/commons/jelly/impl/TagScript.java,v
>retrieving revision 1.50
>diff -u -r1.50 TagScript.java
>--- TagScript.java 20 Jan 2005 05:18:56 -0000 1.50
>+++ TagScript.java 24 Mar 2005 05:31:52 -0000
>@@ -19,9 +19,11 @@
> import java.lang.reflect.InvocationTargetException;
> import java.net.MalformedURLException;
> import java.net.URL;
>+import java.util.Collections;
> import java.util.Hashtable;
> import java.util.Iterator;
> import java.util.Map;
>+import java.util.WeakHashMap;
>
> import org.apache.commons.beanutils.ConvertingWrapDynaBean;
> import org.apache.commons.beanutils.ConvertUtils;
>@@ -104,6 +106,10 @@
>
> /** the url of the script when parsed */
> private URL scriptURL = null;
>+
>+ /** A synchronized WeakHashMap from the current Thread (key) to a Tag
object (value).
>+ */
>+ private Map threadLocalTagCache = Collections.synchronizedMap(new
WeakHashMap());
>
> /**
> * @return a new TagScript based on whether
>@@ -286,11 +292,12 @@
> * @return the tag to be evaluated, creating it lazily if required.
> */
> public Tag getTag(JellyContext context) throws JellyException {
>- Tag tag = (Tag) context.getThreadScriptData(this);
>+ Thread t = Thread.currentThread();
>+ Tag tag = (Tag) threadLocalTagCache.get(t);
> if ( tag == null ) {
> tag = createTag();
> if ( tag != null ) {
>- context.setThreadScriptData(this,tag);
>+ threadLocalTagCache.put(t,tag);
> configureTag(tag,context);
> }
> }
>@@ -514,7 +521,8 @@
> * when a StaticTag is switched with a DynamicTag
> */
> protected void setTag(Tag tag, JellyContext context) {
>- context.setThreadScriptData(this,tag);
>+ Thread t = Thread.currentThread();
>+ threadLocalTagCache.put(t,tag);
> }
>
> /**
>
>
>-------------------------
>
>Index: BaseMemoryLeakTest.java
>===================================================================
>RCS file:
/home/cvs/jakarta-commons/jelly/src/test/org/apache/commons/jelly/core/BaseMemoryLeakTest.java,v
>retrieving revision 1.3
>diff -u -r1.3 BaseMemoryLeakTest.java
>--- BaseMemoryLeakTest.java 20 Jan 2005 05:19:04 -0000 1.3
>+++ BaseMemoryLeakTest.java 24 Mar 2005 05:32:27 -0000
>@@ -130,7 +130,6 @@
> rt.runFinalization();
> rt.gc();
> long middle = rt.totalMemory() - rt.freeMemory();
>- log.info("TagHolderMap has " + jc.getThreadScriptDataMap().size() + "
entries.");
> log.info("Memory test after " + i + " runs: "
> + (middle - start));
> }
>
>
>-------------------------
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCRgY6Ob5RoQhMkRMRAs7gAJ9XBS3MgD5Cxv1JEDLz/fCwbR5kOQCdHAH4
MNAvwfNYkGgb6BiEYx2Vtrk=
=JZuX
-----END PGP SIGNATURE-----


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message