Uploaded image for project: 'FOP'
  1. FOP
  2. FOP-2606

Save memory by clearing Markers

    XMLWordPrintableJSON

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.1
    • Fix Version/s: None
    • Component/s: fo/page
    • Environment:
      all

      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;
      }

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              agneta.walterscheidt@web.de Agneta Walterscheidt
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated: