Taking a look at the code and reading the spec :-
public class PropertyCache {
private Map propCache = Collections.synchronizedMap(new WeakHashMap());
/**
* Checks if the given property is present in the cache - if so,
returns
* a reference to the cached value. Otherwise the given object is
added
* to the cache and returned.
* @param obj
* @return the cached instance
*/
public Property fetch(Property prop) {
Property cacheEntry = (Property) propCache.get(prop);
if (cacheEntry != null) {
return cacheEntry;
} else {
propCache.put(prop, prop);
return prop;
}
}
}
"Implementation note: The value objects in a WeakHashMap are held by
ordinary strong references. Thus care should be taken to ensure that
value objects do not strongly refer to their own keys, either directly
or indirectly, since that will prevent the keys from being discarded.
Note that a value object may refer indirectly to its key via the
WeakHashMap itself; that is, a value object may strongly refer to some
other key object whose associated value object, in turn, strongly refers
to the key of the first value object. One way to deal with this is to
wrap values themselves within WeakReferences before inserting, as in:
m.put(key, new WeakReference(value)), and then unwrapping upon each get."
I have to agree with Jeremias, the value does need to be wrapped in a
WeakReference object when it is put() in the PropertyCache as the value
reference will be held as a strong reference as it directly holds the
same reference value as its key.
This patch should single line patch should fix it.
-- snip --
Index: src/java/org/apache/fop/fo/properties/PropertyCache.java
===================================================================
--- src/java/org/apache/fop/fo/properties/PropertyCache.java
(revision 555659)
+++ src/java/org/apache/fop/fo/properties/PropertyCache.java (working
copy)
@@ -19,6 +19,7 @@
package org.apache.fop.fo.properties;
+import java.lang.ref.WeakReference;
import java.util.Collections;
import java.util.Map;
import java.util.WeakHashMap;
@@ -48,7 +49,7 @@
if (cacheEntry != null) {
return cacheEntry;
} else {
- propCache.put(prop, prop);
+ propCache.put(prop, new WeakReference(prop));
return prop;
}
}
-- snip --
Adrian.
Chris Bowditch wrote:
Jeremias Maerki wrote:
On 20.07.2007 12:51:33 Chris Bowditch wrote:
Jeremias Maerki wrote:
On 20.07.2007 11:52:15 Andreas L Delmelle wrote:
<snip/>
In addition to that there was a bug in FixedLength.equals() that made
the caching effect-less:
http://svn.apache.org/viewvc?view=rev&rev=557934
That was the most likely cause. The equals() method returning
false because of this, would keep on creating separate instances.
How they would be leaked into a subsequent run is not quite clear
to me, though...
Because of the bug in PropertyCache. The two bugs just add to each
other.
Thanks for spotting this bug in FixedLength.equals(). I rebuilt
fop.jar and started a fresh profiling session. After 5000 renderings
with only 16Mb the heap is staying around 3Mb. The number of
FixedLength and WeakHashMap$entry object is staying fixed at around
300. Hip Hip Hooray!!!
But you're running the same document over and over again, right? In that
case, the WeakHashMap doesn't grow because the cached instances are
always reused. That doesn't address the fact that the instances are
still not releasable. The memory leak is still there. That's why it's so
extremely important to be so damn careful about using static variables.
You are right of course. So if we ran lots of different documents with a
very high number of variations in FixedLength then the memory leak still
exists. It does sound to me from what you've been saying about the
WekHashMap that more work in this area is still needed.
<snip/>
Chris
