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 Adrian Cumiskey <de...@cumiskey.com> on 2008/11/10 12:29:09 UTC

FIleChannel accessible BufferedOutputStream

When FOP is started in Main.startFOP() a java.io.BufferedOutputStream is instantiated and wrapped 
around an instantiated java.io.FileOutputStream.  By wrapping the initial java.io.FileOutputStream 
with a java.io.BufferedOutputStream, FOP is not able to potentially make use of the nio FileChannel 
stuff which could provide more efficient final output writing (using modern operating system 
caching) [1].

This would especially be the case for AFP writing as output is initially written in two parts for 
memory saving reasons, one outputstream for document resources and one for the actual document.  On 
completion of rendering the document is appended to the end of the document resources outputstream.

So I am proposing the introduction and instantiation of a FileChannelAccessibleBufferedOutputStream 
which extends java.io.BufferedOutputStream in Main.startFOP().  It will expose the getChannel() 
method of java.io.FileOutputStream.  I have included the small code changes I am proposing below. 
Does this sound reasonable and would this cause any problems for anyone?

Adrian.

[1] http://java.sun.com/j2se/1.4.2/docs/guide/nio

Index: java/org/apache/fop/cli/Main.java
===================================================================
--- java/org/apache/fop/cli/Main.java   (revision 708875)
+++ java/org/apache/fop/cli/Main.java   (working copy)
@@ -21,6 +21,7 @@

  import java.io.File;
  import java.io.FileFilter;
+import java.io.FileOutputStream;
  import java.io.OutputStream;
  import java.lang.reflect.Method;
  import java.net.MalformedURLException;
@@ -28,9 +29,9 @@
  import java.util.List;

  import org.apache.commons.io.IOUtils;
-
  import org.apache.fop.apps.FOUserAgent;
  import org.apache.fop.apps.MimeConstants;
+import org.apache.fop.util.FileChannelAccessibleBufferedOutputStream;

  /**
   * Main command-line class for Apache FOP.
@@ -164,8 +165,9 @@

              try {
                  if (options.getOutputFile() != null) {
-                    out = new java.io.BufferedOutputStream(
-                            new java.io.FileOutputStream(options.getOutputFile()));
+                    FileOutputStream fileOutputStream
+                        = new java.io.FileOutputStream(options.getOutputFile());
+                    out = new FileChannelAccessibleBufferedOutputStream(fileOutputStream);
                      foUserAgent.setOutputFile(options.getOutputFile());
                  } else if (options.isOutputToStdOut()) {
                      out = new java.io.BufferedOutputStream(System.out);

--- snip ---

/*
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
  * this work for additional information regarding copyright ownership.
  * The ASF licenses this file to You under the Apache License, Version 2.0
  * (the "License"); you may not use this file except in compliance with
  * the License.  You may obtain a copy of the License at
  *
  *      http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */

/* $Id: $ */
package org.apache.fop.util;

import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.channels.FileChannel;

/**
  * A BufferedOutputStream implementation that provides access to the underlying FileChannel.
  * This allows for the potential use of nio FileChannel.transferTo() and FileChannel.transferFrom().
  */
public class FileChannelAccessibleBufferedOutputStream extends java.io.BufferedOutputStream {

     private final FileOutputStream fos;

     /**
      * Main constructor
      *
      * @param fos file output stream
      */
     public FileChannelAccessibleBufferedOutputStream(FileOutputStream fos) {
         super(fos);
         this.fos = fos;
     }

     /**
      * Returns the file channel
      *
      * @return the file channel
      * @throws IOException thrown if an I/O exception of some sort has occurred
      */
     public FileChannel getChannel() throws IOException {
         flush(); // write out any buffered data
         return fos.getChannel();
     }
}



Re: FIleChannel accessible BufferedOutputStream

Posted by Adrian Cumiskey <de...@cumiskey.com>.
Jeremias,

To be honest I was just guessing and hoping it would be better, I would have expected some 
improvement..   but alas I've just done some testing and its actually slower..  so I think I'll 
scrap this idea and we'll stick with plain old BufferedOutputStream.  Thanks for the feedback and 
the link too :).

Adrian.

