You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-dev@xmlgraphics.apache.org by bu...@apache.org on 2007/02/19 22:43:51 UTC
DO NOT REPLY [Bug 41656] New: - [PATCH] Proposal: some refactoring in the fo tree
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=41656>.
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=41656
Summary: [PATCH] Proposal: some refactoring in the fo tree
Product: Fop
Version: 1.0dev
Platform: All
OS/Version: other
Status: NEW
Severity: normal
Priority: P2
Component: fo tree
AssignedTo: fop-dev@xmlgraphics.apache.org
ReportedBy: a_l.delmelle@pandora.be
The proposed patch refactors the fo tree in the following ways:
-> removal of the childNodes instance member in fop.fo.FObj
-> addition of a firstChild instance member in fop.fo.FObj
-> addition of a siblings instance member in fop.fo.FONode
-> addition of a FONodeIterator interface in FONode + corresponding implementation in FObj
-> changed implementations of FObj.addChildNode(), .removeChild() and .getChildNodes()
Possible benefits:
Reduction of the number of ArrayLists used by the fo tree.
Before the patch, for every FObj that has at least one child, an ArrayList is instantiated with a default
capacity of 10 elements (a backing Object[10], effectively wasting space on nulls in a substantial
amount of cases)
After the patch, every FObj has a maximum of 3 references: one first-child (possibly null), a preceding
and a following sibling. If the node only has children, it will have only two references: one child, one
uninitialized array of siblings.
Downside is that this means an added reference for any FONode, even FOText or a single child, but this
may be negligeable, considering that most FObj subtypes in general have a quite a few references
already (mostly properties).
Another upside: this refactoring made it possible to remove fo:character nodes during the refinement
white-space-handling /as they were encountered/, so there was no more need to wait for the
RecursiveCharIterator to go out of scope. The FObjIterator does not throw a
ConcurrentModificationException, since there is no real backing list anymore.
More generally, while extremely dangerous if used carelessly, this does open up the possibility of
starting to process the virtual list of child nodes while another thread keeps appending nodes to the
end.
Since this is only a proof-of-concept, I'd appreciate any feedback, especially from the profiler-geeks
out there (Richard? I'd be very interested to see how many more cells you can process before running
into an OOMError...)
I'd expect it to save us 'some' heap space, no idea how much. OTOH, I'd also expect a slowdown --
since an add() means traversing the following-sibling axis until the last one is reached. I have not yet
performed any optimizations where it comes to FOs with a lot of children. Maybe a solution would be to
use an iterator for the addition, and append the new child to its previous element...?
In the (very) long run, parts of the layout algorithm may already be triggered sooner.
For example: layoutmanager initialization, base element list generation, providing new pages...
This latter would, however, take quite a bit of refactoring, so don't get any wild ideas. ;)
--
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.
Re: DO NOT REPLY [Bug 41656] New: - [PATCH] Proposal: some refactoring in the fo tree
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Feb 21, 2007, at 12:16, richardw@geoquip-rnd.demon.co.uk wrote:
> bugzilla@apache.org writes:
>> (Richard? I'd be very interested to see how many more cells you
>> can process before running
>> into an OOMError...)
>
> I'm not dead. I am still paying attention. Life is just hectic atm.
We all know the feeling... Take your time. As I indicated in a recent
mail on fop-users: no strict deadlines here. It should be all about
'the fun of coding', no?
Cheers,
Andreas
Re: DO NOT REPLY [Bug 41656] New: - [PATCH] Proposal: some refactoring in the fo tree
Posted by ri...@geoquip-rnd.demon.co.uk.
bugzilla@apache.org writes:
> (Richard? I'd be very interested to see how many more cells you can process before running
> into an OOMError...)
I'm not dead. I am still paying attention. Life is just hectic atm. I've
still got some more updates to make, including fixing the TXTHandler wrt
the recent changes. Hopefully I'll get back to FOP issues in a few weeks
time,
Regards,
Richard
Re: DO NOT REPLY [Bug 41656] - [PATCH] Proposal: some refactoring in the fo tree
Posted by Andreas L Delmelle <a_...@pandora.be>.
On Jul 7, 2007, at 03:11, bugzilla@apache.org wrote:
>
> ------- Additional Comments From a_l.delmelle@pandora.be
> 2007-07-06 18:11 -------
>
> Looking again at this patch, and Richard's observations, I'd say it
> is worth it. Breaks nothing, and the
> benefit seems to become bigger with the number of FOs.
>
> Patch applied. see: http://svn.apache.org/viewvc?view=rev&rev=554104
Sorry, people. Just noticed I forgot some javadocs... I'll add these
ASAP.
Cheers
Andreas
DO NOT REPLY [Bug 41656] - [PATCH] Proposal: some refactoring in the fo tree
Posted by 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=41656>.
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=41656
a_l.delmelle@pandora.be changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
------- Additional Comments From a_l.delmelle@pandora.be 2007-07-06 18:11 -------
Looking again at this patch, and Richard's observations, I'd say it is worth it. Breaks nothing, and the
benefit seems to become bigger with the number of FOs.
Patch applied. see: http://svn.apache.org/viewvc?view=rev&rev=554104
--
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.
DO NOT REPLY [Bug 41656] - [PATCH] Proposal: some refactoring in the fo tree
Posted by 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=41656>.
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=41656
------- Additional Comments From richardw@geoquip-rnd.demon.co.uk 2007-04-25 08:30 -------
See http://issues.apache.org/bugzilla/show_bug.cgi?id=41044 for details of other
memory usage patches.
Having done some tests, this patch takes the number of processable table cells
on my given example from 30406 up to 32442. The patches in 41044 take the number
from 30406 up to 83197. Applying both patches allows me to process 100803 cells.
Hence, I'd say both patches look like a good improvement - assuming they don't
break anything else I haven't looked at.
--
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.
DO NOT REPLY [Bug 41656] - [PATCH] Proposal: some refactoring in the fo tree
Posted by 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=41656>.
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=41656
------- Additional Comments From a_l.delmelle@pandora.be 2007-02-19 13:44 -------
Created an attachment (id=19612)
--> (http://issues.apache.org/bugzilla/attachment.cgi?id=19612&action=view)
patch containing the proposed changes
--
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.