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 "simon steiner (JIRA)" <ji...@apache.org> on 2017/02/09 14:34:41 UTC
[jira] [Commented] (FOP-2606) Save memory by clearing Markers
[ https://issues.apache.org/jira/browse/FOP-2606?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15859590#comment-15859590 ]
simon steiner commented on FOP-2606:
------------------------------------
Can you attach as a patch file
> Save memory by clearing Markers
> -------------------------------
>
> Key: FOP-2606
> URL: https://issues.apache.org/jira/browse/FOP-2606
> Project: FOP
> Issue Type: Improvement
> Components: fo/page
> Affects Versions: 2.1
> Environment: all
> Reporter: Agneta Walterscheidt
> Priority: Minor
> Labels: memory, performance
>
> When one of my fop transformations went out of memory I inspected the heap dump and found many instances of PageViewport.pageMarkers. These markers are kept for all pages although - as far as I see - for processed pages only the "last-within-page" markers are needed. Moreover the processed pages are searched from back to front which means that it would be sufficient to keep one marker instance for the page-sequence (=> throw away when a new page-sequence starts) and one instance for the document (=> never throw away) and there replace older markers with newer markers.
> I have implemented these changes on the trunk files and executed the build including the tests. You find a patch below.
> I think the best implementation depends on some decisions, for instance where the markers should be kept. Therefore could you please either integrate my changes or implement a similar solution?
> Index: area/AreaTreeModel.java
> ===================================================================
> --- area/AreaTreeModel.java (revision 1742452)
> +++ area/AreaTreeModel.java (working copy)
> @@ -27,6 +27,10 @@
>
> import org.apache.commons.logging.Log;
> import org.apache.commons.logging.LogFactory;
> +import org.apache.fop.fo.flow.AbstractRetrieveMarker;
> +import org.apache.fop.fo.flow.Marker;
> +import org.apache.fop.fo.flow.Markers;
> +import org.apache.fop.fo.flow.RetrieveMarker;
>
> /**
> * This is the model for the area tree object.
> @@ -42,6 +46,11 @@
>
> /** the current page sequence */
> protected PageSequence currentPageSequence;
> +
> + Markers documentMarkers = new Markers();
> +
> + Markers pageSequenceMarkers;
> +
> /** logger instance */
> protected static final Log log = LogFactory.getLog(AreaTreeModel.class);
>
> @@ -64,6 +73,7 @@
> currentPageIndex += currentPageSequence.getPageCount();
> }
> this.currentPageSequence = pageSequence;
> + pageSequenceMarkers = null;
> pageSequenceList.add(currentPageSequence);
> }
>
> @@ -77,7 +87,29 @@
> + currentPageSequence.getPageCount() - 1);
> page.setPageSequence(currentPageSequence);
> }
> +
> + public void addMarkers(Markers markers) {
> + if (markers != null) {
> + if (pageSequenceMarkers == null) {
> + pageSequenceMarkers = markers;
> + } else {
> + pageSequenceMarkers.add(markers);
> + }
> + documentMarkers.add(markers);
> + }
> + }
>
> + public Marker resolveMarker(AbstractRetrieveMarker arm, boolean doc) {
> + Marker mark = null;
> + if (pageSequenceMarkers != null) {
> + mark = pageSequenceMarkers.resolve(arm);
> + }
> + if (mark == null && doc) {
> + mark = documentMarkers.resolve(arm);
> + }
> + return mark;
> + }
> +
> /**
> * Handle an OffDocumentItem
> * @param ext the extension to handle
> Index: area/PageViewport.java
> ===================================================================
> --- area/PageViewport.java (revision 1742452)
> +++ area/PageViewport.java (working copy)
> @@ -538,4 +538,13 @@
> }
> }
>
> + public Markers removeMarkers() {
> + if (pageMarkers == null) {
> + return null;
> + }
> + Markers markers = pageMarkers.createLastMarkers();
> + pageMarkers = null;
> + return markers;
> + }
> +
> }
> Index: fo/flow/Markers.java
> ===================================================================
> --- fo/flow/Markers.java (revision 1742452)
> +++ fo/flow/Markers.java (working copy)
> @@ -213,4 +213,51 @@
> }
> }
>
> + public Markers createLastMarkers() {
> + Markers markers = null;
> + if (lastQualifyingIsAny != null) {
> + markers = new Markers();
> + markers.lastQualifyingIsAny = new HashMap<String, Marker>(lastQualifyingIsAny);
> +
> + if (lastQualifyingIsLast != null) {
> + markers.lastQualifyingIsAny.putAll(lastQualifyingIsLast);
> + }
> + }
> + return markers;
> + }
> +
> + public void add(Markers markers) {
> + if (markers.firstQualifyingIsFirst != null) {
> + if (firstQualifyingIsFirst == null) {
> + firstQualifyingIsFirst = new HashMap<String, Marker>();
> + }
> + firstQualifyingIsFirst.putAll(markers.firstQualifyingIsFirst);
> + }
> + if (markers.firstQualifyingIsAny != null) {
> + if (firstQualifyingIsAny == null) {
> + firstQualifyingIsAny = new HashMap<String, Marker>();
> + }
> + firstQualifyingIsAny.putAll(markers.firstQualifyingIsAny);
> + }
> + if (markers.lastQualifyingIsFirst != null) {
> + if (lastQualifyingIsFirst == null) {
> + lastQualifyingIsFirst = new HashMap<String, Marker>();
> + }
> + lastQualifyingIsFirst.putAll(markers.lastQualifyingIsFirst);
> + }
> + if (markers.lastQualifyingIsLast != null) {
> + if (lastQualifyingIsLast == null) {
> + lastQualifyingIsLast = new HashMap<String, Marker>();
> + }
> + lastQualifyingIsLast.putAll(markers.lastQualifyingIsLast);
> + }
> + if (markers.lastQualifyingIsAny != null) {
> + if (lastQualifyingIsAny == null) {
> + lastQualifyingIsAny = new HashMap<String, Marker>();
> + }
> + lastQualifyingIsAny.putAll(markers.lastQualifyingIsAny);
> + }
> +
> + }
> +
> }
> Index: layoutmgr/AbstractPageSequenceLayoutManager.java
> ===================================================================
> --- layoutmgr/AbstractPageSequenceLayoutManager.java (revision 1742452)
> +++ layoutmgr/AbstractPageSequenceLayoutManager.java (working copy)
> @@ -32,6 +32,7 @@
> import org.apache.fop.datatypes.Numeric;
> import org.apache.fop.fo.Constants;
> import org.apache.fop.fo.flow.Marker;
> +import org.apache.fop.fo.flow.Markers;
> import org.apache.fop.fo.flow.RetrieveMarker;
> import org.apache.fop.fo.pagination.AbstractPageSequence;
>
> @@ -231,31 +232,36 @@
> // get marker from the current markers on area tree
> Marker mark = getCurrentPV().resolveMarker(rm);
> if (mark == null && boundary != EN_PAGE) {
> + boolean doc = (boundary == EN_DOCUMENT);
> + int originalPosition = rm.getPosition();
> + rm.changePositionTo(Constants.EN_LEWP);
> + mark = areaTreeModel.resolveMarker(rm, doc);
> + rm.changePositionTo(originalPosition);
> +
> // go back over pages until mark found
> // if document boundary then keep going
> - boolean doc = (boundary == EN_DOCUMENT);
> - int seq = areaTreeModel.getPageSequenceCount();
> - int page = areaTreeModel.getPageCount(seq) - 1;
> - while (page < 0 && doc && seq > 1) {
> - seq--;
> - page = areaTreeModel.getPageCount(seq) - 1;
> - }
> - while (page >= 0) {
> - PageViewport pv = areaTreeModel.getPage(seq, page);
> - int originalPosition = rm.getPosition();
> - rm.changePositionTo(Constants.EN_LEWP);
> - mark = pv.resolveMarker(rm);
> - // this is probably not necessary since the RM will not be used again, but to be safe...
> - rm.changePositionTo(originalPosition);
> - if (mark != null) {
> - break;
> - }
> - page--;
> - if (page < 0 && doc && seq > 1) {
> - seq--;
> - page = areaTreeModel.getPageCount(seq) - 1;
> - }
> - }
> +// int seq = areaTreeModel.getPageSequenceCount();
> +// int page = areaTreeModel.getPageCount(seq) - 1;
> +// while (page < 0 && doc && seq > 1) {
> +// seq--;
> +// page = areaTreeModel.getPageCount(seq) - 1;
> +// }
> +// while (page >= 0) {
> +// PageViewport pv = areaTreeModel.getPage(seq, page);
> +// int originalPosition = rm.getPosition();
> +// rm.changePositionTo(Constants.EN_LEWP);
> +// mark = pv.resolveMarker(rm);
> +// // this is probably not necessary since the RM will not be used again, but to be safe...
> +// rm.changePositionTo(originalPosition);
> +// if (mark != null) {
> +// break;
> +// }
> +// page--;
> +// if (page < 0 && doc && seq > 1) {
> +// seq--;
> +// page = areaTreeModel.getPageCount(seq) - 1;
> +// }
> +// }
> }
>
> if (mark == null) {
> @@ -317,6 +323,10 @@
> log.debug("page finished: " + curPage.getPageViewport().getPageNumberString()
> + ", current num: " + currentPageNum);
> }
> + Markers markers = curPage.getPageViewport().removeMarkers();
> + if (markers != null) {
> + areaTreeHandler.getAreaTreeModel().addMarkers(markers);
> + }
> curPage = null;
> }
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)