Want to find a memory leak? SAP's memory analyzer is great again to find
out all paths to GC roots:
Class Name
|Shallow Heap |Retained Heap
---------------------------------------------------------------------------------------------------------------------------------------------
org.apache.fop.fo.properties.FixedLength @ 0x6ad2d98
| 24| 24
'- value, referent java.util.WeakHashMap$Entry @ 0x6aeee30
| 40| 120
'- [90] java.util.WeakHashMap$Entry[128] @ 0x6abc818
| 528| 3'592
'- table java.util.WeakHashMap @ 0x6a0b2f0
| 48| 3'672
'- m java.util.Collections$SynchronizedMap @ 0x6a0b2d0
| 32| 3'704
'- propCache org.apache.fop.fo.properties.PropertyCache @
0x6a0b2c0 | 16| 3'720
'- cache class org.apache.fop.fo.properties.FixedLength @
0x31b26a0 | 8| 3'728
|- <class> org.apache.fop.fo.properties.FixedLength @
0x6ab3ff0 | 24| 24
| '- textIndent, lastLineEndIndent
org.apache.fop.fo.flow.Block @ 0xa61ead8 | 184|
496
| |- currentFObj
org.apache.fop.fo.FOTreeBuilder$MainFOHandler @ 0x6ab53a0 [Java Local]|
24| 24
| | '- java.lang.Thread @ 0x6a2aa10 main [Thread]
| 104| 704
| '- parent org.apache.fop.fo.flow.Block @ 0xa61fb48
[Java Local] | 184| 184
|- <class> org.apache.fop.fo.properties.FixedLength @
0xa61f6c8 | 24| 24
|- [555] java.lang.Object[1280] @ 0x6ab9100
| 5'136| 202'392
'- <class> org.apache.fop.fo.properties.FixedLength @
0x6ab4108 | 24| 24
---------------------------------------------------------------------------------------------------------------------------------------------
As a simplification here, let's assume "cache" (PropertyCache in static
variable) is our GC root. Hard references are to "propCache", then "m" and
"table". From there it goes to the WeakHashMap$Entry in the array with a
hard reference. From the Entry, you have a hard reference (!) to the
FixedLength instance through "value" and a weak reference through
"referent" (which you can find in java.lang.ref.Reference). QED.
If this were my current focus, I'd remove PropertyCache in favor of
specialized and optimized FlyWeightFactories and I'd attach those to the
rendering run so I can get rid of the need for thread synchronization
and weak references. I'd do some isolated timing experiments comparing
PropertyCache and the FlyWeightFactories. But since this has to wait
until later this year, I guess I'm happy enough if you guys trying out
Adrian's suggestion in order to get a quick solution.
On 20.07.2007 16:12:45 Andreas L Delmelle wrote:
> On Jul 20, 2007, at 14:15, Jeremias Maerki wrote:
>
> > Another argument: Why did Chris have an OutOfMemoryError in the first
> > place that the fix for FixedLengthProperty solved?
> > The OutOfMemoryError doesn't happen anymore because not so many
> > objects
> > are created to actually be filling the memory.
>
> That was precisely the point, since:
> a) specified properties very frequently have recurring values in the
> same document as well as over different documents.
> b) default/initial values are generally the same for a given property
> in all documents
> c) computed values for inherited properties are frequently identical
> on parent and child
>
> Even if you render a million documents, there would always be one and
> only one FixedLength instance corresponding to an absolute computed
> value of 10pt, no matter if it is used for font-size, line-height,
> padding...
>
> > If the objects in the cache had been releasable, they would have
> > been released
> > by the GC and Chris wouldn't have run into the OutOfMemoryError.
>
> Depends on how you look at it:
> I don't believe the reference to the value object that is held by the
> Map.Entry object itself is of issue here.
> However, since that is a strong reference (what the API doc means to
> say), then if the value would in turn /hold/ (not /be/) a reference
> to the key object, the references would become circular and the fact
> that the key is wrapped in a WeakReference does not do anything useful.
> If key and value are the same objects, there is no circular
> reference, hence no particular problem. The only strong reference to
> the key is then ultimately held by the *Map.Entry*, not the value
> object (which happens to be the key).
>
> Could be I'm incorrect in my interpretation here. If I am, then the
> solution could still be pretty simple (as I see Adrian has already
> suggested):
> replace put(key, value) with put(key, new WeakReference(value));
>
> That would make sure that, even if the WeakHashMap implementation
> cannot resolve situations where one of its entries contains the only
> strong reference to the key --which I doubt to be the meaning of the
> note, but could be wrong--, the entries are always cleared when no
> objects outside the Map refer to the key.
>
> Another note:
> As to /when precisely/ an entry in a WeakHashMap /does/ get cleared
> by GC, that is guaranteed nowhere. The only guarantee is that "an
> entry in a WeakHashMap will automatically be removed when its key is
> no longer in ordinary use". So, it is not because there are no
> references to the key that the entry will be /immediately/ cleared,
> only that it becomes eligible for collection in the next GC cycle.
>
> This could make it JVM-dependent, and may perhaps explain why I could
> not reproduce the bug.... Some JVMs are better at collecting obsolete
> objects held in static Maps than others. If this is a deciding
> factor, I'd only expect this to be a problem in older versions (less
> advanced GC algorithms).
>
> I'm also thinking, maybe the fact that instances corresponding to
> default/initial values are, in turn, cached by the static
> PropertyMakers in FOPropertyMapping causes carry-over of instances
> into other runs.
>
> (check:
> PropertyMaker.make(PropertyList) {
> if (default != null) {
> return default;
> } else {
> ...
> default = p;
> }
> )
>
> That would make instances corresponding to an initial value indeed
> non-collectable once FOP has processed one document.
> Unless when there are disastrous bugs like the one in FixedLength,
> nothing alarming, since those instances will be used many times over
> after that.
>
>
> FWIW: I agree very much that the caches should probably be made
> global in another way, either tie them to the FopFactory somehow, or
> to a specific rendering run, as you suggest. The first would have the
> benefit of being able to re-use the same NumberProperty instance in
> different subsequent renderings.
>
> I experimented a bit with a Map of (Class->Map) mappings, where the
> key in the outer map is the Property-subclass, and the inner Map
> contains the canonical instances for that type of Property. As I
> remember this was quite slow (presumably due to the increased number
> of lookups), so I decided not to employ that particular strategy.
>
> That said, I'm still very open to suggestions.
>
>
> Cheers
>
> Andreas
Jeremias Maerki
