From 4e26880397f917dc537993896adff1dcc0e96685 Mon Sep 17 00:00:00 2001 From: Vincent Hennebert Date: Thu, 22 Mar 2012 17:04:12 +0000 Subject: [PATCH] Bugzilla #46962: Fixed deadlock in PropertyCache. Re-wrote the PropertyCache class, leveraging Java 1.5 concurrency features. Implemented equals and hashCode on many Property sub-classes to make the caching more effective Patch by Alexios Giotis, applied with the following minor modifications: * Javadoc improvements to PropertyCache * Factorize into new CompareUtil class code often used by equals and hashCode implementations git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@1303891 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/fop/datatypes/LengthBase.java | 26 + .../apache/fop/fo/expr/NCnameProperty.java | 17 + .../apache/fop/fo/expr/NumericProperty.java | 22 + .../fop/fo/expr/RelativeNumericProperty.java | 30 ++ src/java/org/apache/fop/fo/flow/Marker.java | 11 +- .../BackgroundPositionShorthand.java | 39 +- .../fop/fo/properties/CharacterProperty.java | 19 +- .../fop/fo/properties/ColorProperty.java | 5 +- .../CommonBorderPaddingBackground.java | 128 +++-- .../apache/fop/fo/properties/CommonFont.java | 62 ++- .../fop/fo/properties/CommonHyphenation.java | 5 +- .../fop/fo/properties/CondLengthProperty.java | 6 +- .../apache/fop/fo/properties/EnumLength.java | 16 + .../apache/fop/fo/properties/EnumNumber.java | 20 +- .../fop/fo/properties/EnumProperty.java | 22 +- .../apache/fop/fo/properties/FixedLength.java | 7 +- .../fop/fo/properties/FontFamilyProperty.java | 4 +- .../fop/fo/properties/KeepProperty.java | 6 +- .../fop/fo/properties/LengthPairProperty.java | 21 + .../fop/fo/properties/LengthProperty.java | 4 + .../fo/properties/LengthRangeProperty.java | 28 ++ .../fop/fo/properties/ListProperty.java | 17 + .../fop/fo/properties/NumberProperty.java | 43 +- .../fop/fo/properties/PercentLength.java | 29 +- .../apache/fop/fo/properties/Property.java | 2 +- .../fop/fo/properties/PropertyCache.java | 459 +++++------------- .../fop/fo/properties/StringProperty.java | 7 +- .../fop/fo/properties/TableColLength.java | 22 + .../properties/ToBeImplementedProperty.java | 19 +- .../apache/fop/fo/properties/URIProperty.java | 27 +- src/java/org/apache/fop/util/CompareUtil.java | 75 +++ status.xml | 3 + .../org/apache/fop/StandardTestSuite.java | 4 +- 33 files changed, 645 insertions(+), 560 deletions(-) create mode 100644 src/java/org/apache/fop/util/CompareUtil.java diff --git a/src/java/org/apache/fop/datatypes/LengthBase.java b/src/java/org/apache/fop/datatypes/LengthBase.java index b7e51e191..7bae525cf 100644 --- a/src/java/org/apache/fop/datatypes/LengthBase.java +++ b/src/java/org/apache/fop/datatypes/LengthBase.java @@ -26,6 +26,7 @@ import org.apache.fop.fo.Constants; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Models a length which can be used as a factor in a percentage length @@ -143,5 +144,30 @@ public class LengthBase implements PercentBase { public Length getBaseLength() { return baseLength; } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + CompareUtil.getHashCode(baseLength); + result = prime * result + baseType; + result = prime * result + CompareUtil.getHashCode(fobj); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof LengthBase)) { + return false; + } + LengthBase other = (LengthBase) obj; + return CompareUtil.equal(baseLength, other.baseLength) + && baseType == other.baseType + && CompareUtil.equal(fobj, other.fobj); + } + } diff --git a/src/java/org/apache/fop/fo/expr/NCnameProperty.java b/src/java/org/apache/fop/fo/expr/NCnameProperty.java index 6bc16f2a9..4b2430c51 100644 --- a/src/java/org/apache/fop/fo/expr/NCnameProperty.java +++ b/src/java/org/apache/fop/fo/expr/NCnameProperty.java @@ -24,6 +24,7 @@ import java.awt.Color; import org.apache.fop.apps.FOUserAgent; import org.apache.fop.fo.properties.Property; import org.apache.fop.util.ColorUtil; +import org.apache.fop.util.CompareUtil; /** * Class for handling NC Name objects @@ -79,4 +80,20 @@ public class NCnameProperty extends Property { return this.ncName; } + @Override + public int hashCode() { + return CompareUtil.getHashCode(ncName); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof NCnameProperty)) { + return false; + } + NCnameProperty other = (NCnameProperty) obj; + return CompareUtil.equal(ncName, other.ncName); + } } diff --git a/src/java/org/apache/fop/fo/expr/NumericProperty.java b/src/java/org/apache/fop/fo/expr/NumericProperty.java index 81e4220ae..a3a108191 100644 --- a/src/java/org/apache/fop/fo/expr/NumericProperty.java +++ b/src/java/org/apache/fop/fo/expr/NumericProperty.java @@ -27,6 +27,7 @@ import org.apache.fop.datatypes.Numeric; import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.properties.FixedLength; import org.apache.fop.fo.properties.Property; +import org.apache.fop.util.CompareUtil; /** * A numeric property which hold the final absolute result of an expression @@ -127,4 +128,25 @@ public class NumericProperty extends Property implements Numeric, Length { return value + "^" + dim; } } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + dim; + result = prime * result + CompareUtil.getHashCode(value); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof NumericProperty)) { + return false; + } + NumericProperty other = (NumericProperty) obj; + return dim == other.dim && CompareUtil.equal(value, other.value); + } } diff --git a/src/java/org/apache/fop/fo/expr/RelativeNumericProperty.java b/src/java/org/apache/fop/fo/expr/RelativeNumericProperty.java index a869b75c8..e99c8c7be 100644 --- a/src/java/org/apache/fop/fo/expr/RelativeNumericProperty.java +++ b/src/java/org/apache/fop/fo/expr/RelativeNumericProperty.java @@ -24,6 +24,7 @@ import org.apache.fop.datatypes.Numeric; import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.properties.Property; import org.apache.fop.fo.properties.TableColLength; +import org.apache.fop.util.CompareUtil; /** @@ -173,6 +174,7 @@ public class RelativeNumericProperty extends Property implements Length { * Cast this numeric as a Length. * @return numeric value as length */ + @Override public Length getLength() { if (dimension == 1) { return this; @@ -182,6 +184,7 @@ public class RelativeNumericProperty extends Property implements Length { } /** @return numeric value */ + @Override public Numeric getNumeric() { return this; } @@ -273,6 +276,7 @@ public class RelativeNumericProperty extends Property implements Length { * Return a string represention of the expression. Only used for debugging. * @return the string representation. */ + @Override public String toString() { switch (operation) { case ADDITION: case SUBTRACTION: @@ -290,4 +294,30 @@ public class RelativeNumericProperty extends Property implements Length { return "unknown operation " + operation; } } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + dimension; + result = prime * result + CompareUtil.getHashCode(op1); + result = prime * result + CompareUtil.getHashCode(op2); + result = prime * result + operation; + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof RelativeNumericProperty)) { + return false; + } + RelativeNumericProperty other = (RelativeNumericProperty) obj; + return dimension == other.dimension + && CompareUtil.equal(op1, other.op1) + && CompareUtil.equal(op2, other.op2) + && operation == other.operation; + } } diff --git a/src/java/org/apache/fop/fo/flow/Marker.java b/src/java/org/apache/fop/fo/flow/Marker.java index 4588a9df3..2aae3435f 100644 --- a/src/java/org/apache/fop/fo/flow/Marker.java +++ b/src/java/org/apache/fop/fo/flow/Marker.java @@ -21,9 +21,6 @@ package org.apache.fop.fo.flow; import java.util.Map; -import org.xml.sax.Attributes; -import org.xml.sax.Locator; - import org.apache.fop.apps.FOPException; import org.apache.fop.fo.FONode; import org.apache.fop.fo.FOTreeBuilderContext; @@ -34,6 +31,8 @@ import org.apache.fop.fo.PropertyListMaker; import org.apache.fop.fo.ValidationException; import org.apache.fop.fo.properties.Property; import org.apache.fop.fo.properties.PropertyCache; +import org.xml.sax.Attributes; +import org.xml.sax.Locator; /** * Class modelling the @@ -357,8 +356,8 @@ public class Marker extends FObjMixed { /** Convenience inner class */ public static final class MarkerAttribute { - private static PropertyCache attributeCache - = new PropertyCache(MarkerAttribute.class); + private static final PropertyCache CACHE + = new PropertyCache(); /** namespace */ protected String namespace; @@ -398,7 +397,7 @@ public class Marker extends FObjMixed { private static MarkerAttribute getInstance( String namespace, String qname, String name, String value) { - return attributeCache.fetch( + return CACHE.fetch( new MarkerAttribute(namespace, qname, name, value)); } diff --git a/src/java/org/apache/fop/fo/properties/BackgroundPositionShorthand.java b/src/java/org/apache/fop/fo/properties/BackgroundPositionShorthand.java index 3c0118181..ff0b2fc75 100644 --- a/src/java/org/apache/fop/fo/properties/BackgroundPositionShorthand.java +++ b/src/java/org/apache/fop/fo/properties/BackgroundPositionShorthand.java @@ -72,32 +72,37 @@ public class BackgroundPositionShorthand extends ListProperty { return p; } + private static final class Dimension1PercentBase implements PercentBase { + /** {@inheritDoc} */ + public int getBaseLength(PercentBaseContext context) throws PropertyException { + return 0; + } + + /** {@inheritDoc} */ + public double getBaseValue() { + return 0; + } + + /** {@inheritDoc} */ + public int getDimension() { + return 1; + } + } + + private static final Dimension1PercentBase DIMENSION_1_PERCENT_BASE + = new Dimension1PercentBase(); + /** * {@inheritDoc} * Returns a {@link org.apache.fop.datatypes.PercentBase} whose * getDimension() returns 1. */ public PercentBase getPercentBase(PropertyList pl) { - return new PercentBase() { - /** {@inheritDoc} */ - public int getBaseLength(PercentBaseContext context) throws PropertyException { - return 0; - } - - /** {@inheritDoc} */ - public double getBaseValue() { - return 0; - } - - /** {@inheritDoc} */ - public int getDimension() { - return 1; - } - - }; + return DIMENSION_1_PERCENT_BASE; } } + /** * Inner class to provide shorthand parsing capabilities * diff --git a/src/java/org/apache/fop/fo/properties/CharacterProperty.java b/src/java/org/apache/fop/fo/properties/CharacterProperty.java index ebded4b50..3c2f56d50 100644 --- a/src/java/org/apache/fop/fo/properties/CharacterProperty.java +++ b/src/java/org/apache/fop/fo/properties/CharacterProperty.java @@ -51,8 +51,8 @@ public final class CharacterProperty extends Property { } /** cache containing all canonical CharacterProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(CharacterProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); private final char character; @@ -69,8 +69,7 @@ public final class CharacterProperty extends Property { * @return the character property instance */ public static CharacterProperty getInstance(char character) { - return (CharacterProperty) CACHE.fetch( - new CharacterProperty(character)); + return CACHE.fetch(new CharacterProperty(character)); } /** @@ -94,22 +93,18 @@ public final class CharacterProperty extends Property { return new Character(character).toString(); } - /** - * {@inheritDoc} - */ + @Override public boolean equals(Object obj) { if (obj instanceof CharacterProperty) { - return (((CharacterProperty)obj).character == this.character); + return (character == ((CharacterProperty) obj).character); } else { return false; } } - /** - * {@inheritDoc} - */ + @Override public int hashCode() { - return (int) character; + return character; } } diff --git a/src/java/org/apache/fop/fo/properties/ColorProperty.java b/src/java/org/apache/fop/fo/properties/ColorProperty.java index 0550ce684..f577cbeec 100644 --- a/src/java/org/apache/fop/fo/properties/ColorProperty.java +++ b/src/java/org/apache/fop/fo/properties/ColorProperty.java @@ -33,8 +33,7 @@ import org.apache.fop.util.ColorUtil; public final class ColorProperty extends Property { /** cache holding canonical ColorProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(ColorProperty.class); + private static final PropertyCache CACHE = new PropertyCache(); /** * The color represented by this property. @@ -104,7 +103,7 @@ public final class ColorProperty extends Property { ColorProperty instance = new ColorProperty( ColorUtil.parseColorString( foUserAgent, value)); - return (ColorProperty)CACHE.fetch(instance); + return CACHE.fetch(instance); } /** diff --git a/src/java/org/apache/fop/fo/properties/CommonBorderPaddingBackground.java b/src/java/org/apache/fop/fo/properties/CommonBorderPaddingBackground.java index d39dc24f0..d33b453a3 100644 --- a/src/java/org/apache/fop/fo/properties/CommonBorderPaddingBackground.java +++ b/src/java/org/apache/fop/fo/properties/CommonBorderPaddingBackground.java @@ -22,6 +22,7 @@ package org.apache.fop.fo.properties; import java.awt.Color; import java.io.FileNotFoundException; import java.io.IOException; +import java.util.Arrays; import org.apache.xmlgraphics.image.loader.ImageException; import org.apache.xmlgraphics.image.loader.ImageInfo; @@ -37,19 +38,20 @@ import org.apache.fop.fo.Constants; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Stores all common border and padding properties. * See Sec. 7.7 of the XSL-FO Standard. */ -public class CommonBorderPaddingBackground { // CSOK: FinalClassCheck +public final class CommonBorderPaddingBackground { /** * cache holding all canonical instances * (w/ absolute background-position-* and padding-*) */ - private static final PropertyCache CACHE - = new PropertyCache(CommonBorderPaddingBackground.class); + private static final PropertyCache CACHE + = new PropertyCache(); private int hash = -1; @@ -102,8 +104,8 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl public static final class BorderInfo { /** cache holding all canonical instances */ - private static final PropertyCache CACHE - = new PropertyCache(BorderInfo.class); + private static final PropertyCache CACHE + = new PropertyCache(); private int mStyle; // Enum for border style private Color mColor; // Border color @@ -168,7 +170,7 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl } } - /** {@inheritDoc} */ + @Override public String toString() { StringBuffer sb = new StringBuffer("BorderInfo"); sb.append(" {"); @@ -181,23 +183,21 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl return sb.toString(); } - /** {@inheritDoc} */ + @Override public boolean equals(Object obj) { if (this == obj) { return true; } - - if (obj instanceof BorderInfo) { - BorderInfo bi = (BorderInfo)obj; - return (this.mColor == bi.mColor - && this.mStyle == bi.mStyle - && this.mWidth == bi.mWidth); + if (!(obj instanceof BorderInfo)) { + return false; } - - return false; + BorderInfo other = (BorderInfo) obj; + return CompareUtil.equal(mColor, other.mColor) + && mStyle == other.mStyle + && CompareUtil.equal(mWidth, other.mWidth); } - /** {@inheritDoc} */ + @Override public int hashCode() { if (this.hash == -1) { int hash = 17; @@ -224,47 +224,47 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl */ private static class ConditionalNullLength extends CondLengthProperty { - /** {@inheritDoc} */ + @Override public Property getComponent(int cmpId) { throw new UnsupportedOperationException(); } - /** {@inheritDoc} */ + @Override public Property getConditionality() { throw new UnsupportedOperationException(); } - /** {@inheritDoc} */ + @Override public Length getLength() { throw new UnsupportedOperationException(); } - /** {@inheritDoc} */ + @Override public Property getLengthComponent() { throw new UnsupportedOperationException(); } - /** {@inheritDoc} */ + @Override public int getLengthValue() { return 0; } - /** {@inheritDoc} */ + @Override public int getLengthValue(PercentBaseContext context) { return 0; } - /** {@inheritDoc} */ + @Override public boolean isDiscard() { return true; } - /** {@inheritDoc} */ + @Override public void setComponent(int cmpId, Property cmpnValue, boolean isDefault) { throw new UnsupportedOperationException(); } - /** {@inheritDoc} */ + @Override public String toString() { return "CondLength[0mpt, discard]"; } @@ -523,9 +523,9 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl } /** - * @param side the side to retrieve - * @param discard indicates whether the .conditionality component should be - * considered (end of a reference-area) + * @param side the side of the border + * @param discard indicates whether the .conditionality component should be considered (end of a + * reference-area) * @return the width of the start-border, taking into account the specified conditionality */ public int getBorderWidth(int side, boolean discard) { @@ -620,7 +620,7 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl + getBorderBeforeWidth(discard) + getBorderAfterWidth(discard); } - /** {@inheritDoc} */ + @Override public String toString() { return "CommonBordersAndPadding (Before, After, Start, End):\n" + "Borders: (" + getBorderBeforeWidth(false) + ", " + getBorderAfterWidth(false) + ", " @@ -731,57 +731,53 @@ public class CommonBorderPaddingBackground { // CSOK: FinalCl return padding; } - /** {@inheritDoc} */ + @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj instanceof CommonBorderPaddingBackground) { - CommonBorderPaddingBackground cbpb = (CommonBorderPaddingBackground)obj; - return (this.backgroundAttachment == cbpb.backgroundAttachment - && this.backgroundColor == cbpb.backgroundColor - && this.backgroundImage.equals(cbpb.backgroundImage) - && this.backgroundPositionHorizontal == cbpb.backgroundPositionHorizontal - && this.backgroundPositionVertical == cbpb.backgroundPositionVertical - && this.backgroundRepeat == cbpb.backgroundRepeat - && this.borderInfo[BEFORE] == cbpb.borderInfo[BEFORE] - && this.borderInfo[AFTER] == cbpb.borderInfo[AFTER] - && this.borderInfo[START] == cbpb.borderInfo[START] - && this.borderInfo[END] == cbpb.borderInfo[END] - && this.padding[BEFORE] == cbpb.padding[BEFORE] - && this.padding[AFTER] == cbpb.padding[AFTER] - && this.padding[START] == cbpb.padding[START] - && this.padding[END] == cbpb.padding[END]); + if (!(obj instanceof CommonBorderPaddingBackground)) { + return false; } - return false; + CommonBorderPaddingBackground other = (CommonBorderPaddingBackground) obj; + return backgroundAttachment == other.backgroundAttachment + && CompareUtil.equal(backgroundColor, other.backgroundColor) + && CompareUtil.equal(backgroundImage, other.backgroundImage) + && CompareUtil.equal(backgroundPositionHorizontal, backgroundPositionHorizontal) + && CompareUtil.equal(backgroundPositionVertical, other.backgroundPositionVertical) + && backgroundRepeat == other.backgroundRepeat + && Arrays.equals(borderInfo, other.borderInfo) + && Arrays.equals(padding, other.padding); } - /** {@inheritDoc} */ + @Override public int hashCode() { if (this.hash == -1) { - int hash = 17; + int hash = getHashCode(backgroundColor, + backgroundImage, + backgroundPositionHorizontal, + backgroundPositionVertical, + borderInfo[BEFORE], + borderInfo[AFTER], + borderInfo[START], + borderInfo[END], + padding[BEFORE], + padding[AFTER], + padding[START], + padding[END]); hash = 37 * hash + backgroundAttachment; - hash = 37 * hash + (backgroundColor == null ? 0 : backgroundColor.hashCode()); - hash = 37 * hash + (backgroundImage == null ? 0 : backgroundImage.hashCode()); - hash = 37 * hash - + (backgroundPositionHorizontal == null - ? 0 : backgroundPositionHorizontal.hashCode()); - hash = 37 * hash - + (backgroundPositionVertical == null - ? 0 : backgroundPositionVertical.hashCode()); hash = 37 * hash + backgroundRepeat; - hash = 37 * hash + (borderInfo[BEFORE] == null ? 0 : borderInfo[BEFORE].hashCode()); - hash = 37 * hash + (borderInfo[AFTER] == null ? 0 : borderInfo[AFTER].hashCode()); - hash = 37 * hash + (borderInfo[START] == null ? 0 : borderInfo[START].hashCode()); - hash = 37 * hash + (borderInfo[END] == null ? 0 : borderInfo[END].hashCode()); - hash = 37 * hash + (padding[BEFORE] == null ? 0 : padding[BEFORE].hashCode()); - hash = 37 * hash + (padding[AFTER] == null ? 0 : padding[AFTER].hashCode()); - hash = 37 * hash + (padding[START] == null ? 0 : padding[START].hashCode()); - hash = 37 * hash + (padding[END] == null ? 0 : padding[END].hashCode()); this.hash = hash; } - return this.hash; } + + private int getHashCode(Object... objects) { + int hash = 17; + for (Object o : objects) { + hash = 37 * hash + (o == null ? 0 : o.hashCode()); + } + return hash; + } } diff --git a/src/java/org/apache/fop/fo/properties/CommonFont.java b/src/java/org/apache/fop/fo/properties/CommonFont.java index b12f84ad2..7ec1f219f 100644 --- a/src/java/org/apache/fop/fo/properties/CommonFont.java +++ b/src/java/org/apache/fop/fo/properties/CommonFont.java @@ -29,6 +29,7 @@ import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; import org.apache.fop.fonts.FontInfo; import org.apache.fop.fonts.FontTriplet; +import org.apache.fop.util.CompareUtil; /** * Collection of CommonFont properties @@ -37,11 +38,10 @@ public final class CommonFont { /** cache holding canonical CommonFont instances (only those with * absolute font-size and font-size-adjust) */ - private static final PropertyCache CACHE - = new PropertyCache(CommonFont.class); + private static final PropertyCache CACHE = new PropertyCache(); /** hashcode of this instance */ - private int hash = 0; + private int hash = -1; /** The "font-family" property. */ private final FontFamilyProperty fontFamily; @@ -134,10 +134,10 @@ public final class CommonFont { /** @return an array with the font-family names */ private String[] getFontFamily() { - List lst = fontFamily.getList(); + List lst = fontFamily.getList(); String[] fontFamily = new String[lst.size()]; for (int i = 0, c = lst.size(); i < c; i++) { - fontFamily[i] = ((Property)lst.get(i)).getString(); + fontFamily[i] = lst.get(i).getString(); } return fontFamily; } @@ -227,48 +227,40 @@ public final class CommonFont { } /** {@inheritDoc} */ - public boolean equals(Object o) { - - if (o == null) { - return false; - } - - if (this == o) { + @Override + public boolean equals(Object obj) { + if (this == obj) { return true; } - - if (o instanceof CommonFont) { - CommonFont cf = (CommonFont) o; - return (cf.fontFamily == this.fontFamily) - && (cf.fontSelectionStrategy == this.fontSelectionStrategy) - && (cf.fontStretch == this.fontStretch) - && (cf.fontStyle == this.fontStyle) - && (cf.fontVariant == this.fontVariant) - && (cf.fontWeight == this.fontWeight) - && (cf.fontSize == this.fontSize) - && (cf.fontSizeAdjust == this.fontSizeAdjust); + if (!(obj instanceof CommonFont)) { + return false; } - return false; + CommonFont other = (CommonFont) obj; + return CompareUtil.equal(fontFamily, other.fontFamily) + && CompareUtil.equal(fontSelectionStrategy, other.fontSelectionStrategy) + && CompareUtil.equal(fontSize, other.fontSize) + && CompareUtil.equal(fontSizeAdjust, other.fontSizeAdjust) + && CompareUtil.equal(fontStretch, other.fontStretch) + && CompareUtil.equal(fontStyle, other.fontStyle) + && CompareUtil.equal(fontVariant, other.fontVariant) + && CompareUtil.equal(fontWeight, other.fontWeight); } /** {@inheritDoc} */ public int hashCode() { - if (this.hash == -1) { int hash = 17; - hash = 37 * hash + (fontSize == null ? 0 : fontSize.hashCode()); - hash = 37 * hash + (fontSizeAdjust == null ? 0 : fontSizeAdjust.hashCode()); - hash = 37 * hash + (fontFamily == null ? 0 : fontFamily.hashCode()); - hash = 37 * hash + (fontSelectionStrategy == null - ? 0 : fontSelectionStrategy.hashCode()); - hash = 37 * hash + (fontStretch == null ? 0 : fontStretch.hashCode()); - hash = 37 * hash + (fontStyle == null ? 0 : fontStyle.hashCode()); - hash = 37 * hash + (fontVariant == null ? 0 : fontVariant.hashCode()); - hash = 37 * hash + (fontStretch == null ? 0 : fontStretch.hashCode()); + hash = 37 * hash + CompareUtil.getHashCode(fontSize); + hash = 37 * hash + CompareUtil.getHashCode(fontSizeAdjust); + hash = 37 * hash + CompareUtil.getHashCode(fontFamily); + hash = 37 * hash + CompareUtil.getHashCode(fontSelectionStrategy); + hash = 37 * hash + CompareUtil.getHashCode(fontStretch); + hash = 37 * hash + CompareUtil.getHashCode(fontStyle); + hash = 37 * hash + CompareUtil.getHashCode(fontVariant); + hash = 37 * hash + CompareUtil.getHashCode(fontWeight); this.hash = hash; } return hash; - } } diff --git a/src/java/org/apache/fop/fo/properties/CommonHyphenation.java b/src/java/org/apache/fop/fo/properties/CommonHyphenation.java index 7b9b5bc82..621e2a7c0 100644 --- a/src/java/org/apache/fop/fo/properties/CommonHyphenation.java +++ b/src/java/org/apache/fop/fo/properties/CommonHyphenation.java @@ -21,7 +21,6 @@ package org.apache.fop.fo.properties; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - import org.apache.fop.fo.Constants; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; @@ -38,7 +37,8 @@ public final class CommonHyphenation { /** Logger */ private static final Log LOG = LogFactory.getLog(CommonHyphenation.class); - private static final PropertyCache CACHE = new PropertyCache(CommonHyphenation.class); + private static final PropertyCache CACHE = + new PropertyCache(); private int hash = 0; @@ -118,7 +118,6 @@ public final class CommonHyphenation { hyphenationRemainCharacterCount); return CACHE.fetch(instance); - } private static final char HYPHEN_MINUS = '-'; diff --git a/src/java/org/apache/fop/fo/properties/CondLengthProperty.java b/src/java/org/apache/fop/fo/properties/CondLengthProperty.java index 1ab7ec3ad..07eec7361 100644 --- a/src/java/org/apache/fop/fo/properties/CondLengthProperty.java +++ b/src/java/org/apache/fop/fo/properties/CondLengthProperty.java @@ -33,8 +33,8 @@ import org.apache.fop.fo.expr.PropertyException; public class CondLengthProperty extends Property implements CompoundDatatype { /** cache holding canonical instances (for absolute conditional lengths) */ - private static final PropertyCache CACHE - = new PropertyCache(CondLengthProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); /** components */ private Property length; @@ -159,7 +159,7 @@ public class CondLengthProperty extends Property implements CompoundDatatype { */ public CondLengthProperty getCondLength() { if (this.length.getLength().isAbsolute()) { - CondLengthProperty clp = (CondLengthProperty) CACHE.fetch(this); + CondLengthProperty clp = CACHE.fetch(this); if (clp == this) { isCached = true; } diff --git a/src/java/org/apache/fop/fo/properties/EnumLength.java b/src/java/org/apache/fop/fo/properties/EnumLength.java index d2480beb2..9fd75259a 100644 --- a/src/java/org/apache/fop/fo/properties/EnumLength.java +++ b/src/java/org/apache/fop/fo/properties/EnumLength.java @@ -20,6 +20,7 @@ package org.apache.fop.fo.properties; import org.apache.fop.datatypes.PercentBaseContext; +import org.apache.fop.util.CompareUtil; /** * A length quantity in XSL which is specified as an enum, such as "auto" @@ -93,5 +94,20 @@ public class EnumLength extends LengthProperty { return enumProperty.getObject(); } + @Override + public int hashCode() { + return enumProperty.hashCode(); + } + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof EnumLength)) { + return false; + } + EnumLength other = (EnumLength) obj; + return CompareUtil.equal(enumProperty, other.enumProperty); + } } diff --git a/src/java/org/apache/fop/fo/properties/EnumNumber.java b/src/java/org/apache/fop/fo/properties/EnumNumber.java index 153a716df..9c018e0eb 100644 --- a/src/java/org/apache/fop/fo/properties/EnumNumber.java +++ b/src/java/org/apache/fop/fo/properties/EnumNumber.java @@ -22,6 +22,7 @@ package org.apache.fop.fo.properties; import org.apache.fop.datatypes.Numeric; import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * A number quantity in XSL which is specified as an enum, such as "no-limit". @@ -29,8 +30,8 @@ import org.apache.fop.fo.expr.PropertyException; public final class EnumNumber extends Property implements Numeric { /** cache holding all canonical EnumNumber instances */ - private static final PropertyCache CACHE - = new PropertyCache(EnumNumber.class); + private static final PropertyCache CACHE + = new PropertyCache(); private final EnumProperty enumProperty; @@ -50,8 +51,7 @@ public final class EnumNumber extends Property implements Numeric { * @return the canonical instance */ public static EnumNumber getInstance(Property enumProperty) { - return (EnumNumber)CACHE.fetch( - new EnumNumber((EnumProperty) enumProperty)); + return CACHE.fetch(new EnumNumber((EnumProperty) enumProperty)); } /** {@inheritDoc} */ @@ -69,16 +69,20 @@ public final class EnumNumber extends Property implements Numeric { return enumProperty.getObject(); } - /** {@inheritDoc} */ + @Override public boolean equals(Object obj) { - if (obj instanceof EnumNumber) { - return (((EnumNumber)obj).enumProperty == this.enumProperty); - } else { + if (this == obj) { + return true; + } + if (!(obj instanceof EnumNumber)) { return false; } + EnumNumber other = (EnumNumber) obj; + return CompareUtil.equal(enumProperty, other.enumProperty); } /** {@inheritDoc} */ + @Override public int hashCode() { return enumProperty.hashCode(); } diff --git a/src/java/org/apache/fop/fo/properties/EnumProperty.java b/src/java/org/apache/fop/fo/properties/EnumProperty.java index ae704f1c9..f429b95ac 100644 --- a/src/java/org/apache/fop/fo/properties/EnumProperty.java +++ b/src/java/org/apache/fop/fo/properties/EnumProperty.java @@ -22,6 +22,7 @@ package org.apache.fop.fo.properties; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Superclass for properties that wrap an enumeration value @@ -29,8 +30,8 @@ import org.apache.fop.fo.expr.PropertyException; public final class EnumProperty extends Property { /** cache holding all canonical EnumProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(EnumProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); /** * Inner class for creating EnumProperty instances @@ -93,8 +94,7 @@ public final class EnumProperty extends Property { * @return an enumeration property */ public static EnumProperty getInstance(int explicitValue, String text) { - return (EnumProperty) CACHE.fetch( - new EnumProperty(explicitValue, text)); + return CACHE.fetch(new EnumProperty(explicitValue, text)); } /** @@ -111,24 +111,18 @@ public final class EnumProperty extends Property { return text; } - /** - * {@inheritDoc} - */ + @Override public boolean equals(Object obj) { if (obj instanceof EnumProperty) { EnumProperty ep = (EnumProperty)obj; - return (ep.value == this.value) - && ((ep.text == this.text) - || (ep.text != null - && ep.text.equals(this.text))); + return this.value == ep.value + && CompareUtil.equal(text, ep.text); } else { return false; } } - /** - * {@inheritDoc} - */ + @Override public int hashCode() { return value + text.hashCode(); } diff --git a/src/java/org/apache/fop/fo/properties/FixedLength.java b/src/java/org/apache/fop/fo/properties/FixedLength.java index c35c6f6c9..0cf27e390 100644 --- a/src/java/org/apache/fop/fo/properties/FixedLength.java +++ b/src/java/org/apache/fop/fo/properties/FixedLength.java @@ -45,8 +45,7 @@ public final class FixedLength extends LengthProperty { public static final String MPT = "mpt"; /** cache holding all canonical FixedLength instances */ - private static final PropertyCache CACHE - = new PropertyCache(FixedLength.class); + private static final PropertyCache CACHE = new PropertyCache(); /** canonical zero-length instance */ public static final FixedLength ZERO_FIXED_LENGTH = new FixedLength(0, FixedLength.MPT, 1.0f); @@ -82,10 +81,8 @@ public final class FixedLength extends LengthProperty { if (numUnits == 0.0) { return ZERO_FIXED_LENGTH; } else { - return (FixedLength)CACHE.fetch( - new FixedLength(numUnits, units, sourceResolution)); + return CACHE.fetch(new FixedLength(numUnits, units, sourceResolution)); } - } /** diff --git a/src/java/org/apache/fop/fo/properties/FontFamilyProperty.java b/src/java/org/apache/fop/fo/properties/FontFamilyProperty.java index f6fa806c9..ff7619928 100644 --- a/src/java/org/apache/fop/fo/properties/FontFamilyProperty.java +++ b/src/java/org/apache/fop/fo/properties/FontFamilyProperty.java @@ -31,8 +31,8 @@ import org.apache.fop.fo.expr.PropertyException; public final class FontFamilyProperty extends ListProperty { /** cache holding all canonical FontFamilyProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(FontFamilyProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); private int hash = 0; diff --git a/src/java/org/apache/fop/fo/properties/KeepProperty.java b/src/java/org/apache/fop/fo/properties/KeepProperty.java index 9d04ce780..32e94f47a 100644 --- a/src/java/org/apache/fop/fo/properties/KeepProperty.java +++ b/src/java/org/apache/fop/fo/properties/KeepProperty.java @@ -30,8 +30,8 @@ import org.apache.fop.fo.expr.PropertyException; public final class KeepProperty extends Property implements CompoundDatatype { /** class holding all canonical KeepProperty instances*/ - private static final PropertyCache CACHE - = new PropertyCache(KeepProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); private boolean isCachedValue = false; private Property withinLine; @@ -165,7 +165,7 @@ public final class KeepProperty extends Property implements CompoundDatatype { * this property */ public KeepProperty getKeep() { - KeepProperty keep = (KeepProperty) CACHE.fetch(this); + KeepProperty keep = CACHE.fetch(this); /* make sure setComponent() can never alter cached values */ keep.isCachedValue = true; return keep; diff --git a/src/java/org/apache/fop/fo/properties/LengthPairProperty.java b/src/java/org/apache/fop/fo/properties/LengthPairProperty.java index 9840c4683..00898fb88 100644 --- a/src/java/org/apache/fop/fo/properties/LengthPairProperty.java +++ b/src/java/org/apache/fop/fo/properties/LengthPairProperty.java @@ -23,6 +23,7 @@ import org.apache.fop.datatypes.CompoundDatatype; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Superclass for properties wrapping a LengthPair value @@ -150,4 +151,24 @@ public class LengthPairProperty extends Property implements CompoundDatatype { return this; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + CompareUtil.getHashCode(bpd); + result = prime * result + CompareUtil.getHashCode(ipd); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof LengthPairProperty)) { + return false; + } + LengthPairProperty other = (LengthPairProperty) obj; + return CompareUtil.equal(bpd, other.bpd) && CompareUtil.equal(ipd, other.ipd); + } } diff --git a/src/java/org/apache/fop/fo/properties/LengthProperty.java b/src/java/org/apache/fop/fo/properties/LengthProperty.java index 3f569054e..402d2a623 100644 --- a/src/java/org/apache/fop/fo/properties/LengthProperty.java +++ b/src/java/org/apache/fop/fo/properties/LengthProperty.java @@ -48,6 +48,7 @@ public abstract class LengthProperty extends Property } /** {@inheritDoc} */ + @Override public Property convertProperty(Property p, PropertyList propertyList, FObj fo) throws PropertyException { @@ -80,16 +81,19 @@ public abstract class LengthProperty extends Property } /** @return this.length cast as a Numeric */ + @Override public Numeric getNumeric() { return this; } /** @return this.length */ + @Override public Length getLength() { return this; } /** @return this.length cast as an Object */ + @Override public Object getObject() { return this; } diff --git a/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java b/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java index d8e8ee272..581ac2d84 100644 --- a/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java +++ b/src/java/org/apache/fop/fo/properties/LengthRangeProperty.java @@ -26,6 +26,7 @@ import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; import org.apache.fop.traits.MinOptMax; +import org.apache.fop.util.CompareUtil; /** * Superclass for properties that contain LengthRange values @@ -314,4 +315,31 @@ public class LengthRangeProperty extends Property implements CompoundDatatype { return this; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + bfSet; + result = prime * result + (consistent ? 1231 : 1237); + result = prime * result + CompareUtil.getHashCode(minimum); + result = prime * result + CompareUtil.getHashCode(optimum); + result = prime * result + CompareUtil.getHashCode(maximum); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof LengthRangeProperty)) { + return false; + } + LengthRangeProperty other = (LengthRangeProperty) obj; + return bfSet == other.bfSet + && consistent == other.consistent + && CompareUtil.equal(minimum, other.minimum) + && CompareUtil.equal(optimum, other.optimum) + && CompareUtil.equal(maximum, other.maximum); + } } diff --git a/src/java/org/apache/fop/fo/properties/ListProperty.java b/src/java/org/apache/fop/fo/properties/ListProperty.java index 6b1ffcea0..c0e1d7436 100644 --- a/src/java/org/apache/fop/fo/properties/ListProperty.java +++ b/src/java/org/apache/fop/fo/properties/ListProperty.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Superclass for properties that are lists of other properties @@ -105,4 +106,20 @@ public class ListProperty extends Property { return list; } + @Override + public int hashCode() { + return list.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof ListProperty)) { + return false; + } + ListProperty other = (ListProperty) obj; + return CompareUtil.equal(list, other.list); + } } diff --git a/src/java/org/apache/fop/fo/properties/NumberProperty.java b/src/java/org/apache/fop/fo/properties/NumberProperty.java index 97844d723..de31e03e7 100644 --- a/src/java/org/apache/fop/fo/properties/NumberProperty.java +++ b/src/java/org/apache/fop/fo/properties/NumberProperty.java @@ -28,6 +28,7 @@ import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * Class for handling numeric properties @@ -106,8 +107,8 @@ public final class NumberProperty extends Property implements Numeric { } /** cache holding all canonical NumberProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(NumberProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); private final Number number; @@ -144,8 +145,7 @@ public final class NumberProperty extends Property implements Numeric { * @return the canonical NumberProperty */ public static NumberProperty getInstance(Double num) { - return (NumberProperty)CACHE.fetch( - new NumberProperty(num.doubleValue())); + return CACHE.fetch(new NumberProperty(num.doubleValue())); } /** @@ -155,8 +155,7 @@ public final class NumberProperty extends Property implements Numeric { * @return the canonical NumberProperty */ public static NumberProperty getInstance(Integer num) { - return (NumberProperty)CACHE.fetch( - new NumberProperty(num.intValue())); + return CACHE.fetch(new NumberProperty(num.intValue())); } /** @@ -166,8 +165,7 @@ public final class NumberProperty extends Property implements Numeric { * @return the canonical NumberProperty */ public static NumberProperty getInstance(double num) { - return (NumberProperty)CACHE.fetch( - new NumberProperty(num)); + return CACHE.fetch(new NumberProperty(num)); } /** @@ -177,8 +175,7 @@ public final class NumberProperty extends Property implements Numeric { * @return the canonical NumberProperty */ public static NumberProperty getInstance(int num) { - return (NumberProperty)CACHE.fetch( - new NumberProperty(num)); + return CACHE.fetch(new NumberProperty(num)); } /** @@ -269,21 +266,21 @@ public final class NumberProperty extends Property implements Numeric { } /** {@inheritDoc} */ - public int hashCode() { - return number.hashCode(); - } - - /** {@inheritDoc} */ - public boolean equals(Object o) { - if (o == this) { + @Override + public boolean equals(Object obj) { + if (this == obj) { return true; } - if (o instanceof NumberProperty) { - NumberProperty np = (NumberProperty) o; - return (np.number == this.number - || (this.number != null - && this.number.equals(np.number))); + if (!(obj instanceof NumberProperty)) { + return false; } - return false; + NumberProperty other = (NumberProperty) obj; + return CompareUtil.equal(number, other.number); + } + + /** {@inheritDoc} */ + @Override + public int hashCode() { + return number.hashCode(); } } diff --git a/src/java/org/apache/fop/fo/properties/PercentLength.java b/src/java/org/apache/fop/fo/properties/PercentLength.java index 4e83fb919..e5e1cb0eb 100644 --- a/src/java/org/apache/fop/fo/properties/PercentLength.java +++ b/src/java/org/apache/fop/fo/properties/PercentLength.java @@ -22,6 +22,7 @@ package org.apache.fop.fo.properties; import org.apache.fop.datatypes.PercentBase; import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.expr.PropertyException; +import org.apache.fop.util.CompareUtil; /** * a percent specified length quantity in XSL @@ -38,9 +39,7 @@ public class PercentLength extends LengthProperty { * A PercentBase implementation that contains the base length to which the * {@link #factor} should be applied to compute the actual length */ - private PercentBase lbase = null; - - private double resolvedValue; + private PercentBase lbase; /** * Main constructor. Construct an object based on a factor (the percent, @@ -88,8 +87,7 @@ public class PercentLength extends LengthProperty { /** {@inheritDoc} */ public double getNumericValue(PercentBaseContext context) { try { - resolvedValue = factor * lbase.getBaseLength(context); - return resolvedValue; + return factor * lbase.getBaseLength(context); } catch (PropertyException exc) { log.error(exc); return 0; @@ -124,4 +122,25 @@ public class PercentLength extends LengthProperty { return sb.toString(); } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + CompareUtil.getHashCode(factor); + result = prime * result + CompareUtil.getHashCode(lbase); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof PercentLength)) { + return false; + } + PercentLength other = (PercentLength) obj; + return CompareUtil.equal(factor, other.factor) + && CompareUtil.equal(lbase, other.lbase); + } } diff --git a/src/java/org/apache/fop/fo/properties/Property.java b/src/java/org/apache/fop/fo/properties/Property.java index e5ee87bd6..a396d4bba 100644 --- a/src/java/org/apache/fop/fo/properties/Property.java +++ b/src/java/org/apache/fop/fo/properties/Property.java @@ -33,7 +33,7 @@ import org.apache.fop.fo.Constants; /** * Base class for all property objects */ -public class Property { +public abstract class Property { /** Logger for all property classes */ protected static final Log log = LogFactory.getLog(PropertyMaker.class); diff --git a/src/java/org/apache/fop/fo/properties/PropertyCache.java b/src/java/org/apache/fop/fo/properties/PropertyCache.java index cb11cf6fa..d08164b6c 100644 --- a/src/java/org/apache/fop/fo/properties/PropertyCache.java +++ b/src/java/org/apache/fop/fo/properties/PropertyCache.java @@ -19,298 +19,87 @@ package org.apache.fop.fo.properties; -import java.lang.ref.ReferenceQueue; import java.lang.ref.WeakReference; +import java.util.Iterator; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; -import org.apache.fop.fo.flow.Marker; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; /** - * Dedicated cache, meant for storing canonical instances - * of property-related classes. - * The public access points are overloaded fetch() methods - * that each correspond to a cached type. - * It is designed especially to be used concurrently by multiple threads, - * drawing heavily upon the principles behind Java 1.5's - * ConcurrentHashMap. + * Thread-safe cache that minimizes the memory requirements by fetching an instance from the cache + * that is equal to the given one. Internally the instances are stored in WeakReferences in order to + * be reclaimed when they are no longer referenced. + * @param The type of values that are cached */ -public final class PropertyCache { +public final class PropertyCache { - private static final int SEGMENT_COUNT = 32; //0x20 - private static final int INITIAL_BUCKET_COUNT = SEGMENT_COUNT; + private static final Log LOG = LogFactory.getLog(PropertyCache.class); - /** bitmask to apply to the hash to get to the - * corresponding cache segment */ - private static final int SEGMENT_MASK = SEGMENT_COUNT - 1; //0x1F /** - * Indicates whether the cache should be used at all - * Can be controlled by the system property: - * org.apache.fop.fo.properties.use-cache + * Determines if the cache is used based on the value of the system property + * org.apache.fop.fo.properties.use-cache */ private final boolean useCache; - /** the segments array (length = 32) */ - private CacheSegment[] segments = new CacheSegment[SEGMENT_COUNT]; - /** the table of hash-buckets */ - private CacheEntry[] table = new CacheEntry[INITIAL_BUCKET_COUNT]; - - private Class runtimeType; - - private final boolean[] votesForRehash = new boolean[SEGMENT_COUNT]; - - /* same hash function as used by java.util.HashMap */ - private static int hash(Object x) { - return hash(x.hashCode()); - } - - private static int hash(int hashCode) { - int h = hashCode; - h += ~(h << 9); - h ^= (h >>> 14); - h += (h << 4); - h ^= (h >>> 10); - return h; - } - - /* shortcut function */ - private static boolean eq(Object p, Object q) { - return (p == q || (p != null && p.equals(q))); - } - - /* Class modeling a cached entry */ - private static class CacheEntry extends WeakReference { - private volatile CacheEntry nextEntry; - private final int hash; - - /* main constructor */ - public CacheEntry(Object p, CacheEntry nextEntry, ReferenceQueue refQueue) { - super(p, refQueue); - this.nextEntry = nextEntry; - this.hash = hash(p); - } - - /* main constructor */ - public CacheEntry(Object p, CacheEntry nextEntry) { - super(p); - this.nextEntry = nextEntry; - this.hash = hash(p); - } - - } - - /* Wrapper objects to synchronize on */ - private static final class CacheSegment { - CacheSegment() { - } - private int count = 0; - int getCount() { - return count; - } - } - - private void cleanSegment(int segmentIndex) { - CacheSegment segment = segments[segmentIndex]; - - int oldCount = segment.count; - - /* clean all buckets in this segment */ - for (int bucketIndex = segmentIndex; - bucketIndex < table.length; - bucketIndex += SEGMENT_COUNT) { - CacheEntry prev = null; - CacheEntry entry = table[bucketIndex]; - if (entry == null) { - continue; - } - do { - if (entry.get() == null) { - if (prev == null) { - table[bucketIndex] = entry.nextEntry; - } else { - prev.nextEntry = entry.nextEntry; - } - segment.count--; - assert segment.count >= 0; - } else { - prev = entry; - } - entry = entry.nextEntry; - } while (entry != null); - } - - synchronized (votesForRehash) { - if (oldCount > segment.count) { - votesForRehash[segmentIndex] = false; - return; - } - /* cleanup had no effect */ - if (!votesForRehash[segmentIndex]) { - /* first time for this segment */ - votesForRehash[segmentIndex] = true; - int voteCount = 0; - for (int i = SEGMENT_MASK + 1; --i >= 0;) { - if (votesForRehash[i]) { - voteCount++; - } - } - if (voteCount > SEGMENT_MASK / 4) { - rehash(SEGMENT_MASK); - /* reset votes */ - for (int i = SEGMENT_MASK + 1; --i >= 0;) { - votesForRehash[i] = false; - } - - } - } - } - } - - /* - * Puts a new instance in the cache. - * If the total number of entries for the corresponding - * segment exceeds twice the amount of hash-buckets, a - * cleanup will be performed to try and remove obsolete - * entries. + /** + * The underlying map that stores WeakReferences to the cached entries. The map keys are the + * hashCode of the cached entries. The map values are a WeakRefence to the cached entries. When + * two cached entries have the same hash code, the last one is kept but this should be an + * exception case (otherwise the hashCode() method of T needs to be fixed). */ - private void put(Object o) { - - int hash = hash(o); - int segmentIndex = hash & SEGMENT_MASK; - CacheSegment segment = segments[segmentIndex]; - - synchronized (segment) { - int index = hash & (table.length - 1); - CacheEntry entry = table[index]; - - if (entry == null) { - entry = new CacheEntry(o, null); - table[index] = entry; - segment.count++; - } else { - Object p = entry.get(); - if (eq(p, o)) { - return; - } else { - CacheEntry newEntry = new CacheEntry(o, entry); - table[index] = newEntry; - segment.count++; - } - } - - if (segment.count > (2 * table.length)) { - cleanSegment(segmentIndex); - } - } - } - - - /* Gets a cached instance. Returns null if not found */ - private Object get(Object o) { - - int hash = hash(o); - int index = hash & (table.length - 1); - - CacheEntry entry = table[index]; - Object q; - - /* try non-synched first */ - for (CacheEntry e = entry; e != null; e = e.nextEntry) { - if ( e.hash == hash ) { - q = e.get(); - if ( ( q != null ) && eq ( q, o ) ) { - return q; - } - } - } - - /* retry synched, only if the above attempt did not succeed, - * as another thread may, in the meantime, have added a - * corresponding entry */ - CacheSegment segment = segments[hash & SEGMENT_MASK]; - synchronized (segment) { - entry = table[index]; - for (CacheEntry e = entry; e != null; e = e.nextEntry) { - if ( e.hash == hash ) { - q = e.get(); - if ( ( q != null ) && eq ( q, o ) ) { - return q; - } - } - } - } - return null; - } + private final ConcurrentMap> map; - /* - * Recursively acquires locks on all 32 segments, - * extends the cache and redistributes the entries. - * + /** + * Counts the number of entries put in the map in order to periodically check and remove the + * entries whose referents have been reclaimed. */ - private void rehash(int index) { - - CacheSegment seg = segments[index]; - synchronized (seg) { - if (index > 0) { - /* need to recursively acquire locks on all segments */ - rehash(index - 1); - } else { - /* double the amount of buckets */ - int newLength = table.length << 1; - if (newLength > 0) { //no overflow? - /* reset segment counts */ - for (int i = segments.length; --i >= 0;) { - segments[i].count = 0; - } + private final AtomicInteger putCounter; - CacheEntry[] newTable = new CacheEntry[newLength]; + /** + * Lock to prevent concurrent cleanup of the map. + */ + private final Lock cleanupLock; - int hash; - int idx; - Object o; - newLength--; - for (int i = table.length; --i >= 0;) { - for (CacheEntry c = table[i]; c != null; c = c.nextEntry) { - o = c.get(); - if (o != null) { - hash = c.hash; - idx = hash & newLength; - newTable[idx] = new CacheEntry(o, newTable[idx]); - segments[hash & SEGMENT_MASK].count++; - } - } - } - table = newTable; - } - } - } - } + private final AtomicInteger hashCodeCollisionCounter; /** - * Default constructor. - * - * @param c Runtime type of the objects that will be stored in the cache + * Creates a new cache. The "org.apache.fop.fo.properties.use-cache" system + * property is used to determine whether properties should actually be + * cached or not. If not, then the {@link #fetch(Object)} method will simply + * return its argument. To enable the cache, set this property to "true" + * (case insensitive). */ - public PropertyCache(Class c) { - this.useCache = Boolean.valueOf(System.getProperty( - "org.apache.fop.fo.properties.use-cache", "true") - ).booleanValue(); + public PropertyCache() { + this.useCache = Boolean.valueOf( + System.getProperty("org.apache.fop.fo.properties.use-cache", "true")) + .booleanValue(); if (useCache) { - for (int i = SEGMENT_MASK + 1; --i >= 0;) { - segments[i] = new CacheSegment(); - } + map = new ConcurrentHashMap>(); + putCounter = new AtomicInteger(); + cleanupLock = new ReentrantLock(); + hashCodeCollisionCounter = new AtomicInteger(); + } else { + map = null; + putCounter = null; + cleanupLock = null; + hashCodeCollisionCounter = null; } - this.runtimeType = c; } /** - * Generic fetch() method. - * Checks if the given Object is present in the cache - - * if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. + * Returns a cached version of the given object. If the object is not yet in + * the cache, it will be added and then returned. * - * @param obj the Object to check for - * @return the cached instance + * @param obj an object + * @return a cached version of the object */ - private Object fetch(Object obj) { + public T fetch(T obj) { if (!this.useCache) { return obj; } @@ -319,96 +108,80 @@ public final class PropertyCache { return null; } - Object cacheEntry = get(obj); - if (cacheEntry != null) { - return cacheEntry; - } - put(obj); - return obj; - } + Integer hashCode = Integer.valueOf(obj.hashCode()); - /** - * Checks if the given {@link Property} is present in the cache - - * if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param prop the Property instance to check for - * @return the cached instance - */ - public Property fetch(Property prop) { + WeakReference weakRef = map.get(hashCode); + if (weakRef == null) { + weakRef = map.putIfAbsent(hashCode, new WeakReference(obj)); + attemptCleanup(); - return (Property) fetch((Object) prop); - } + if (weakRef == null) { + return obj; + } + // else another thread added a value, continue. + } - /** - * Checks if the given {@link CommonHyphenation} is present in the cache - - * if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param chy the CommonHyphenation instance to check for - * @return the cached instance - */ - public CommonHyphenation fetch(CommonHyphenation chy) { + T cached = weakRef.get(); + if (cached != null) { + if (eq(cached, obj)) { + return cached; + } else { + /* + * Log a message when obj.getClass() does not implement correctly the equals() or + * hashCode() method. It is expected that only very few objects will have the + * same hashCode but will not be equal. + */ + if ((hashCodeCollisionCounter.incrementAndGet() % 10) == 0) { + LOG.info(hashCodeCollisionCounter.get() + " hashCode() collisions for " + + obj.getClass().getName()); + } + } - return (CommonHyphenation) fetch((Object) chy); - } + } - /** - * Checks if the given {@link CommonFont} is present in the cache - - * if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param cf the CommonFont instance to check for - * @return the cached instance - */ - public CommonFont fetch(CommonFont cf) { + // Adds a new or replaces an existing entry with obj that has the same hash code + map.put(hashCode, new WeakReference(obj)); + attemptCleanup(); + return obj; - return (CommonFont) fetch((Object) cf); + /* + * Another thread might add first. We could check this using map.replace() instead of + * map.put() and then recursively call fetch(obj). But if in the meantime, garbage + * collection kicks in, we might end up with a StackOverflowException. Not caching an entry + * is tolerable, after all it's configurable. + */ } - /** - * Checks if the given {@link CommonBorderPaddingBackground} is present in the cache - - * if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param cbpb the CommonBorderPaddingBackground instance to check for - * @return the cached instance - */ - public CommonBorderPaddingBackground fetch(CommonBorderPaddingBackground cbpb) { - return (CommonBorderPaddingBackground) fetch((Object) cbpb); - } + private void attemptCleanup() { + if ((putCounter.incrementAndGet() % 10000) != 0) { + return; + } - /** - * Checks if the given {@link CommonBorderPaddingBackground.BorderInfo} is present - * in the cache - if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param bi the BorderInfo instance to check for - * @return the cached instance - */ - public CommonBorderPaddingBackground.BorderInfo fetch( - CommonBorderPaddingBackground.BorderInfo bi) { - return (CommonBorderPaddingBackground.BorderInfo) fetch((Object) bi); + // Lock as there is no need for concurrent cleanup and protect us, on JDK5, from + // http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6312056 + if (cleanupLock.tryLock()) { + try { + cleanReclaimedMapEntries(); + } finally { + cleanupLock.unlock(); + } + } } - /** - * Checks if the given {@link org.apache.fop.fo.flow.Marker.MarkerAttribute} is present - * in the cache - if so, returns a reference to the cached instance. - * Otherwise the given object is added to the cache and returned. - * - * @param ma the MarkerAttribute instance to check for - * @return the cached instance - */ - public Marker.MarkerAttribute fetch( - Marker.MarkerAttribute ma) { - return (Marker.MarkerAttribute) fetch((Object) ma); + private void cleanReclaimedMapEntries() { + Iterator>> iterator = map.entrySet().iterator(); + while (iterator.hasNext()) { + Map.Entry> entry = iterator.next(); + WeakReference weakRef = entry.getValue(); + T r = weakRef.get(); + if (r == null) { + iterator.remove(); + } + } } - /** {@inheritDoc} */ - public String toString() { - return super.toString() + "[runtimeType=" + this.runtimeType + "]"; + private boolean eq(Object p, Object q) { + return (p == q || p.equals(q)); } - - } diff --git a/src/java/org/apache/fop/fo/properties/StringProperty.java b/src/java/org/apache/fop/fo/properties/StringProperty.java index 9bbe33070..d9718564f 100644 --- a/src/java/org/apache/fop/fo/properties/StringProperty.java +++ b/src/java/org/apache/fop/fo/properties/StringProperty.java @@ -78,8 +78,8 @@ public final class StringProperty extends Property { } /** cache containing all canonical StringProperty instances */ - private static final PropertyCache CACHE - = new PropertyCache(StringProperty.class); + private static final PropertyCache CACHE + = new PropertyCache(); /** canonical instance for empty strings */ public static final StringProperty EMPTY_STRING_PROPERTY = new StringProperty(""); @@ -104,8 +104,7 @@ public final class StringProperty extends Property { if ("".equals(str) || str == null) { return EMPTY_STRING_PROPERTY; } else { - return (StringProperty)CACHE.fetch( - new StringProperty(str)); + return CACHE.fetch(new StringProperty(str)); } } diff --git a/src/java/org/apache/fop/fo/properties/TableColLength.java b/src/java/org/apache/fop/fo/properties/TableColLength.java index ccb85bcfb..6c30b3123 100644 --- a/src/java/org/apache/fop/fo/properties/TableColLength.java +++ b/src/java/org/apache/fop/fo/properties/TableColLength.java @@ -22,6 +22,7 @@ package org.apache.fop.fo.properties; import org.apache.fop.datatypes.LengthBase; import org.apache.fop.datatypes.PercentBaseContext; import org.apache.fop.fo.FObj; +import org.apache.fop.util.CompareUtil; /** * A table-column width specification, possibly including some @@ -111,4 +112,25 @@ public class TableColLength extends LengthProperty { return (Double.toString(tcolUnits) + " table-column-units"); } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + CompareUtil.getHashCode(column); + result = prime * result + CompareUtil.getHashCode(tcolUnits); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof TableColLength)) { + return false; + } + TableColLength other = (TableColLength) obj; + return CompareUtil.equal(column, other.column) + && CompareUtil.equal(tcolUnits, other.tcolUnits); + } } diff --git a/src/java/org/apache/fop/fo/properties/ToBeImplementedProperty.java b/src/java/org/apache/fop/fo/properties/ToBeImplementedProperty.java index 59a6cafef..0315e0e74 100644 --- a/src/java/org/apache/fop/fo/properties/ToBeImplementedProperty.java +++ b/src/java/org/apache/fop/fo/properties/ToBeImplementedProperty.java @@ -40,15 +40,14 @@ public class ToBeImplementedProperty extends Property { super(propId); } - /** {@inheritDoc} */ + @Override public Property convertProperty(Property p, PropertyList propertyList, FObj fo) { if (p instanceof ToBeImplementedProperty) { return p; } - ToBeImplementedProperty val - = new ToBeImplementedProperty(getPropId()); + ToBeImplementedProperty val = new ToBeImplementedProperty(getPropId()); return val; } } @@ -67,5 +66,19 @@ public class ToBeImplementedProperty extends Property { // log.warn("property - \"" + propName // + "\" is not implemented yet."); } + + @Override + public int hashCode() { + return 0; + } + + @Override + public boolean equals(Object obj) { + return true; + /* + * Since a PropertyCache is not used here, returning true helps the PropertyCache when a non + * implemented property is part of an implemented one. + */ + } } diff --git a/src/java/org/apache/fop/fo/properties/URIProperty.java b/src/java/org/apache/fop/fo/properties/URIProperty.java index 9da01570c..cd610d35e 100644 --- a/src/java/org/apache/fop/fo/properties/URIProperty.java +++ b/src/java/org/apache/fop/fo/properties/URIProperty.java @@ -19,6 +19,8 @@ package org.apache.fop.fo.properties; +import static org.apache.fop.fo.Constants.PR_X_XML_BASE; + import java.net.URI; import java.net.URISyntaxException; @@ -26,8 +28,7 @@ import org.apache.fop.datatypes.URISpecification; import org.apache.fop.fo.FObj; import org.apache.fop.fo.PropertyList; import org.apache.fop.fo.expr.PropertyException; - -import static org.apache.fop.fo.Constants.PR_X_XML_BASE; +import org.apache.fop.util.CompareUtil; /** * Class modeling a property that has a value of type <uri-specification>. @@ -145,4 +146,26 @@ public class URIProperty extends Property { } } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + CompareUtil.getHashCode(getSpecifiedValue()); + result = prime * result + CompareUtil.getHashCode(resolvedURI); + return result; + } + + @Override + public boolean equals(Object obj) { + if (obj == null) { + return false; + } + if (!(obj instanceof URIProperty)) { + return false; + } + URIProperty other = (URIProperty) obj; + return CompareUtil.equal(getSpecifiedValue(), other.getSpecifiedValue()) + && CompareUtil.equal(resolvedURI, other.resolvedURI); + } + } diff --git a/src/java/org/apache/fop/util/CompareUtil.java b/src/java/org/apache/fop/util/CompareUtil.java new file mode 100644 index 000000000..c539ba416 --- /dev/null +++ b/src/java/org/apache/fop/util/CompareUtil.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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. + */ + +/* $Id$ */ + +package org.apache.fop.util; + +/** + * A utility class that provides helper methods for implementing equals and hashCode. + */ +public final class CompareUtil { + + private CompareUtil() { + } + + /** + * Compares two objects for equality. + * + * @param o1 an object + * @param o2 another object + * @return true if either o1 and o2 are null or if o1.equals(o2) + */ + public static boolean equal(Object o1, Object o2) { + return o1 == null ? o2 == null : o1 == o2 || o1.equals(o2); + } + + /** + * Returns the hash code of the given object. + * + * @param object an object + * @return object.hashCode(), or 0 if object is null + */ + public static int getHashCode(Object object) { + return object == null ? 0 : object.hashCode(); + } + + /** + * Compares two numbers for equality. Uses the same comparison algorithm as + * the {@link Double#equals(Object)} method. + * + * @param n1 a number + * @param n2 another number + * @return true if the two numbers are equal, false otherwise + */ + public static boolean equal(double n1, double n2) { + return Double.doubleToLongBits(n1) == Double.doubleToLongBits(n2); + } + + /** + * Returns a hash code for the given number. Applies the same algorithm as + * the {@link Double#hashCode()} method. + * + * @param number a number + * @return a hash code for that number + */ + public static int getHashCode(double number) { + long bits = Double.doubleToLongBits(number); + return (int) (bits ^ (bits >>> 32)); + } + +} diff --git a/status.xml b/status.xml index e34ecdb48..64c2d2ec3 100644 --- a/status.xml +++ b/status.xml @@ -62,6 +62,9 @@ documents. Example: the fix of marks layering will be such a case when it's done. --> + + Fixed deadlock in PropertyCache. + Added configuration option to set the version of the output PDF document. diff --git a/test/java/org/apache/fop/StandardTestSuite.java b/test/java/org/apache/fop/StandardTestSuite.java index eecdeb671..8649fdfa8 100644 --- a/test/java/org/apache/fop/StandardTestSuite.java +++ b/test/java/org/apache/fop/StandardTestSuite.java @@ -34,9 +34,10 @@ import org.apache.fop.fonts.type1.AdobeStandardEncodingTestCase; import org.apache.fop.image.loader.batik.ImageLoaderTestCase; import org.apache.fop.image.loader.batik.ImagePreloaderTestCase; import org.apache.fop.intermediate.IFMimickingTestCase; +import org.apache.fop.layoutmgr.PageSequenceLayoutManagerTestCase; +import org.apache.fop.pdf.PDFLibraryTestSuite; import org.apache.fop.render.extensions.prepress.PageBoundariesTestCase; import org.apache.fop.render.extensions.prepress.PageScaleTestCase; -import org.apache.fop.layoutmgr.PageSequenceLayoutManagerTestCase; import org.apache.fop.render.pdf.PDFAConformanceTestCase; import org.apache.fop.render.pdf.PDFCMapTestCase; import org.apache.fop.render.pdf.PDFEncodingTestCase; @@ -45,7 +46,6 @@ import org.apache.fop.render.pdf.RenderPDFTestSuite; import org.apache.fop.render.ps.PSTestSuite; import org.apache.fop.render.rtf.RichTextFormatTestSuite; import org.apache.fop.traits.MinOptMaxTestCase; -import org.apache.fop.pdf.PDFLibraryTestSuite; /** * Test suite for basic functionality of FOP. -- 2.39.5