commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From p...@apache.org
Subject cvs commit: jakarta-commons/jelly/src/java/org/apache/commons/jelly/util TagUtils.java
Date Tue, 28 Dec 2004 01:39:23 GMT
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 {@link 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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message