(In reply to comment #3)
> Thanks for the patch, Dario. I agree with Andreas that the code in
> AbstractLayoutManager should really be in BlockStackingLayoutManager. I can
> faintly remember that I've once investigated the right time to release memory
> in a layout manager. I think that was for page-number-citation-last. I'm not
> sure that we already always know exactly when a layout manager is really
> finished. Anyway, we've got a good test suite so any bugs in that area should
> quickly be found.
I was working on this patch in October, I remember that the automated test on the area generated was failing because the check was based on the freed area... Anyway I've never encountered problems in my typical usage, fop just prints this warning when updating the area:
11-mar-2009 9.24.58 org.apache.fop.area.inline.UnresolvedPageNumber resolveIDRef
AVVERTENZA: Cannot update the IPD of an unresolved page number. No font information available.
But the final result is fine.
> @Andreas: what does "beneficial" mean? Beneficial in terms of memory
> consumption or of speed? I think that can be mutually exclusive. Before we
> decide to enable this by default I would want to have some numbers (performance
> & memory usage) to base the decision upon.
I think that area caching shouldn't be the default option: it provides a significant memory improvement for some cases (large documents with many page-seq plus page citation) but in most cases it would just slowdown the process (disk access for read/write) without any benefit.
It is useful in my scenario where I produce 50000 pages document, that means 400-500 MB of RAM and 4 GB of disk cache (that otherwise should reside in memory...).
For completeness, if I just want the total page number I can achieve better memory results by using the hack of the two pass indicated in http://xmlgraphics.apache.org/fop/fo.html#fo-total-pages, but it is slower. Theoretically it should be twice slower, but in practice the update process plus the disk access reduces the time gap.
Anyway I think that the two pass method can't work if you have many cross reference between page-seqs and not only the total page number.
> Another comment to the patch: I'm not particularly happy about calling the
> temporary area tree unloading to disk simply "the cache". We have a font cache,
> an image cache and others. I'm not even sure if this is really a classic
> "cache" as such. We need a better name for this IMO. How about
> FOUserAgent.setTemporaryAreaTreeSerializationEnabled(boolean)? Or
> FOUserAgent.setDiskBasedRenderPagesModel(boolean)? A bit lengthy but at least
> more speaking. Or FOUserAgent.setConserveMemoryPolicy(boolean) which would be a
> less specific approach that other code parts also could use as hint?
I agree, "cache" is inappropriate, "setConserveMemoryPolicy" is nice and blink the eye to non fop-dev that just want to embed fop.