Implemented a cache for property value lookup.

git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@197736 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 119 insertions, 12 deletions

diff --git a/src/java/org/apache/fop/fo/PropertyList.java b/src/java/org/apache/fop/fo/PropertyList.java
index e03a4cacd..39ce8a7f1 100644
--- a/src/java/org/apache/fop/fo/PropertyList.java
+++ b/src/java/org/apache/fop/fo/PropertyList.java
@@ -19,6 +19,7 @@
 package org.apache.fop.fo;
 
 // Java
+import java.util.Map;
 import java.util.HashMap;
 import org.xml.sax.Attributes;
 
@@ -27,6 +28,9 @@ import org.apache.fop.apps.FOPException;
 import org.apache.fop.fo.properties.Property;
 import org.apache.fop.fo.properties.PropertyMaker;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 /**
  * Class containing the collection of properties for a given FObj.
  */
@@ -97,6 +101,14 @@ public class PropertyList extends HashMap {
     private String namespace = "";
     private FObj fobj = null;
 
+    private Log log = LogFactory.getLog(PropertyList.class);
+
+    /**
+     * Cache for properties looked up via maker.findProperty
+     * with bTryInherit == true
+     */
+    private Map cache = new HashMap();
+
     /**
      * Basic constructor.
      * @param parentPropertyList the PropertyList belonging to the new objects
@@ -247,7 +259,73 @@ public class PropertyList extends HashMap {
         return null;
     }
 
+    /**
+     * Wrapper around PropertyMaker.findProperty using the cache;
+     * use this method only if bTryInherit == true.
+     * The propertyMaker parameter is there
+     * to avoid repeated lookup of the maker
+     * in an alternating sequence of calls
+     * between findProperty and maker.findProperty.
+     * This would not be valid for FO elements
+     * which have their own list of property makers,
+     * see findMaker(propId).
+     * @param propId the ID of the property
+     * @param propertyMaker the maker of the property
+     * @return the cached property value
+     */
+    public Property findProperty (int propId, PropertyMaker propertyMaker) 
+        throws FOPException {
+        Property p;
+        if (isInCache(propId)) {
+            p = getFromCache(propId);
+        } else {
+            p = propertyMaker.findProperty(this, true);
+            addToCache(propId, p);
+        }
+        return p;
+    }
 
+    /**
+     * Add a property value to the cache.
+     * The cached value may be null,
+     * meaning that no property value has been specified by the user
+     * on this FO element or, in the case of inheritable properties,
+     * on an ancester FO element.
+     * @param propId the ID of the property
+     * @param prop the property value being cached
+     */
+    private void addToCache(int propId, Property prop) {
+        String propertyName = FOPropertyMapping.getPropertyName(propId);
+        log.trace("PropertyList.addToCache: "
+                  + propertyName + ", " + getFObj().getName());
+        cache.put(new Integer(propId), prop);
+    }
+
+    /**
+     * Check whether a property is in the cache.
+     * The presence of a key for a property
+     * means that a value for this property has been cached.
+     * @return whether a property is in the cache
+     */
+    public boolean isInCache(int propId) {
+        // Uncomment one or the other to use/not use the cache
+        return cache.containsKey(new Integer(propId));
+        // return false;
+    }
+
+    /**
+     * Retrieve a property from the cache
+     * @param propId the ID of the property
+     * @return the cached property value
+     */
+    public Property getFromCache(int propId) {
+        Property prop;
+        String propertyName = FOPropertyMapping.getPropertyName(propId);
+        prop = (Property) cache.get(new Integer(propId));
+        log.trace("PropertyList.getFromCache: "
+                  + propertyName + ", " + getFObj().getName());
+        return prop;
+    }
 
     /**
      * Return the "nearest" specified value for the given property.
diff --git a/src/java/org/apache/fop/fo/properties/CompoundPropertyMaker.java b/src/java/org/apache/fop/fo/properties/CompoundPropertyMaker.java
index db1f9fd5b..9e4de0684 100644
--- a/src/java/org/apache/fop/fo/properties/CompoundPropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/CompoundPropertyMaker.java
@@ -156,7 +156,7 @@ public class CompoundPropertyMaker extends PropertyMaker {
      * isn't the right type for this compound property.
      * @param p The Property object return by the expression parser
      * @param propertyList The PropertyList object being built for this FO.
-     * @param fo The current FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @return A Property of the correct type or null if the parsed value
      * can't be converted to the correct type.
      * @throws FOPException for invalid or inconsistent FO input
@@ -197,7 +197,7 @@ public class CompoundPropertyMaker extends PropertyMaker {
      * Create a Property object from an attribute specification.
      * @param propertyList The PropertyList object being built for this FO.
      * @param value The attribute value.
-     * @param fo The current FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @return The initialized Property object.
      * @throws FOPException for invalid or inconsistent FO input
      */    
@@ -216,7 +216,7 @@ public class CompoundPropertyMaker extends PropertyMaker {
      * @param subpropId The Constants ID of the subproperty (component)
      *        whose value is specified.
      * @param propertyList The propertyList being built.
-     * @param fo The FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @param value the value of the
      * @return baseProp (or if null, a new compound property object) with
      * the new subproperty added
@@ -260,7 +260,7 @@ public class CompoundPropertyMaker extends PropertyMaker {
         for (int i = 0; i < Constants.COMPOUND_COUNT; i++) {
             PropertyMaker submaker = subproperties[i];
             if (submaker != null) {
-                Property subprop = submaker.make(propertyList, submaker.defaultValue, parentFO);
+                Property subprop = submaker.make(propertyList);
                 data.setComponent(submaker.getPropId() & Constants.COMPOUND_MASK, subprop, true);
             }
         }
diff --git a/src/java/org/apache/fop/fo/properties/PropertyMaker.java b/src/java/org/apache/fop/fo/properties/PropertyMaker.java
index 53b58c12e..5a32d02a6 100644
--- a/src/java/org/apache/fop/fo/properties/PropertyMaker.java
+++ b/src/java/org/apache/fop/fo/properties/PropertyMaker.java
@@ -32,6 +32,9 @@ import org.apache.fop.fo.ShorthandParser;
 import org.apache.fop.fo.expr.PropertyInfo;
 import org.apache.fop.fo.expr.PropertyParser;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+
 
 /**
  * Base class for all property makers
@@ -52,6 +55,8 @@ public class PropertyMaker implements Cloneable {
     protected Property defaultProperty;
     protected CorrespondingPropertyMaker corresponding;
 
+    private Log log = LogFactory.getLog(PropertyMaker.class);
+
     /**
      * @return the name of the property for this Maker
      */
@@ -228,11 +233,15 @@ public class PropertyMaker implements Cloneable {
      * we try to compute it from the corresponding relative property: this
      * happens in computeProperty.
      */
-    protected Property findProperty(PropertyList propertyList, 
+    public Property findProperty(PropertyList propertyList, 
                                  boolean bTryInherit)
         throws FOPException
     {
         Property p = null;
+        
+        log.trace("PropertyMaker.findProperty: "
+                  + FOPropertyMapping.getPropertyName(propId)
+                  + ", " + propertyList.getFObj().getName());
 
         if (corresponding != null && corresponding.isCorrespondingForced(propertyList)) {
             p = corresponding.compute(propertyList);
@@ -248,7 +257,12 @@ public class PropertyMaker implements Cloneable {
                 // else inherit (if has parent and is inheritable)
                 PropertyList parentPropertyList = propertyList.getParentPropertyList(); 
                 if (parentPropertyList != null && isInherited()) {
-                    p = findProperty(parentPropertyList, true);
+                    if (!contextDep) {
+                        // use the cache
+                        p = parentPropertyList.findProperty(propId, this);
+                    } else {
+                        p = findProperty(parentPropertyList, bTryInherit);
+                    }
                 }
             }
         }
@@ -270,7 +284,15 @@ public class PropertyMaker implements Cloneable {
                         boolean bTryInherit, boolean bTryDefault)
         throws FOPException
     {
-        Property p = findProperty(propertyList, bTryInherit);
+        Property p;
+
+        if (!contextDep && bTryInherit) {
+            // use the cache
+            p = propertyList.findProperty(propId, this);
+        } else {
+            p = findProperty(propertyList, bTryInherit);
+        }
+
         if (p == null && bTryDefault) {    // default value for this FO!
             try {
                 p = make(propertyList);
@@ -355,8 +377,13 @@ public class PropertyMaker implements Cloneable {
      */
     public Property make(PropertyList propertyList) throws FOPException {
         if (defaultProperty != null) {
+            log.trace("PropertyMaker.make: reusing defaultProperty, "
+                  + FOPropertyMapping.getPropertyName(propId));
             return defaultProperty;
         }
+        log.trace("PropertyMaker.make: making default property value, "
+                  + FOPropertyMapping.getPropertyName(propId)
+                  + ", " + propertyList.getFObj().getName());
         Property p = make(propertyList, defaultValue, propertyList.getParentFObj());
         if (!contextDep) {
             defaultProperty = p;
@@ -368,7 +395,7 @@ public class PropertyMaker implements Cloneable {
      * Create a Property object from an attribute specification.
      * @param propertyList The PropertyList object being built for this FO.
      * @param value The attribute value.
-     * @param fo The current FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @return The initialized Property object.
      * @throws FOPException for invalid or inconsistent FO input
      */
@@ -411,7 +438,7 @@ public class PropertyMaker implements Cloneable {
      * @param subpropId The Constants ID of the subproperty (component)
      *        whose value is specified.
      * @param propertyList The propertyList being built.
-     * @param fo The FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @param value the value of the
      * @return baseProp (or if null, a new compound property object) with
      * the new subproperty added
@@ -511,7 +538,7 @@ public class PropertyMaker implements Cloneable {
      * It is overridden by subclasses.
      * @param p The Property object return by the expression parser
      * @param propertyList The PropertyList object being built for this FO.
-     * @param fo The current FO whose properties are being set.
+     * @param fo The parent FO for the FO whose property is being made.
      * @return A Property of the correct type or null if the parsed value
      * can't be converted to the correct type.
      * @throws FOPException for invalid or inconsistent FO input
@@ -530,7 +557,7 @@ public class PropertyMaker implements Cloneable {
      * @param p property whose datatype should be converted
      * @param propertyList collection of properties. (TODO: explain why
      * this is needed, or remove it from the signature.)
-     * @param fo the FObj to which this property is attached. (TODO: explain
+     * @param fo The parent FO for the FO whose property is being made.
      * why this is needed, or remove it from the signature).
      * @return an Property with the appropriate datatype used
      */
@@ -603,4 +630,4 @@ public class PropertyMaker implements Cloneable {
             return null;
         }
     }
-}
\ No newline at end of file
+}
diff --git a/src/java/org/apache/fop/layoutmgr/PageLayoutManager.java b/src/java/org/apache/fop/layoutmgr/PageLayoutManager.java
index 08832ae0b..2bd133353 100644
--- a/src/java/org/apache/fop/layoutmgr/PageLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/PageLayoutManager.java
@@ -186,6 +186,7 @@ public class PageLayoutManager extends AbstractLayoutManager implements Runnable
      */
     protected void doLayout() {
 
+        log.debug("Starting layout");
         // this should be done another way
         makeNewPage(false, false);
         createBodyMainReferenceArea();
@@ -206,6 +207,7 @@ public class PageLayoutManager extends AbstractLayoutManager implements Runnable
             }
         }
         pageCount--;
+        log.debug("Ending layout");
     }
 
     /**