You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by ad...@apache.org on 2006/12/17 13:00:01 UTC

svn commit: r487972 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java

Author: adelmelle
Date: Sun Dec 17 04:00:00 2006
New Revision: 487972

URL: http://svn.apache.org/viewvc?view=rev&rev=487972
Log:
Optimization in Marker.java: reduction of distinct MarkerAttribute instances (see also Bugzilla 41044)

Modified:
    xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java

Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java?view=diff&rev=487972&r1=487971&r2=487972
==============================================================================
--- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java (original)
+++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java Sun Dec 17 04:00:00 2006
@@ -19,7 +19,9 @@
 
 package org.apache.fop.fo.flow;
 
-import java.util.HashMap;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
 
 import org.xml.sax.Attributes;
 import org.xml.sax.Locator;
@@ -29,7 +31,6 @@
 import org.apache.fop.fo.FONode;
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.FObjMixed;
-import org.apache.fop.fo.FOPropertyMapping;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.PropertyListMaker;
 import org.apache.fop.fo.ValidationException;
@@ -44,7 +45,7 @@
     // End of property values
 
     private PropertyListMaker savePropertyListMaker;
-    private HashMap descendantPropertyLists = new HashMap();
+    private Map descendantPropertyLists = new java.util.HashMap();
 
     /**
      * Create a marker fo.
@@ -60,8 +61,8 @@
     public void bind(PropertyList pList) throws FOPException {
         if (findAncestor(FO_FLOW) < 0) {
             invalidChildError(locator, FO_URI, "marker", 
-                "An fo:marker is permitted only as the descendant " +
-                "of an fo:flow");
+                "An fo:marker is permitted only as the descendant " 
+                    + "of an fo:flow");
         }
         
         markerClassName = pList.get(PR_MARKER_CLASS_NAME).getString();
@@ -147,7 +148,7 @@
         sb.append(" {").append(getMarkerClassName()).append("}");
         return sb.toString();
     }
-    
+
     /**
      * An implementation of PropertyList which only stores the explicitly
      * specified properties/attributes as bundles of name-value-namespace
@@ -155,42 +156,7 @@
      */
     protected class MarkerPropertyList extends PropertyList 
             implements Attributes {
-        
-        protected class MarkerAttribute {
-            
-            protected String namespace;
-            protected String qname;
-            protected String name;
-            protected String value;
-            
-            /**
-             * Main constructor
-             * @param namespace the namespace URI
-             * @param qname the qualified name
-             * @param name  the name
-             * @param value the value
-             */
-            public MarkerAttribute(String namespace, String qname, 
-                    String name, String value) {
-                this.namespace = namespace;
-                this.qname = qname;
-                this.name = (name == null ? qname : name);
-                this.value = value;
-            }
-            
-            /**
-             * Convenience constructor for FO attributes
-             * @param name  the attribute name
-             * @param value the attribute value
-             */
-            public MarkerAttribute(String name, String value) {
-                this.namespace = null;
-                this.qname = name;
-                this.name = name;
-                this.value = value;
-            }
-        }
-        
+                
         /** the array of attributes **/
         private MarkerAttribute[] attribs;
         
@@ -230,8 +196,13 @@
                 name = attributes.getLocalName(i);
                 value = attributes.getValue(i);
                 
-                this.attribs[i] = 
-                    new MarkerAttribute(namespace, qname, name, value);
+                if (namespace == null || "".equals(namespace)) {
+                    this.attribs[i] = 
+                        MarkerAttribute.getFOAttributeInstance(name, value);
+                } else {
+                    this.attribs[i] = 
+                        new MarkerAttribute(namespace, qname, name, value);
+                }
             }
         }
         
@@ -395,6 +366,66 @@
                 return getValue(index);
             }
             return null;
