You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by gm...@apache.org on 2005/06/06 07:46:00 UTC

cvs commit: xml-fop/src/java/org/apache/fop/area PageViewport.java

gmazza      2005/06/05 22:46:00

  Modified:    src/java/org/apache/fop/layoutmgr
                        PageSequenceLayoutManager.java
               src/java/org/apache/fop/area PageViewport.java
  Log:
  Minor PSLM simplifications.
  
  Revision  Changes    Path
  1.64      +8 -25     xml-fop/src/java/org/apache/fop/layoutmgr/PageSequenceLayoutManager.java
  
  Index: PageSequenceLayoutManager.java
  ===================================================================
  RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/layoutmgr/PageSequenceLayoutManager.java,v
  retrieving revision 1.63
  retrieving revision 1.64
  diff -u -r1.63 -r1.64
  --- PageSequenceLayoutManager.java	5 Jun 2005 07:11:22 -0000	1.63
  +++ PageSequenceLayoutManager.java	6 Jun 2005 05:46:00 -0000	1.64
  @@ -75,12 +75,6 @@
        */
       private FlowLayoutManager childFLM = null;
   
  -    /**
  -     * The collection of StaticContentLayoutManager objects that
  -     * are associated with this Page Sequence, keyed by flow-name.
  -     */
  -    //private HashMap staticContentLMs = new HashMap(4);
  -
       private int startPageNum = 0;
       private int currentPageNum = 0;
   
  @@ -107,9 +101,10 @@
       }
   
       /**
  -     * Start the layout of this page sequence.
  -     * This completes the layout of the page sequence
  -     * which creates and adds all the pages to the area tree.
  +     * Activate the layout of this page sequence.
  +     * PageViewports corresponding to each page generated by this 
  +     * page sequence will be created and sent to the AreaTreeModel
  +     * for rendering.
        */
       public void activateLayout() {
           startPageNum = pageSeq.getStartingPageNumber();
  @@ -457,17 +452,10 @@
               throw new IllegalArgumentException("Cannot create page: " + fopex.getMessage());
           }
   
  -        if (log.isDebugEnabled()) {
  -            log.debug("[" + curPV.getPageNumberString() + (bIsBlank ? "*" : "") + "]");
  -        }
  -
  -        curPV.createSpan(false);
  +        log.debug("[" + curPV.getPageNumberString() + (bIsBlank ? "*" : "") + "]");
           return curPV;
       }
   
  -    /* TODO: See if can initialize the SCLM's just once for
  -     * the page sequence, instead of after every page.
  -     */
       private void layoutSideRegion(int regionID) {
           SideRegion reg = (SideRegion)curPV.getSPM().getRegion(regionID);
           if (reg == null) {
  @@ -482,7 +470,6 @@
               getLayoutManagerMaker().makeStaticContentLayoutManager(
                       this, sc, reg);
           lm.doLayout();
  -        lm.reset(null);
       }
   
       private void finishPage() {
  @@ -528,12 +515,8 @@
       }
   
       /**
  -     * If we have already started to layout content on a page,
  -     * and there is a forced break, see if we need to generate
  -     * an empty page.
  -     * Note that if not all content is placed, we aren't sure whether
  -     * it will flow onto another page or not, so we'd probably better
  -     * block until the queue of layoutable stuff is empty!
  +     * Check if a blank page is needed to accomodate
  +     * desired even or odd page number.
        * @param breakVal - value of break-before or break-after trait.
        */
       private boolean needBlankPageBeforeNew(int breakVal) {
  
  
  
  1.17      +1 -1      xml-fop/src/java/org/apache/fop/area/PageViewport.java
  
  Index: PageViewport.java
  ===================================================================
  RCS file: /home/cvs/xml-fop/src/java/org/apache/fop/area/PageViewport.java,v
  retrieving revision 1.16
  retrieving revision 1.17
  diff -u -r1.16 -r1.17
  --- PageViewport.java	16 May 2005 02:54:19 -0000	1.16
  +++ PageViewport.java	6 Jun 2005 05:46:00 -0000	1.17
  @@ -33,7 +33,6 @@
   
   import org.apache.fop.fo.Constants;
   import org.apache.fop.fo.pagination.SimplePageMaster;
  -import org.apache.fop.layoutmgr.StaticContentLayoutManager;
   
   /**
    * Page viewport that specifies the viewport area and holds the page contents.
  @@ -85,6 +84,7 @@
           pageNumberString = pageStr;
           viewArea = new Rectangle(0, 0, pageWidth, pageHeight);
           page = new Page(spm);
  +        createSpan(false);
       }
   
       /**
  
  
  

---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: Impact of logging on performance (was: Re: cvs commit: xml-fop/src/java/org/apache/fop/area PageViewport.java)

Posted by Jeremias Maerki <de...@greenmail.ch>.
On 06.06.2005 09:15:34 Jeremias Maerki wrote:
> operator it depends on the intelligence of the JIT compiler whether it

That should have been the Java Compiler, not the JIT compiler.

Jeremias Maerki


Re: Impact of logging on performance (was: Re: cvs commit: xml-fop/src/java/org/apache/fop/area PageViewport.java)

Posted by Glen Mazza <gr...@verizon.net>.
Yes, you're absolutely correct.  Up until now my "ROT" was pure line
count -- if just one, don't bother with the isDebugEnabled().  But now I see
the need to also check the parameter supplied.  BTW, if you're making
changes to the code today if you can take care of this it would be
appreciated.  Else I'll return it when I next make a CVS commit.

Thanks,
Glen

----- Original Message ----- 
>
> General rule of thumb: Always surround a debug log statement with a
> isDebugEnabled() check if the log statement is non-trivial (i.e.
> not a simple constant String).
>


Impact of logging on performance (was: Re: cvs commit: xml-fop/src/java/org/apache/fop/area PageViewport.java)

Posted by Jeremias Maerki <de...@greenmail.ch>.
An explanation why the "log.isDebugEnabled()" was there and why it
should be added again (maybe not necessarily in this case since it's
only called once per page but as a general rule):

"+" operations with Strings usually results in a StringBuffer being
created the two Strings before and after the operator being combined and
then converted to a String object again. If you have multiple "+"
operator it depends on the intelligence of the JIT compiler whether it
creates only one StringBuffer or one for each "+" operation. Older VMs
always created multiple StringBuffers.

What happens without the isDebugEnabled() is that the combined String is
always (!) built (possibly multiple new String and StringBuffer
instances as well as possibly multiple calls to other methods for
building te String) and then passed into the debug() method. Only then
will the logging implementation check if the log level is sufficient to
output the log statement.

General rule of thumb: Always surround a debug log statement with a
isDebugEnabled() check if the log statement is non-trivial (i.e.
not a simple constant String).

More information:
http://jakarta.apache.org/commons/logging/api/org/apache/commons/logging/Log.html
http://logging.apache.org/log4j/docs/manual.html#performance
http://excalibur.apache.org/apidocs/org/apache/avalon/framework/logger/Logger.html
https://www.qos.ch/ac2001/F11-190.html

On 06.06.2005 07:46:00 gmazza wrote:
>   -        if (log.isDebugEnabled()) {
>   -            log.debug("[" + curPV.getPageNumberString() + (bIsBlank ? "*" : "") + "]");
>   -        }
>   -
>   -        curPV.createSpan(false);
>   +        log.debug("[" + curPV.getPageNumberString() + (bIsBlank ? "*" : "") + "]");
>            return curPV;


Jeremias Maerki