[FOP-2606] Save memory by clearing Markers

• Type: Improvement
• Status: Open
• Priority: Minor
• Resolution: Unresolved
• Affects Version/s: 2.1
• Component/s: fo/page
• Labels: memory performance
• Environment: all
• Assignee: Unassigned
• Reporter: Agneta Walterscheidt
• Created: 05/May/16 18:33
• Updated: 09/Feb/17 14:33

Description

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)

  Unknown macro: {+// 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;
}