Jeremias Maerki wrote:
> So what will happen if there's no FileChannel available (for example, in
> a servlet)? Will the AFP output stop working?
> 
> In additional to what Max wrote (preserving compatibility with output
> targets that are not nio-compatible), I'd say that bit of potential
> performance improvement is totally irrelevant in the command-line after
> all the class loading and JIT. It might help in a server-environment,
> though. But did you make any measurements that indicate that the
> performance would indeed be considerably better? Or are you just
> guessing/hoping?
> 
> http://www.cs.cmu.edu/~jch/java/rules.html
> Steve McConnell's "Code Complete" comes to mind, too.
> 
> 
> On 10.11.2008 13:13:03 Adrian Cumiskey wrote:
>> Max,
>>
>> I understand what you are saying, but I am trying not to change too much code or FOP's API with this 
>> change.  Currently the BufferedOutputStream is passed as an OutputStream reference all the way down 
>> the calling chain during rendering.
>>
>> So although your suggestion would result in a much neater result, changing the OutputStream 
>> references to some sort of custom OutputSource would be quite a big change to the FOP API and may 
>> break existing FOP integrations.  This is why I was just proposing the subclassing 
>> BufferedOutputStream to expose the getChannel() method for the cases where new I/O would be useful.
>>
>> Adrian.
>>
>> Max Berger wrote:
>>> Adrian,
>>>
>>> when doing this you need to be very careful that either the
>>> BufferedOutputStream OR the FileChannel methods are used. If you start
>>> mixing both you will get very strange effects.  So for safety I would
>>> prefer to pass either a stream or a channel to a renderer, but not both.
>>>
>>> Also, you need to make sure that the AFP renderer work if just a Stream
>>> is available - this could be needed in cases where the output is stdout
>>> (although I don't know if this would be a common use case).
>>>
>>> Maybe a wrapper class similar to the idea of org.xml.sax.InputSource
>>> would do the trick?
>>>
>>> Max
>>>
>>> Adrian Cumiskey schrieb:
>>>> When FOP is started in Main.startFOP() a java.io.BufferedOutputStream is
>>>> instantiated and wrapped around an instantiated
>>>> java.io.FileOutputStream.  By wrapping the initial
>>>> java.io.FileOutputStream with a java.io.BufferedOutputStream, FOP is not
>>>> able to potentially make use of the nio FileChannel stuff which could
>>>> provide more efficient final output writing (using modern operating
>>>> system caching) [1].
>>>>
>>>> This would especially be the case for AFP writing as output is initially
>>>> written in two parts for memory saving reasons, one outputstream for
>>>> document resources and one for the actual document.  On completion of
>>>> rendering the document is appended to the end of the document resources
>>>> outputstream.
>>>>
>>>> So I am proposing the introduction and instantiation of a
>>>> FileChannelAccessibleBufferedOutputStream which extends
>>>> java.io.BufferedOutputStream in Main.startFOP().  It will expose the
>>>> getChannel() method of java.io.FileOutputStream.  I have included the
>>>> small code changes I am proposing below. Does this sound reasonable and
>>>> would this cause any problems for anyone?
>>>>
>>>> Adrian.
>>>>
>>>> [1] http://java.sun.com/j2se/1.4.2/docs/guide/nio
> 
> <snip/>
> 
> 
> Jeremias Maerki
> 
> 


Re: FIleChannel accessible BufferedOutputStream

Posted by Adrian Cumiskey <de...@cumiskey.com>.
Jeremias Maerki wrote:
> So what will happen if there's no FileChannel available (for example, in
> a servlet)? Will the AFP output stop working?

Of course there would have been provision for plain old OutputStream handling, this really goes 
without saying doesn't it?

> In additional to what Max wrote (preserving compatibility with output
> targets that are not nio-compatible), I'd say that bit of potential
> performance improvement is totally irrelevant in the command-line after
> all the class loading and JIT. It might help in a server-environment,
> though. But did you make any measurements that indicate that the
> performance would indeed be considerably better? Or are you just
> guessing/hoping?

I was just plugging it in and testing it out on the command line, of course I appreciate there are 
other deployment environments...  I was just looking for an improvement in the raw file writing 
speed.  Anyway the new I/O offered no performance improvements so its all academic.

Adrian.

Re: FIleChannel accessible BufferedOutputStream

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
So what will happen if there's no FileChannel available (for example, in
a servlet)? Will the AFP output stop working?

In additional to what Max wrote (preserving compatibility with output
targets that are not nio-compatible), I'd say that bit of potential
performance improvement is totally irrelevant in the command-line after
all the class loading and JIT. It might help in a server-environment,
though. But did you make any measurements that indicate that the
performance would indeed be considerably better? Or are you just
guessing/hoping?

http://www.cs.cmu.edu/~jch/java/rules.html
Steve McConnell's "Code Complete" comes to mind, too.


