DO NOT REPLY [Bug 41044] - [PATCH] FOP memory usage patches.

[bu...@apache.org]

DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG�
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=41044>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND�
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=41044





------- Additional Comments From a_l.delmelle@pandora.be  2007-06-25 15:39 -------
Hi Richard,

I've finally found some time to finish reviewing your patch, and it looked good at first glance, apart 
from some style issues (javadocs, bracing conditional branches even if there's only one statement, 
blabla) and what I take to be some small typos (for example: in CommonHyphenation.equals(), you 
compared the country, language and script properties to themselves instead of to their counterparts 
from the other instance).

Unfortunately, when I ran the junit tests, there seems to be a lot of work before we can apply it as is... I 
get 70 errors. :/ (a lot of them NPEs)
Am I doing something wrong, or did you skip the testsuite?

The only thing I really threw out were the deprecated-tags from CompoundDataType and 
LengthRangeProperty. This does need some further thought, nevertheless.
For CommonBorderPaddingBackground.setBorderInfo(), I agreed. So much even that I removed 
setBorderInfo() and setPadding() completely, instead of merely deprecating.

For now, I'll offer a bit more info on the rationale behind the mutability of compounds, and an idea on 
how to tackle it.
Compounds would be tricky in terms of canonicalization, because, in order to judge which compound 
instance you need, you have to have all the components available (e.g.: for space-before you need 
the .minimum, .maximum, .optimum, .conditionality and .precedence components).
This means that, currently, in order to correctly canonicalize a compound, we would have to wait until 
all attributes for a given FO have been converted to properties. We cannot do so at the time the 
compound is first created, since it will at that point only have the specified value of /one/ of the 
components (the first one in the list)

The sole reason why they are mutable is that they are not created in one go, but rather, the base-
property is created once the first component is encountered in the attribute-list. Further on in the 
process, as the other components are converted, the PropertyMakers need to be able to change the 
base-property (through setComponent())

A possible solution I'm thinking of is to rewrite part of the convertAttributeToProperty-loop in 
PropertyList. If we can make sure that compounds are always created as a single compound property 
(instead of the multiple components each setting the component of a base-property), then they would 
not need to be mutable at all, and the issue is rendered moot. 
FWIW, I've always disliked a bit the fact that the attributes are simply looped over. The hacks a bit 
higher up, in addAttributesToList(), for extracting column-number and font-size before all others, are 
to me an indication that this should be revisited.

As I roughly see it ATM, a compound would be made/instantiated using a list of attributes, instead of 
only one. As soon as one component is encountered, we would immediately scan the Attributes, 
extracting all the other components, and can then pass them all into the compound's PropertyMaker. As 
such, there is no more need for the setComponent() methods, since by converting one component's 
attribute, we would immediately convert the whole compound.

WDYT?

Andreas

-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.