+        }
+    }
+    
+    /**
+     * Convenience inner class
+     */
+    private static final class MarkerAttribute {
+        
+        private static Map foAttributeCache = 
+            Collections.synchronizedMap(new java.util.HashMap());
+
+        protected String namespace;
+        protected String qname;
+        protected String name;
+        protected String value;
+                    
+        /**
+         * Main constructor
+         * @param namespace the namespace URI
+         * @param qname the qualified name
+         * @param name  the name
+         * @param value the value
+         */
+        private MarkerAttribute(String namespace, String qname, 
+                                    String name, String value) {
+            this.namespace = namespace;
+            this.qname = qname;
+            this.name = (name == null ? qname : name);
+            this.value = value;
+        }
+        
+        /**
+         * Convenience method, reduces the number
+         * of distinct MarkerAttribute instances
+         * 
+         * @param name  the attribute name
+         * @param value the attribute value
+         * @return the single MarkerAttribute instance corresponding to 
+         *          the name/value-pair
+         */
+        private static MarkerAttribute getFOAttributeInstance(
+                                            String name, String value) {
+            MarkerAttribute newInstance = null;
+            Map valueCache;
+            if (!foAttributeCache.containsKey(name)) {
+                newInstance = new MarkerAttribute(null, name, name, value);
+                valueCache = Collections.synchronizedMap(
+                                new java.util.HashMap());
+                valueCache.put(value, newInstance);
+                foAttributeCache.put(name, valueCache);
+            } else {
+                valueCache = (Map) foAttributeCache.get(name);
+                if (valueCache.containsKey(value)) {
+                    newInstance = (MarkerAttribute) valueCache.get(value);
+                } else {
+                    newInstance = new MarkerAttribute(null, name, name, value);
+                    valueCache.put(value, newInstance);
+                }
+            }
+            return newInstance;
         }
     }
 }



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r487972 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java

Posted by Andreas L Delmelle <a_...@pandora.be>.
On Dec 20, 2006, at 21:10, Andreas L Delmelle wrote:

> On Dec 20, 2006, at 20:45, Jeremias Maerki wrote:
>
>> I wonder about the effect of that on very long running server
>> applications producing all kinds of different documents. There's no
>> chance for freeing instances here if memory is needed. I assume  
>> that in
>> this case the set of instances will still remain relatively small.  
>> But
>> still, this is memory that is never freed and some instances may  
>> never
>> be reused after a particular rendering run.
>
> <snip />
> Anyway, I'll see if I can adapt to use WeakHashMap too. Should work.

Done. Committed to the release branch as well.

Cheers,

Andreas


Re: svn commit: r487972 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java

Posted by Andreas L Delmelle <a_...@pandora.be>.
On Dec 20, 2006, at 20:45, Jeremias Maerki wrote:

> I wonder about the effect of that on very long running server
> applications producing all kinds of different documents. There's no
> chance for freeing instances here if memory is needed. I assume  
> that in
> this case the set of instances will still remain relatively small. But
> still, this is memory that is never freed and some instances may never
> be reused after a particular rendering run.

True indeed. My bad. Richard was a bit smarter, and used a  
WeakHashMap, so that the instances that are no longer referenced can  
be released.
OTOH, since the access to the propertyCaches is not synchronized in  
Richard's patch, I'm wondering whether or not the possibility arises  
that two parallel threads issue a put() on the Map at the same time  
for an equal instance...?

Maybe we're both slightly off...

Anyway, I'll see if I can adapt to use WeakHashMap too. Should work.

Thanks for the vigilance!

Cheers,

Andreas


Re: svn commit: r487972 - /xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
I know I'm f**ing late to chime in here. But I haven't read all content
on fop-dev from this months, yet, and I'm currently going through the
SVN changes. Sorry. Anyway...

I wonder about the effect of that on very long running server
applications producing all kinds of different documents. There's no
chance for freeing instances here if memory is needed. I assume that in
this case the set of instances will still remain relatively small. But
still, this is memory that is never freed and some instances may never
be reused after a particular rendering run.

On 17.12.2006 13:00:01 adelmelle wrote:
> Author: adelmelle
> Date: Sun Dec 17 04:00:00 2006
> New Revision: 487972
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=487972
> Log:
> Optimization in Marker.java: reduction of distinct MarkerAttribute instances (see also Bugzilla 41044)
> 
> Modified:
>     xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java
> 
> Modified: xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java
> URL: http://svn.apache.org/viewvc/xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java?view=diff&rev=487972&r1=487971&r2=487972
> ==============================================================================
> --- xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java (original)
> +++ xmlgraphics/fop/trunk/src/java/org/apache/fop/fo/flow/Marker.java Sun Dec 17 04:00:00 2006
<snip/>
> +    private static final class MarkerAttribute {
> +        
> +        private static Map foAttributeCache = 
> +            Collections.synchronizedMap(new java.util.HashMap());
> +
> +        protected String namespace;
> +        protected String qname;
> +        protected String name;
> +        protected String value;
<snip/>

Jeremias Maerki