On 10.11.2008 13:13:03 Adrian Cumiskey wrote:
> Max,
> 
> I understand what you are saying, but I am trying not to change too much code or FOP's API with this 
> change.  Currently the BufferedOutputStream is passed as an OutputStream reference all the way down 
> the calling chain during rendering.
> 
> So although your suggestion would result in a much neater result, changing the OutputStream 
> references to some sort of custom OutputSource would be quite a big change to the FOP API and may 
> break existing FOP integrations.  This is why I was just proposing the subclassing 
> BufferedOutputStream to expose the getChannel() method for the cases where new I/O would be useful.
> 
> Adrian.
> 
> Max Berger wrote:
> > 
> > Adrian,
> > 
> > when doing this you need to be very careful that either the
> > BufferedOutputStream OR the FileChannel methods are used. If you start
> > mixing both you will get very strange effects.  So for safety I would
> > prefer to pass either a stream or a channel to a renderer, but not both.
> > 
> > Also, you need to make sure that the AFP renderer work if just a Stream
> > is available - this could be needed in cases where the output is stdout
> > (although I don't know if this would be a common use case).
> > 
> > Maybe a wrapper class similar to the idea of org.xml.sax.InputSource
> > would do the trick?
> > 
> > Max
> > 
> > Adrian Cumiskey schrieb:
> >> When FOP is started in Main.startFOP() a java.io.BufferedOutputStream is
> >> instantiated and wrapped around an instantiated
> >> java.io.FileOutputStream.  By wrapping the initial
> >> java.io.FileOutputStream with a java.io.BufferedOutputStream, FOP is not
> >> able to potentially make use of the nio FileChannel stuff which could
> >> provide more efficient final output writing (using modern operating
> >> system caching) [1].
> >>
> >> This would especially be the case for AFP writing as output is initially
> >> written in two parts for memory saving reasons, one outputstream for
> >> document resources and one for the actual document.  On completion of
> >> rendering the document is appended to the end of the document resources
> >> outputstream.
> >>
> >> So I am proposing the introduction and instantiation of a
> >> FileChannelAccessibleBufferedOutputStream which extends
> >> java.io.BufferedOutputStream in Main.startFOP().  It will expose the
> >> getChannel() method of java.io.FileOutputStream.  I have included the
> >> small code changes I am proposing below. Does this sound reasonable and
> >> would this cause any problems for anyone?
> >>
> >> Adrian.
> >>
> >> [1] http://java.sun.com/j2se/1.4.2/docs/guide/nio

<snip/>


Jeremias Maerki


Re: FIleChannel accessible BufferedOutputStream

Posted by Adrian Cumiskey <de...@cumiskey.com>.
Max,

I understand what you are saying, but I am trying not to change too much code or FOP's API with this 
change.  Currently the BufferedOutputStream is passed as an OutputStream reference all the way down 
the calling chain during rendering.

So although your suggestion would result in a much neater result, changing the OutputStream 
references to some sort of custom OutputSource would be quite a big change to the FOP API and may 
break existing FOP integrations.  This is why I was just proposing the subclassing 
BufferedOutputStream to expose the getChannel() method for the cases where new I/O would be useful.

Adrian.

Max Berger wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Adrian,
> 
> when doing this you need to be very careful that either the
> BufferedOutputStream OR the FileChannel methods are used. If you start
> mixing both you will get very strange effects.  So for safety I would
> prefer to pass either a stream or a channel to a renderer, but not both.
> 
> Also, you need to make sure that the AFP renderer work if just a Stream
> is available - this could be needed in cases where the output is stdout
> (although I don't know if this would be a common use case).
> 
> Maybe a wrapper class similar to the idea of org.xml.sax.InputSource
> would do the trick?
> 
> Max
> 
> Adrian Cumiskey schrieb:
>> When FOP is started in Main.startFOP() a java.io.BufferedOutputStream is
>> instantiated and wrapped around an instantiated
>> java.io.FileOutputStream.  By wrapping the initial
>> java.io.FileOutputStream with a java.io.BufferedOutputStream, FOP is not
>> able to potentially make use of the nio FileChannel stuff which could
>> provide more efficient final output writing (using modern operating
>> system caching) [1].
>>
>> This would especially be the case for AFP writing as output is initially
>> written in two parts for memory saving reasons, one outputstream for
>> document resources and one for the actual document.  On completion of
>> rendering the document is appended to the end of the document resources
>> outputstream.
>>
>> So I am proposing the introduction and instantiation of a
>> FileChannelAccessibleBufferedOutputStream which extends
>> java.io.BufferedOutputStream in Main.startFOP().  It will expose the
>> getChannel() method of java.io.FileOutputStream.  I have included the
>> small code changes I am proposing below. Does this sound reasonable and
>> would this cause any problems for anyone?
>>
>> Adrian.
>>
>> [1] http://java.sun.com/j2se/1.4.2/docs/guide/nio
>>
>> Index: java/org/apache/fop/cli/Main.java
>> ===================================================================
>> --- java/org/apache/fop/cli/Main.java   (revision 708875)
>> +++ java/org/apache/fop/cli/Main.java   (working copy)
>> @@ -21,6 +21,7 @@
>>
>>  import java.io.File;
>>  import java.io.FileFilter;
>> +import java.io.FileOutputStream;
>>  import java.io.OutputStream;
>>  import java.lang.reflect.Method;
>>  import java.net.MalformedURLException;
>> @@ -28,9 +29,9 @@
>>  import java.util.List;
>>
>>  import org.apache.commons.io.IOUtils;
>> -
>>  import org.apache.fop.apps.FOUserAgent;
>>  import org.apache.fop.apps.MimeConstants;
>> +import org.apache.fop.util.FileChannelAccessibleBufferedOutputStream;
>>
>>  /**
>>   * Main command-line class for Apache FOP.
>> @@ -164,8 +165,9 @@
>>
>>              try {
>>                  if (options.getOutputFile() != null) {
>> -                    out = new java.io.BufferedOutputStream(
>> -                            new
>> java.io.FileOutputStream(options.getOutputFile()));
>> +                    FileOutputStream fileOutputStream
>> +                        = new
>> java.io.FileOutputStream(options.getOutputFile());
>> +                    out = new
>> FileChannelAccessibleBufferedOutputStream(fileOutputStream);
>>                      foUserAgent.setOutputFile(options.getOutputFile());
>>                  } else if (options.isOutputToStdOut()) {
>>                      out = new java.io.BufferedOutputStream(System.out);
>>
>> --- snip ---
>>
>> /*
>>  * Licensed to the Apache Software Foundation (ASF) under one or more
>>  * contributor license agreements.  See the NOTICE file distributed with
>>  * this work for additional information regarding copyright ownership.
>>  * The ASF licenses this file to You under the Apache License, Version 2.0
>>  * (the "License"); you may not use this file except in compliance with
>>  * the License.  You may obtain a copy of the License at
>>  *
>>  *      http://www.apache.org/licenses/LICENSE-2.0
>>  *
>>  * Unless required by applicable law or agreed to in writing, software
>>  * distributed under the License is distributed on an "AS IS" BASIS,
>>  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>>  * See the License for the specific language governing permissions and
>>  * limitations under the License.
>>  */
>>
>> /* $Id: $ */
>> package org.apache.fop.util;
>>
>> import java.io.FileOutputStream;
>> import java.io.IOException;
>> import java.nio.channels.FileChannel;
>>
>> /**
>>  * A BufferedOutputStream implementation that provides access to the
>> underlying FileChannel.
>>  * This allows for the potential use of nio FileChannel.transferTo() and
>> FileChannel.transferFrom().
>>  */
>> public class FileChannelAccessibleBufferedOutputStream extends
>> java.io.BufferedOutputStream {
>>
>>     private final FileOutputStream fos;
>>
>>     /**
>>      * Main constructor
>>      *
>>      * @param fos file output stream
>>      */
>>     public FileChannelAccessibleBufferedOutputStream(FileOutputStream
>> fos) {
>>         super(fos);
>>         this.fos = fos;
>>     }
>>
>>     /**
>>      * Returns the file channel
>>      *
>>      * @return the file channel
>>      * @throws IOException thrown if an I/O exception of some sort has
>> occurred
>>      */
>>     public FileChannel getChannel() throws IOException {
>>         flush(); // write out any buffered data
>>         return fos.getChannel();
>>     }
>> }
>>
>>
> 
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
> Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
> 
> iEYEARECAAYFAkkYIYQACgkQjh41xmdUNRvO8gCgmEO2G8ojHuvsuy3d29/kmzp3
> 8GcAn0NT2kGuIzz8hQkhOE48nhw55g6P
> =dIBF
> -----END PGP SIGNATURE-----
> 


Re: FIleChannel accessible BufferedOutputStream

Posted by Max Berger <ma...@berger.name>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Adrian,

when doing this you need to be very careful that either the
BufferedOutputStream OR the FileChannel methods are used. If you start
mixing both you will get very strange effects.  So for safety I would
prefer to pass either a stream or a channel to a renderer, but not both.

Also, you need to make sure that the AFP renderer work if just a Stream
is available - this could be needed in cases where the output is stdout
(although I don't know if this would be a common use case).

Maybe a wrapper class similar to the idea of org.xml.sax.InputSource
would do the trick?

Max

Adrian Cumiskey schrieb:
> When FOP is started in Main.startFOP() a java.io.BufferedOutputStream is
> instantiated and wrapped around an instantiated
> java.io.FileOutputStream.  By wrapping the initial
> java.io.FileOutputStream with a java.io.BufferedOutputStream, FOP is not
> able to potentially make use of the nio FileChannel stuff which could
> provide more efficient final output writing (using modern operating
> system caching) [1].
> 
> This would especially be the case for AFP writing as output is initially
> written in two parts for memory saving reasons, one outputstream for
> document resources and one for the actual document.  On completion of
> rendering the document is appended to the end of the document resources
> outputstream.
> 
> So I am proposing the introduction and instantiation of a
> FileChannelAccessibleBufferedOutputStream which extends
> java.io.BufferedOutputStream in Main.startFOP().  It will expose the
> getChannel() method of java.io.FileOutputStream.  I have included the
> small code changes I am proposing below. Does this sound reasonable and
> would this cause any problems for anyone?
> 
> Adrian.
> 
> [1] http://java.sun.com/j2se/1.4.2/docs/guide/nio
> 
> Index: java/org/apache/fop/cli/Main.java
> ===================================================================
> --- java/org/apache/fop/cli/Main.java   (revision 708875)
> +++ java/org/apache/fop/cli/Main.java   (working copy)
> @@ -21,6 +21,7 @@
> 
>  import java.io.File;
>  import java.io.FileFilter;
> +import java.io.FileOutputStream;
>  import java.io.OutputStream;
>  import java.lang.reflect.Method;
>  import java.net.MalformedURLException;
> @@ -28,9 +29,9 @@
>  import java.util.List;
> 
>  import org.apache.commons.io.IOUtils;
> -
>  import org.apache.fop.apps.FOUserAgent;
>  import org.apache.fop.apps.MimeConstants;
> +import org.apache.fop.util.FileChannelAccessibleBufferedOutputStream;
> 
>  /**
>   * Main command-line class for Apache FOP.
> @@ -164,8 +165,9 @@
> 
>              try {
>                  if (options.getOutputFile() != null) {
> -                    out = new java.io.BufferedOutputStream(
> -                            new
> java.io.FileOutputStream(options.getOutputFile()));
> +                    FileOutputStream fileOutputStream
> +                        = new
> java.io.FileOutputStream(options.getOutputFile());
> +                    out = new
> FileChannelAccessibleBufferedOutputStream(fileOutputStream);
>                      foUserAgent.setOutputFile(options.getOutputFile());
>                  } else if (options.isOutputToStdOut()) {
>                      out = new java.io.BufferedOutputStream(System.out);
> 
> --- snip ---
> 
> /*
>  * Licensed to the Apache Software Foundation (ASF) under one or more
>  * contributor license agreements.  See the NOTICE file distributed with
>  * this work for additional information regarding copyright ownership.
>  * The ASF licenses this file to You under the Apache License, Version 2.0
>  * (the "License"); you may not use this file except in compliance with
>  * the License.  You may obtain a copy of the License at
>  *
>  *      http://www.apache.org/licenses/LICENSE-2.0
>  *
>  * Unless required by applicable law or agreed to in writing, software
>  * distributed under the License is distributed on an "AS IS" BASIS,
>  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>  * See the License for the specific language governing permissions and
>  * limitations under the License.
>  */
> 
> /* $Id: $ */
> package org.apache.fop.util;
> 
> import java.io.FileOutputStream;
> import java.io.IOException;
> import java.nio.channels.FileChannel;
> 
> /**
>  * A BufferedOutputStream implementation that provides access to the
> underlying FileChannel.
>  * This allows for the potential use of nio FileChannel.transferTo() and
> FileChannel.transferFrom().
>  */
> public class FileChannelAccessibleBufferedOutputStream extends
> java.io.BufferedOutputStream {
> 
>     private final FileOutputStream fos;
> 
>     /**
>      * Main constructor
>      *
>      * @param fos file output stream
>      */
>     public FileChannelAccessibleBufferedOutputStream(FileOutputStream
> fos) {
>         super(fos);
>         this.fos = fos;
>     }
> 
>     /**
>      * Returns the file channel
>      *
>      * @return the file channel
>      * @throws IOException thrown if an I/O exception of some sort has
> occurred
>      */
>     public FileChannel getChannel() throws IOException {
>         flush(); // write out any buffered data
>         return fos.getChannel();
>     }
> }
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkkYIYQACgkQjh41xmdUNRvO8gCgmEO2G8ojHuvsuy3d29/kmzp3
8GcAn0NT2kGuIzz8hQkhOE48nhw55g6P
=dIBF
-----END PGP SIGNATURE-----