[ 
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)

Reply via email to