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
