You are viewing a plain text version of this content. The canonical link for it is here.
Posted to rpc-dev@xml.apache.org by Daniel Rall <dl...@finemaltcoding.com> on 2002/02/13 01:36:36 UTC

XmlRpcServer found un-streamy

When the XmlRpcServer class responds to requests, all the data in the
response is first buffered in memory in whatever form is reasonable
for output (such as a Hashtable), then written to a StringBuffer, then
converted to a String, then a converted to byte[].  That means the
same data is buffered in memory (in various forms) a minimum of 4
times (!!).  If the response contains large quantities of data,
imagine the repurcussions...

The current performance for large data sets is just not sufficient.  I
would like to make the API more "streamy".  Velocity's template
processing API
<http://jakarta.apache.org/velocity/api/org/apache/velocity/app/Velocity.html#mergeTemplate(java.lang.String,%20java.lang.String,%20org.apache.velocity.context.Context,%20java.io.Writer)>
is a good example of how I would like the XML-RPC server to act.

Thoughts, comments?

                             Thanks, Dan

Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Daniel Rall" <dl...@finemaltcoding.com>
To: <rp...@xml.apache.org>
Sent: Monday, February 18, 2002 11:10 PM
Subject: Re: XmlRpcServer found un-streamy


[snip]

> > 1. It might be reasonable to perform two passes over the result
> > object, one to compute the content length, and then one to stream it
> > out.
>
> The value of this change would depend heavily on how long it takes to
> create the response, and just doesn't seem very elegant (though
> neither does a frozen XML-RPC spec, bleh).

One solution I have considered in MinML-RPC (which generally runs in *very*
memory constrained environments) is to have a fixed buffer. The result is
written to the buffer. If it overflows then we continue just counting the
byes and do a second pass to send the data. If it fits in the buffer then we
just do one pass. This makes the small result case (which is the most likely
case in an embedded system) quite quick and means the large result case
works slowly as opposed to not working at all.

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Daniel Rall" <dl...@finemaltcoding.com>
To: <rp...@xml.apache.org>
Sent: Monday, February 18, 2002 11:10 PM
Subject: Re: XmlRpcServer found un-streamy


[snip]

> > 1. It might be reasonable to perform two passes over the result
> > object, one to compute the content length, and then one to stream it
> > out.
>
> The value of this change would depend heavily on how long it takes to
> create the response, and just doesn't seem very elegant (though
> neither does a frozen XML-RPC spec, bleh).

One solution I have considered in MinML-RPC (which generally runs in *very*
memory constrained environments) is to have a fixed buffer. The result is
written to the buffer. If it overflows then we continue just counting the
byes and do a second pass to send the data. If it fits in the buffer then we
just do one pass. This makes the small result case (which is the most likely
case in an embedded system) quite quick and means the large result case
works slowly as opposed to not working at all.

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by Daniel Rall <dl...@finemaltcoding.com>.
Timothy Peierls <ti...@peierls.net> writes:

> Daniel Rall wrote:
> >> That means the same data is buffered in memory (in various 
>>> forms) a minimum of 4 times (!!). If the response contains 
>>> large quantities of data, imagine the repercussions...
>
> I've observed significant overhead when returning large result
> objects. It used to be worse, before the intermediate values were
> cleared in XmlRpc.Worker, but it's still wasteful.
>
> So far, the response to this has centered on how the possibilities for
> streaming are limited by the need to produce the response body in
> memory before writing the Content-length header. However, there are
> other ways things could be improved.

Tim, thanks a lot for your comments.

> 1. It might be reasonable to perform two passes over the result
> object, one to compute the content length, and then one to stream it
> out.

The value of this change would depend heavily on how long it takes to
create the response, and just doesn't seem very elegant (though
neither does a frozen XML-RPC spec, bleh).

> 2. In any event, why not have XmlWriter write directly to an
> OutputStreamWriter that wraps a ByteArrayOutputStream (maybe itself
> wrapped in a BufferedWriter)? This would eliminate the need for a
> StringBuffer, which is one of the problem areas that Dan is worried
> about.

I was thinking along the same lines, but your description really
helped crystalize my thoughts.  I made XmlWriter sub-class the JDK's
OutputStreamWriter (for simplicity), and moved writeObject() into it.
I am about to commit to CVS, but have attached the patch to the end of
this message for comment and review.  As this is a fairly extensive
change, it would be great to get some eyes on it.

> 3. And, at the risk of being a one-note Johnny, I'll point out that
> were we to use List and Map instead of Vector and Hashtable, server
> side code that, for example, currently constructs a Vector in memory
> just to hold the results of iterating over some internal data
> structure might instead be able to use a custom List implementation
> that would effectively stream those results directly from the List's
> Iterator without having to form an intermediate array or Vector.

Tim, I would love to see some code implementing this idea!


All, please comment on these changes:

Index: src/java/org/apache/xmlrpc/XmlRpc.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpc.java,v
retrieving revision 1.16
diff -u -r1.16 XmlRpc.java
--- src/java/org/apache/xmlrpc/XmlRpc.java	18 Feb 2002 17:06:15 -0000	1.16
+++ src/java/org/apache/xmlrpc/XmlRpc.java	18 Feb 2002 22:44:16 -0000
@@ -72,6 +72,7 @@
  * @see XmlRpcClient
  *
  * @author <a href="mailto:hannes@apache.org">Hannes Wallnoefer</a>
+ * @author <a href="mailto:dlr@finemaltcoding.com">Daniel Rall</a>
  */
 public abstract class XmlRpc 
     extends HandlerBase
@@ -331,89 +332,6 @@
     }
 
     /**
-      * Writes the XML representation of a supported Java object to
-      * the XML writer.
-      */
-    void writeObject (Object what, XmlWriter writer)
-    {
-        writer.startElement ("value");
-        if (what == null)
-        {
-            throw new RuntimeException ("null value not supported by XML-RPC");
-        }
-        else if (what instanceof String)
-        {
-            writer.chardata (what.toString ());
-        }
-        else if (what instanceof Integer)
-        {
-            writer.startElement ("int");
-            writer.write (what.toString ());
-            writer.endElement ("int");
-        }
-        else if (what instanceof Boolean)
-        {
-            writer.startElement ("boolean");
-            writer.write (((Boolean) what).booleanValue () ? "1" : "0");
-            writer.endElement ("boolean");
-        }
-        else if (what instanceof Double || what instanceof Float)
-        {
-            writer.startElement ("double");
-            writer.write (what.toString ());
-            writer.endElement ("double");
-        }
-        else if (what instanceof Date)
-        {
-            writer.startElement ("dateTime.iso8601");
-            Date d = (Date) what;
-            writer.write (dateformat.format (d));
-            writer.endElement ("dateTime.iso8601");
-        }
-        else if (what instanceof byte[])
-        {
-            writer.startElement ("base64");
-            writer.write (Base64.encode ((byte[]) what));
-            writer.endElement ("base64");
-        }
-        else if (what instanceof Vector)
-        {
-            writer.startElement ("array");
-            writer.startElement ("data");
-            Vector v = (Vector) what;
-            int l2 = v.size ();
-            for (int i2 = 0; i2 < l2; i2++)
-                writeObject (v.elementAt (i2), writer);
-            writer.endElement ("data");
-            writer.endElement ("array");
-        }
-        else if (what instanceof Hashtable)
-        {
-            writer.startElement ("struct");
-            Hashtable h = (Hashtable) what;
-            for (Enumeration e = h.keys (); e.hasMoreElements ();)
-            {
-                String nextkey = (String) e.nextElement ();
-                Object nextval = h.get (nextkey);
-                writer.startElement ("member");
-                writer.startElement ("name");
-                writer.write (nextkey);
-                writer.endElement ("name");
-                writeObject (nextval, writer);
-                writer.endElement ("member");
-            }
-            writer.endElement ("struct");
-        }
-        else
-        {
-            throw new RuntimeException ("unsupported Java type: " +
-                    what.getClass ());
-        }
-        writer.endElement ("value");
-    }
-
-
-    /**
       *  This method is called when a root level object has been parsed.
       */
     abstract void objectParsed (Object what);
@@ -700,9 +618,14 @@
 
 
     /**
-     * A quick and dirty XML writer.
+     * A quick and dirty XML writer.  If you feed it a
+     * <code>ByteArrayInputStream</code>, it may be necessary to call
+     * <code>writer.flush()</code> before calling
+     * <code>buffer.toByteArray()</code> to get the data written to
+     * your byte buffer.
      */
     class XmlWriter
+        extends OutputStreamWriter
     {
         protected static final String PROLOG_START =
             "<?xml version=\"1.0\" encoding=\"";
@@ -713,55 +636,137 @@
         protected static final String GREATER_THAN_ENTITY = "&gt;";
         protected static final String AMPERSAND_ENTITY = "&amp;";
 
-        /**
-         * The buffer to write to.
-         */
-        StringBuffer buf;
-
-        /**
-         * The encoding to use.
-         */
-        String enc;
-
-        public XmlWriter (StringBuffer buf)
+        public XmlWriter (OutputStream out)
+            throws UnsupportedEncodingException, IOException
         {
             // The default encoding used for XML-RPC is ISO-8859-1.
-            this (buf, encoding);
+            this (out, encoding);
         }
 
-        public XmlWriter (StringBuffer buf, String enc)
+        public XmlWriter (OutputStream out, String enc)
+            throws UnsupportedEncodingException, IOException
         {
-            this.buf = buf;
-            this.enc = enc;
+            super(out, enc);
 
             // Add the XML prolog (which includes the encoding)
-            buf.append (PROLOG_START);
-            buf.append (encodings.getProperty (enc, enc));
-            buf.append (PROLOG_END);
+            write(PROLOG_START);
+            write(encodings.getProperty(enc, enc));
+            write(PROLOG_END);
         }
 
-        public void startElement (String elem)
+        /**
+         * Writes the XML representation of a supported Java object
+         * type.
+         *
+         * @param obj The <code>Object</code> to write.
+         */
+        public void writeObject (Object obj)
+            throws IOException
         {
-            buf.append ('<');
-            buf.append (elem);
-            buf.append ('>');
+            startElement ("value");
+            if (obj == null)
+            {
+                throw new RuntimeException("null value not supported by XML-RPC");
+            }
+            else if (obj instanceof String)
+            {
+                chardata(obj.toString());
+            }
+            else if (obj instanceof Integer)
+            {
+                startElement("int");
+                write(obj.toString());
+                endElement("int");
+            }
+            else if (obj instanceof Boolean)
+            {
+                startElement("boolean");
+                write(((Boolean) obj).booleanValue() ? "1" : "0");
+                endElement("boolean");
+            }
+            else if (obj instanceof Double || obj instanceof Float)
+            {
+                startElement("double");
+                write(obj.toString());
+                endElement("double");
+            }
+            else if (obj instanceof Date)
+            {
+                startElement("dateTime.iso8601");
+                Date d = (Date) obj;
+                write(dateformat.format(d));
+                endElement("dateTime.iso8601");
+            }
+            else if (obj instanceof byte[])
+            {
+                startElement("base64");
+                write(Base64.encode((byte[]) obj));
+                endElement("base64");
+            }
+            else if (obj instanceof Vector)
+            {
+                startElement("array");
+                startElement("data");
+                Vector array = (Vector) obj;
+                int size = array.size();
+                for (int i = 0; i < size; i++)
+                {
+                    writeObject(array.elementAt(i));
+                }
+                endElement("data");
+                endElement("array");
+            }
+            else if (obj instanceof Hashtable)
+            {
+                startElement("struct");
+                Hashtable struct = (Hashtable) obj;
+                for (Enumeration e = struct.keys(); e.hasMoreElements(); )
+                {
+                    String nextkey = (String) e.nextElement();
+                    Object nextval = struct.get(nextkey);
+                    startElement("member");
+                    startElement("name");
+                    write(nextkey);
+                    endElement("name");
+                    writeObject(nextval);
+                    endElement("member");
+                }
+                endElement("struct");
+            }
+            else
+            {
+                throw new RuntimeException("unsupported Java type: " +
+                                            obj.getClass());
+            }
+            endElement("value");
+        }
+
+        protected void startElement (String elem)
+            throws IOException
+        {
+            write('<');
+            write(elem);
+            write('>');
         }
 
-        public void endElement (String elem)
+        protected void endElement (String elem)
+            throws IOException
         {
-            buf.append (CLOSING_TAG_START);
-            buf.append (elem);
-            buf.append ('>');
+            write(CLOSING_TAG_START);
+            write(elem);
+            write('>');
         }
 
-        public void emptyElement (String elem)
+        protected void emptyElement (String elem)
+            throws IOException
         {
-            buf.append ('<');
-            buf.append (elem);
-            buf.append (SINGLE_TAG_END);
+            write('<');
+            write(elem);
+            write(SINGLE_TAG_END);
         }
 
-        public void chardata (String text)
+        protected void chardata (String text)
+            throws IOException
         {
             int l = text.length ();
             for (int i = 0; i < l; i++)
@@ -769,39 +774,19 @@
                 char c = text.charAt (i);
                 switch (c)
                 {
-                case '<' :
-                    buf.append (LESS_THAN_ENTITY);
+                case '<':
+                    write(LESS_THAN_ENTITY);
                     break;
-                case '>' :
-                    buf.append (GREATER_THAN_ENTITY);
+                case '>':
+                    write(GREATER_THAN_ENTITY);
                     break;
-                case '&' :
-                    buf.append (AMPERSAND_ENTITY);
+                case '&':
+                    write(AMPERSAND_ENTITY);
                     break;
-                default :
-                    buf.append (c);
+                default:
+                    write(c);
                 }
             }
-        }
-
-        public void write (char[] text)
-        {
-            buf.append (text);
-        }
-
-        public void write (String text)
-        {
-            buf.append (text);
-        }
-
-        public String toString ()
-        {
-            return buf.toString ();
-        }
-
-        public byte[] getBytes () throws UnsupportedEncodingException
-        {
-            return buf.toString ().getBytes (enc);
         }
     }
 }
Index: src/java/org/apache/xmlrpc/XmlRpcClient.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcClient.java,v
retrieving revision 1.5
diff -u -r1.5 XmlRpcClient.java
--- src/java/org/apache/xmlrpc/XmlRpcClient.java	18 Feb 2002 17:06:15 -0000	1.5
+++ src/java/org/apache/xmlrpc/XmlRpcClient.java	18 Feb 2002 22:44:17 -0000
@@ -254,7 +254,11 @@
     {
         boolean fault;
         Object result = null;
-        StringBuffer strbuf;
+
+        /**
+         * The output buffer used in creating a request.
+         */
+        ByteArrayOutputStream buffer;
 
         CallData call;
 
@@ -298,12 +302,15 @@
             catch (Exception x)
             {
                 if (callback != null)
+                {
                     try
                     {
                         callback.handleError (x, url, method);
                     }
                     catch (Exception ignore)
-                    {}
+                    {
+                    }
+                }
             }
         }
 
@@ -323,14 +330,19 @@
             {
                 ByteArrayOutputStream bout = new ByteArrayOutputStream ();
 
-                if (strbuf == null)
-                    strbuf = new StringBuffer ();
+                if (buffer == null)
+                {
+                    buffer = new ByteArrayOutputStream();
+                }
                 else
-                    strbuf.setLength (0);
+                {
+                    buffer.reset();
+                }
 
-                XmlWriter writer = new XmlWriter (strbuf);
+                XmlWriter writer = new XmlWriter (buffer);
                 writeRequest (writer, method, params);
-                byte[] request = writer.getBytes();
+                writer.flush();
+                byte[] request = buffer.toByteArray();
 
                 URLConnection con = url.openConnection ();
                 con.setDoInput (true);
@@ -407,7 +419,7 @@
             for (int i = 0; i < l; i++)
             {
                 writer.startElement ("param");
-                writeObject (params.elementAt (i), writer);
+                writer.writeObject (params.elementAt (i));
                 writer.endElement ("param");
             }
             writer.endElement ("params");
Index: src/java/org/apache/xmlrpc/XmlRpcClientLite.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcClientLite.java,v
retrieving revision 1.2
diff -u -r1.2 XmlRpcClientLite.java
--- src/java/org/apache/xmlrpc/XmlRpcClientLite.java	18 Feb 2002 17:06:15 -0000	1.2
+++ src/java/org/apache/xmlrpc/XmlRpcClientLite.java	18 Feb 2002 22:44:18 -0000
@@ -126,7 +126,6 @@
 
     class LiteWorker extends Worker implements Runnable
     {
-
         HttpClient client = null;
 
         public LiteWorker ()
@@ -134,21 +133,25 @@
             super ();
         }
 
-
-        Object execute (String method,
-                Vector params) throws XmlRpcException, IOException
+        Object execute (String method, Vector params)
+            throws XmlRpcException, IOException
         {
             long now = System.currentTimeMillis ();
             fault = false;
             try
             {
-                if (strbuf == null)
-                    strbuf = new StringBuffer ();
+                if (buffer == null)
+                {
+                    buffer = new ByteArrayOutputStream();
+                }
                 else
-                    strbuf.setLength (0);
-                XmlWriter writer = new XmlWriter (strbuf);
+                {
+                    buffer.reset();
+                }
+                XmlWriter writer = new XmlWriter (buffer);
                 writeRequest (writer, method, params);
-                byte[] request = writer.getBytes();
+                writer.flush();
+                byte[] request = buffer.toByteArray();
 
                 // and send it to the server
                 if (client == null)
Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
retrieving revision 1.10
diff -u -r1.10 XmlRpcServer.java
--- src/java/org/apache/xmlrpc/XmlRpcServer.java	18 Feb 2002 17:06:15 -0000	1.10
+++ src/java/org/apache/xmlrpc/XmlRpcServer.java	18 Feb 2002 22:44:19 -0000
@@ -66,6 +66,7 @@
  * HTTP listener, use the WebServer class instead.
  *
  * @author <a href="mailto:hannes@apache.org">Hannes Wallnoefer</a>
+ * @author <a href="mailto:dlr@finemaltcoding.com">Daniel Rall</a>
  */
 public class XmlRpcServer
 {
@@ -150,8 +151,9 @@
      */
     class Worker extends XmlRpc
     {
-        Vector inParams;
-        StringBuffer strbuf;
+        private Vector inParams;
+        private ByteArrayOutputStream buffer;
+        private XmlWriter writer;
 
         /**
          * Creates a new instance.
@@ -159,7 +161,7 @@
         protected Worker()
         {
             inParams = new Vector();
-            strbuf = new StringBuffer();
+            buffer = new ByteArrayOutputStream();
         }
 
         /**
@@ -175,7 +177,7 @@
             finally
             {
                 // Release most of our resources
-                strbuf.setLength (0);
+                buffer.reset();
                 inParams.removeAllElements();
             }
         }
@@ -227,17 +229,22 @@
 
                 Object outParam;
                 if (handler instanceof AuthenticatedXmlRpcHandler)
+                {
                     outParam = ((AuthenticatedXmlRpcHandler) handler).
                             execute (methodName, inParams, user, password);
+                }
                 else
+                {
                     outParam = ((XmlRpcHandler) handler).execute (
                             methodName, inParams);
+                }
                 if (debug)
                     System.err.println ("outparam = "+outParam);
 
-                XmlWriter writer = new XmlWriter (strbuf);
+                writer = new XmlWriter(buffer);
                 writeResponse (outParam, writer);
-                result = writer.getBytes ();
+                writer.flush();
+                result = buffer.toByteArray();
             }
             catch (Exception x)
             {
@@ -249,32 +256,69 @@
                 // It is possible that something is in the buffer
                 // if there were an exception during the writeResponse()
                 // call above.
-                strbuf.setLength(0);
+                buffer.reset();
+
+                writer = null;
+                try
+                {
+                    writer = new XmlWriter(buffer);
+                }
+                catch (UnsupportedEncodingException encx)
+                {
+                    System.err.println("XmlRpcServer attempted to use " +
+                                       "unsupported encoding: " + encx);
+                    // NOTE: If we weren't already using the default
+                    // encoding, we could try it here.
+                }
+                catch (IOException iox)
+                {
+                    System.err.println("XmlRpcServer experienced I/O error " +
+                                       "writing error response: " + iox);
+                }
 
-                XmlWriter writer = new XmlWriter (strbuf);
                 String message = x.toString ();
-                // check if XmlRpcException was thrown so we can get an error code
+                // Retrieve XmlRpcException error code (if possible).
                 int code = x instanceof XmlRpcException ?
                         ((XmlRpcException) x).code : 0;
                 try
                 {
                     writeError (code, message, writer);
+                    writer.flush();
                 }
-                catch (XmlRpcException e)
+                catch (Exception e)
                 {
                     // Unlikely to occur, as we just sent a struct
                     // with an int and a string.
                     System.err.println("Unable to send error response to " +
                                        "client: " + e);
                 }
-                try
+
+                // If we were able to create a XmlWriter, we should
+                // have a response.
+                if (writer != null)
                 {
-                    result = writer.getBytes ();
+                    result = buffer.toByteArray();
                 }
-                catch (UnsupportedEncodingException encx)
+                else
+                {
+                    result = "".getBytes();
+                }
+            }
+            finally
+            {
+                if (writer != null)
                 {
-                    System.err.println ("XmlRpcServer.execute: "+encx);
-                    result = writer.toString().getBytes();
+                    try
+                    {
+                        writer.close();
+                    }
+                    catch (IOException iox)
+                    {
+                        // This is non-fatal, but worth logging a
+                        // warning for.
+                        System.err.println("Exception closing output stream: " +
+                                           iox);
+                    }
                 }
             }
             if (debug)
@@ -284,8 +328,9 @@
         }
 
         /**
-          * Called when an object to be added to the argument list has been parsed.
-          */
+         * Called when an object to be added to the argument list has
+         * been parsed.
+         */
         void objectParsed (Object what)
         {
             inParams.addElement (what);
@@ -294,14 +339,14 @@
         /**
           * Writes an XML-RPC response to the XML writer.
           */
-        void writeResponse (Object param,
-                XmlWriter writer) throws XmlRpcException
+        void writeResponse (Object param, XmlWriter writer)
+            throws XmlRpcException, IOException
         {
             writer.startElement ("methodResponse");
             // if (param == null) param = ""; // workaround for Frontier bug
             writer.startElement ("params");
             writer.startElement ("param");
-            writeObject (param, writer);
+            writer.writeObject (param);
             writer.endElement ("param");
             writer.endElement ("params");
             writer.endElement ("methodResponse");
@@ -310,8 +355,8 @@
         /**
          * Writes an XML-RPC error response to the XML writer.
          */
-        void writeError (int code, String message,
-                XmlWriter writer) throws XmlRpcException
+        void writeError (int code, String message, XmlWriter writer)
+            throws XmlRpcException, IOException
         {
             // System.err.println ("error: "+message);
             Hashtable h = new Hashtable ();
@@ -319,7 +364,7 @@
             h.put ("faultString", message);
             writer.startElement ("methodResponse");
             writer.startElement ("fault");
-            writeObject (h, writer);
+            writer.writeObject (h);
             writer.endElement ("fault");
             writer.endElement ("methodResponse");
         }

Re: XmlRpcServer found un-streamy

Posted by Daniel Rall <dl...@finemaltcoding.com>.
Timothy Peierls <ti...@peierls.net> writes:

> Daniel Rall wrote:
> >> That means the same data is buffered in memory (in various 
>>> forms) a minimum of 4 times (!!). If the response contains 
>>> large quantities of data, imagine the repercussions...
>
> I've observed significant overhead when returning large result
> objects. It used to be worse, before the intermediate values were
> cleared in XmlRpc.Worker, but it's still wasteful.
>
> So far, the response to this has centered on how the possibilities for
> streaming are limited by the need to produce the response body in
> memory before writing the Content-length header. However, there are
> other ways things could be improved.

Tim, thanks a lot for your comments.

> 1. It might be reasonable to perform two passes over the result
> object, one to compute the content length, and then one to stream it
> out.

The value of this change would depend heavily on how long it takes to
create the response, and just doesn't seem very elegant (though
neither does a frozen XML-RPC spec, bleh).

> 2. In any event, why not have XmlWriter write directly to an
> OutputStreamWriter that wraps a ByteArrayOutputStream (maybe itself
> wrapped in a BufferedWriter)? This would eliminate the need for a
> StringBuffer, which is one of the problem areas that Dan is worried
> about.

I was thinking along the same lines, but your description really
helped crystalize my thoughts.  I made XmlWriter sub-class the JDK's
OutputStreamWriter (for simplicity), and moved writeObject() into it.
I am about to commit to CVS, but have attached the patch to the end of
this message for comment and review.  As this is a fairly extensive
change, it would be great to get some eyes on it.

> 3. And, at the risk of being a one-note Johnny, I'll point out that
> were we to use List and Map instead of Vector and Hashtable, server
> side code that, for example, currently constructs a Vector in memory
> just to hold the results of iterating over some internal data
> structure might instead be able to use a custom List implementation
> that would effectively stream those results directly from the List's
> Iterator without having to form an intermediate array or Vector.

Tim, I would love to see some code implementing this idea!


All, please comment on these changes:

Index: src/java/org/apache/xmlrpc/XmlRpc.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpc.java,v
retrieving revision 1.16
diff -u -r1.16 XmlRpc.java
--- src/java/org/apache/xmlrpc/XmlRpc.java	18 Feb 2002 17:06:15 -0000	1.16
+++ src/java/org/apache/xmlrpc/XmlRpc.java	18 Feb 2002 22:44:16 -0000
@@ -72,6 +72,7 @@
  * @see XmlRpcClient
  *
  * @author <a href="mailto:hannes@apache.org">Hannes Wallnoefer</a>
+ * @author <a href="mailto:dlr@finemaltcoding.com">Daniel Rall</a>
  */
 public abstract class XmlRpc 
     extends HandlerBase
@@ -331,89 +332,6 @@
     }
 
     /**
-      * Writes the XML representation of a supported Java object to
-      * the XML writer.
-      */
-    void writeObject (Object what, XmlWriter writer)
-    {
-        writer.startElement ("value");
-        if (what == null)
-        {
-            throw new RuntimeException ("null value not supported by XML-RPC");
-        }
-        else if (what instanceof String)
-        {
-            writer.chardata (what.toString ());
-        }
-        else if (what instanceof Integer)
-        {
-            writer.startElement ("int");
-            writer.write (what.toString ());
-            writer.endElement ("int");
-        }
-        else if (what instanceof Boolean)
-        {
-            writer.startElement ("boolean");
-            writer.write (((Boolean) what).booleanValue () ? "1" : "0");
-            writer.endElement ("boolean");
-        }
-        else if (what instanceof Double || what instanceof Float)
-        {
-            writer.startElement ("double");
-            writer.write (what.toString ());
-            writer.endElement ("double");
-        }
-        else if (what instanceof Date)
-        {
-            writer.startElement ("dateTime.iso8601");
-            Date d = (Date) what;
-            writer.write (dateformat.format (d));
-            writer.endElement ("dateTime.iso8601");
-        }
-        else if (what instanceof byte[])
-        {
-            writer.startElement ("base64");
-            writer.write (Base64.encode ((byte[]) what));
-            writer.endElement ("base64");
-        }
-        else if (what instanceof Vector)
-        {
-            writer.startElement ("array");
-            writer.startElement ("data");
-            Vector v = (Vector) what;
-            int l2 = v.size ();
-            for (int i2 = 0; i2 < l2; i2++)
-                writeObject (v.elementAt (i2), writer);
-            writer.endElement ("data");
-            writer.endElement ("array");
-        }
-        else if (what instanceof Hashtable)
-        {
-            writer.startElement ("struct");
-            Hashtable h = (Hashtable) what;
-            for (Enumeration e = h.keys (); e.hasMoreElements ();)
-            {
-                String nextkey = (String) e.nextElement ();
-                Object nextval = h.get (nextkey);
-                writer.startElement ("member");
-                writer.startElement ("name");
-                writer.write (nextkey);
-                writer.endElement ("name");
-                writeObject (nextval, writer);
-                writer.endElement ("member");
-            }
-            writer.endElement ("struct");
-        }
-        else
-        {
-            throw new RuntimeException ("unsupported Java type: " +
-                    what.getClass ());
-        }
-        writer.endElement ("value");
-    }
-
-
-    /**
       *  This method is called when a root level object has been parsed.
       */
     abstract void objectParsed (Object what);
@@ -700,9 +618,14 @@
 
 
     /**
-     * A quick and dirty XML writer.
+     * A quick and dirty XML writer.  If you feed it a
+     * <code>ByteArrayInputStream</code>, it may be necessary to call
+     * <code>writer.flush()</code> before calling
+     * <code>buffer.toByteArray()</code> to get the data written to
+     * your byte buffer.
      */
     class XmlWriter
+        extends OutputStreamWriter
     {
         protected static final String PROLOG_START =
             "<?xml version=\"1.0\" encoding=\"";
@@ -713,55 +636,137 @@
         protected static final String GREATER_THAN_ENTITY = "&gt;";
         protected static final String AMPERSAND_ENTITY = "&amp;";
 
-        /**
-         * The buffer to write to.
-         */
-        StringBuffer buf;
-
-        /**
-         * The encoding to use.
-         */
-        String enc;
-
-        public XmlWriter (StringBuffer buf)
+        public XmlWriter (OutputStream out)
+            throws UnsupportedEncodingException, IOException
         {
             // The default encoding used for XML-RPC is ISO-8859-1.
-            this (buf, encoding);
+            this (out, encoding);
         }
 
-        public XmlWriter (StringBuffer buf, String enc)
+        public XmlWriter (OutputStream out, String enc)
+            throws UnsupportedEncodingException, IOException
         {
-            this.buf = buf;
-            this.enc = enc;
+            super(out, enc);
 
             // Add the XML prolog (which includes the encoding)
-            buf.append (PROLOG_START);
-            buf.append (encodings.getProperty (enc, enc));
-            buf.append (PROLOG_END);
+            write(PROLOG_START);
+            write(encodings.getProperty(enc, enc));
+            write(PROLOG_END);
         }
 
-        public void startElement (String elem)
+        /**
+         * Writes the XML representation of a supported Java object
+         * type.
+         *
+         * @param obj The <code>Object</code> to write.
+         */
+        public void writeObject (Object obj)
+            throws IOException
         {
-            buf.append ('<');
-            buf.append (elem);
-            buf.append ('>');
+            startElement ("value");
+            if (obj == null)
+            {
+                throw new RuntimeException("null value not supported by XML-RPC");
+            }
+            else if (obj instanceof String)
+            {
+                chardata(obj.toString());
+            }
+            else if (obj instanceof Integer)
+            {
+                startElement("int");
+                write(obj.toString());
+                endElement("int");
+            }
+            else if (obj instanceof Boolean)
+            {
+                startElement("boolean");
+                write(((Boolean) obj).booleanValue() ? "1" : "0");
+                endElement("boolean");
+            }
+            else if (obj instanceof Double || obj instanceof Float)
+            {
+                startElement("double");
+                write(obj.toString());
+                endElement("double");
+            }
+            else if (obj instanceof Date)
+            {
+                startElement("dateTime.iso8601");
+                Date d = (Date) obj;
+                write(dateformat.format(d));
+                endElement("dateTime.iso8601");
+            }
+            else if (obj instanceof byte[])
+            {
+                startElement("base64");
+                write(Base64.encode((byte[]) obj));
+                endElement("base64");
+            }
+            else if (obj instanceof Vector)
+            {
+                startElement("array");
+                startElement("data");
+                Vector array = (Vector) obj;
+                int size = array.size();
+                for (int i = 0; i < size; i++)
+                {
+                    writeObject(array.elementAt(i));
+                }
+                endElement("data");
+                endElement("array");
+            }
+            else if (obj instanceof Hashtable)
+            {
+                startElement("struct");
+                Hashtable struct = (Hashtable) obj;
+                for (Enumeration e = struct.keys(); e.hasMoreElements(); )
+                {
+                    String nextkey = (String) e.nextElement();
+                    Object nextval = struct.get(nextkey);
+                    startElement("member");
+                    startElement("name");
+                    write(nextkey);
+                    endElement("name");
+                    writeObject(nextval);
+                    endElement("member");
+                }
+                endElement("struct");
+            }
+            else
+            {
+                throw new RuntimeException("unsupported Java type: " +
+                                            obj.getClass());
+            }
+            endElement("value");
+        }
+
+        protected void startElement (String elem)
+            throws IOException
+        {
+            write('<');
+            write(elem);
+            write('>');
         }
 
-        public void endElement (String elem)
+        protected void endElement (String elem)
+            throws IOException
         {
-            buf.append (CLOSING_TAG_START);
-            buf.append (elem);
-            buf.append ('>');
+            write(CLOSING_TAG_START);
+            write(elem);
+            write('>');
         }
 
-        public void emptyElement (String elem)
+        protected void emptyElement (String elem)
+            throws IOException
         {
-            buf.append ('<');
-            buf.append (elem);
-            buf.append (SINGLE_TAG_END);
+            write('<');
+            write(elem);
+            write(SINGLE_TAG_END);
         }
 
-        public void chardata (String text)
+        protected void chardata (String text)
+            throws IOException
         {
             int l = text.length ();
             for (int i = 0; i < l; i++)
@@ -769,39 +774,19 @@
                 char c = text.charAt (i);
                 switch (c)
                 {
-                case '<' :
-                    buf.append (LESS_THAN_ENTITY);
+                case '<':
+                    write(LESS_THAN_ENTITY);
                     break;
-                case '>' :
-                    buf.append (GREATER_THAN_ENTITY);
+                case '>':
+                    write(GREATER_THAN_ENTITY);
                     break;
-                case '&' :
-                    buf.append (AMPERSAND_ENTITY);
+                case '&':
+                    write(AMPERSAND_ENTITY);
                     break;
-                default :
-                    buf.append (c);
+                default:
+                    write(c);
                 }
             }
-        }
-
-        public void write (char[] text)
-        {
-            buf.append (text);
-        }
-
-        public void write (String text)
-        {
-            buf.append (text);
-        }
-
-        public String toString ()
-        {
-            return buf.toString ();
-        }
-
-        public byte[] getBytes () throws UnsupportedEncodingException
-        {
-            return buf.toString ().getBytes (enc);
         }
     }
 }
Index: src/java/org/apache/xmlrpc/XmlRpcClient.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcClient.java,v
retrieving revision 1.5
diff -u -r1.5 XmlRpcClient.java
--- src/java/org/apache/xmlrpc/XmlRpcClient.java	18 Feb 2002 17:06:15 -0000	1.5
+++ src/java/org/apache/xmlrpc/XmlRpcClient.java	18 Feb 2002 22:44:17 -0000
@@ -254,7 +254,11 @@
     {
         boolean fault;
         Object result = null;
-        StringBuffer strbuf;
+
+        /**
+         * The output buffer used in creating a request.
+         */
+        ByteArrayOutputStream buffer;
 
         CallData call;
 
@@ -298,12 +302,15 @@
             catch (Exception x)
             {
                 if (callback != null)
+                {
                     try
                     {
                         callback.handleError (x, url, method);
                     }
                     catch (Exception ignore)
-                    {}
+                    {
+                    }
+                }
             }
         }
 
@@ -323,14 +330,19 @@
             {
                 ByteArrayOutputStream bout = new ByteArrayOutputStream ();
 
-                if (strbuf == null)
-                    strbuf = new StringBuffer ();
+                if (buffer == null)
+                {
+                    buffer = new ByteArrayOutputStream();
+                }
                 else
-                    strbuf.setLength (0);
+                {
+                    buffer.reset();
+                }
 
-                XmlWriter writer = new XmlWriter (strbuf);
+                XmlWriter writer = new XmlWriter (buffer);
                 writeRequest (writer, method, params);
-                byte[] request = writer.getBytes();
+                writer.flush();
+                byte[] request = buffer.toByteArray();
 
                 URLConnection con = url.openConnection ();
                 con.setDoInput (true);
@@ -407,7 +419,7 @@
             for (int i = 0; i < l; i++)
             {
                 writer.startElement ("param");
-                writeObject (params.elementAt (i), writer);
+                writer.writeObject (params.elementAt (i));
                 writer.endElement ("param");
             }
             writer.endElement ("params");
Index: src/java/org/apache/xmlrpc/XmlRpcClientLite.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcClientLite.java,v
retrieving revision 1.2
diff -u -r1.2 XmlRpcClientLite.java
--- src/java/org/apache/xmlrpc/XmlRpcClientLite.java	18 Feb 2002 17:06:15 -0000	1.2
+++ src/java/org/apache/xmlrpc/XmlRpcClientLite.java	18 Feb 2002 22:44:18 -0000
@@ -126,7 +126,6 @@
 
     class LiteWorker extends Worker implements Runnable
     {
-
         HttpClient client = null;
 
         public LiteWorker ()
@@ -134,21 +133,25 @@
             super ();
         }
 
-
-        Object execute (String method,
-                Vector params) throws XmlRpcException, IOException
+        Object execute (String method, Vector params)
+            throws XmlRpcException, IOException
         {
             long now = System.currentTimeMillis ();
             fault = false;
             try
             {
-                if (strbuf == null)
-                    strbuf = new StringBuffer ();
+                if (buffer == null)
+                {
+                    buffer = new ByteArrayOutputStream();
+                }
                 else
-                    strbuf.setLength (0);
-                XmlWriter writer = new XmlWriter (strbuf);
+                {
+                    buffer.reset();
+                }
+                XmlWriter writer = new XmlWriter (buffer);
                 writeRequest (writer, method, params);
-                byte[] request = writer.getBytes();
+                writer.flush();
+                byte[] request = buffer.toByteArray();
 
                 // and send it to the server
                 if (client == null)
Index: src/java/org/apache/xmlrpc/XmlRpcServer.java
===================================================================
RCS file: /home/cvs/xml-rpc/src/java/org/apache/xmlrpc/XmlRpcServer.java,v
retrieving revision 1.10
diff -u -r1.10 XmlRpcServer.java
--- src/java/org/apache/xmlrpc/XmlRpcServer.java	18 Feb 2002 17:06:15 -0000	1.10
+++ src/java/org/apache/xmlrpc/XmlRpcServer.java	18 Feb 2002 22:44:19 -0000
@@ -66,6 +66,7 @@
  * HTTP listener, use the WebServer class instead.
  *
  * @author <a href="mailto:hannes@apache.org">Hannes Wallnoefer</a>
+ * @author <a href="mailto:dlr@finemaltcoding.com">Daniel Rall</a>
  */
 public class XmlRpcServer
 {
@@ -150,8 +151,9 @@
      */
     class Worker extends XmlRpc
     {
-        Vector inParams;
-        StringBuffer strbuf;
+        private Vector inParams;
+        private ByteArrayOutputStream buffer;
+        private XmlWriter writer;
 
         /**
          * Creates a new instance.
@@ -159,7 +161,7 @@
         protected Worker()
         {
             inParams = new Vector();
-            strbuf = new StringBuffer();
+            buffer = new ByteArrayOutputStream();
         }
 
         /**
@@ -175,7 +177,7 @@
             finally
             {
                 // Release most of our resources
-                strbuf.setLength (0);
+                buffer.reset();
                 inParams.removeAllElements();
             }
         }
@@ -227,17 +229,22 @@
 
                 Object outParam;
                 if (handler instanceof AuthenticatedXmlRpcHandler)
+                {
                     outParam = ((AuthenticatedXmlRpcHandler) handler).
                             execute (methodName, inParams, user, password);
+                }
                 else
+                {
                     outParam = ((XmlRpcHandler) handler).execute (
                             methodName, inParams);
+                }
                 if (debug)
                     System.err.println ("outparam = "+outParam);
 
-                XmlWriter writer = new XmlWriter (strbuf);
+                writer = new XmlWriter(buffer);
                 writeResponse (outParam, writer);
-                result = writer.getBytes ();
+                writer.flush();
+                result = buffer.toByteArray();
             }
             catch (Exception x)
             {
@@ -249,32 +256,69 @@
                 // It is possible that something is in the buffer
                 // if there were an exception during the writeResponse()
                 // call above.
-                strbuf.setLength(0);
+                buffer.reset();
+
+                writer = null;
+                try
+                {
+                    writer = new XmlWriter(buffer);
+                }
+                catch (UnsupportedEncodingException encx)
+                {
+                    System.err.println("XmlRpcServer attempted to use " +
+                                       "unsupported encoding: " + encx);
+                    // NOTE: If we weren't already using the default
+                    // encoding, we could try it here.
+                }
+                catch (IOException iox)
+                {
+                    System.err.println("XmlRpcServer experienced I/O error " +
+                                       "writing error response: " + iox);
+                }
 
-                XmlWriter writer = new XmlWriter (strbuf);
                 String message = x.toString ();
-                // check if XmlRpcException was thrown so we can get an error code
+                // Retrieve XmlRpcException error code (if possible).
                 int code = x instanceof XmlRpcException ?
                         ((XmlRpcException) x).code : 0;
                 try
                 {
                     writeError (code, message, writer);
+                    writer.flush();
                 }
-                catch (XmlRpcException e)
+                catch (Exception e)
                 {
                     // Unlikely to occur, as we just sent a struct
                     // with an int and a string.
                     System.err.println("Unable to send error response to " +
                                        "client: " + e);
                 }
-                try
+
+                // If we were able to create a XmlWriter, we should
+                // have a response.
+                if (writer != null)
                 {
-                    result = writer.getBytes ();
+                    result = buffer.toByteArray();
                 }
-                catch (UnsupportedEncodingException encx)
+                else
+                {
+                    result = "".getBytes();
+                }
+            }
+            finally
+            {
+                if (writer != null)
                 {
-                    System.err.println ("XmlRpcServer.execute: "+encx);
-                    result = writer.toString().getBytes();
+                    try
+                    {
+                        writer.close();
+                    }
+                    catch (IOException iox)
+                    {
+                        // This is non-fatal, but worth logging a
+                        // warning for.
+                        System.err.println("Exception closing output stream: " +
+                                           iox);
+                    }
                 }
             }
             if (debug)
@@ -284,8 +328,9 @@
         }
 
         /**
-          * Called when an object to be added to the argument list has been parsed.
-          */
+         * Called when an object to be added to the argument list has
+         * been parsed.
+         */
         void objectParsed (Object what)
         {
             inParams.addElement (what);
@@ -294,14 +339,14 @@
         /**
           * Writes an XML-RPC response to the XML writer.
           */
-        void writeResponse (Object param,
-                XmlWriter writer) throws XmlRpcException
+        void writeResponse (Object param, XmlWriter writer)
+            throws XmlRpcException, IOException
         {
             writer.startElement ("methodResponse");
             // if (param == null) param = ""; // workaround for Frontier bug
             writer.startElement ("params");
             writer.startElement ("param");
-            writeObject (param, writer);
+            writer.writeObject (param);
             writer.endElement ("param");
             writer.endElement ("params");
             writer.endElement ("methodResponse");
@@ -310,8 +355,8 @@
         /**
          * Writes an XML-RPC error response to the XML writer.
          */
-        void writeError (int code, String message,
-                XmlWriter writer) throws XmlRpcException
+        void writeError (int code, String message, XmlWriter writer)
+            throws XmlRpcException, IOException
         {
             // System.err.println ("error: "+message);
             Hashtable h = new Hashtable ();
@@ -319,7 +364,7 @@
             h.put ("faultString", message);
             writer.startElement ("methodResponse");
             writer.startElement ("fault");
-            writeObject (h, writer);
+            writer.writeObject (h);
             writer.endElement ("fault");
             writer.endElement ("methodResponse");
         }

Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
Daniel Rall wrote:
>> That means the same data is buffered in memory (in various 
>> forms) a minimum of 4 times (!!). If the response contains 
>> large quantities of data, imagine the repercussions...

I've observed significant overhead when returning large result
objects. It used to be worse, before the intermediate values were
cleared in XmlRpc.Worker, but it's still wasteful.

So far, the response to this has centered on how the possibilities for
streaming are limited by the need to produce the response body in
memory before writing the Content-length header. However, there are
other ways things could be improved.

1. It might be reasonable to perform two passes over the result
object, one to compute the content length, and then one to stream it
out.

2. In any event, why not have XmlWriter write directly to an
OutputStreamWriter that wraps a ByteArrayOutputStream (maybe itself
wrapped in a BufferedWriter)? This would eliminate the need for a
StringBuffer, which is one of the problem areas that Dan is worried
about.

3. And, at the risk of being a one-note Johnny, I'll point out that
were we to use List and Map instead of Vector and Hashtable, server
side code that, for example, currently constructs a Vector in memory
just to hold the results of iterating over some internal data
structure might instead be able to use a custom List implementation
that would effectively stream those results directly from the List's
Iterator without having to form an intermediate array or Vector.

Of these, #2 seems to me the easiest to contemplate.

--tim

Re: XmlRpcServer found un-streamy

Posted by Steffen Schwigon <sc...@webit.de>.
Hannes Wallnoefer wrote:
> 
> Yes, this  is very unefficient for large resonses. The reason for doing
> it this way is that the XML-RPC says: "The Content-Length must be
> specified and must be correct." So we need the whole response to be
> packed before we can start sending headers.
> 
> Now I don't think this is really needed for the XML-RPC clients out
> there. Most that I'm aware of use a full HTTP client and should do fine
> without 

There is at least on implementation that uses the Content-length:
header. My own xmlrpc-client. :-), a quick (and you may call it 
therefore dirty) C++ client from the times when only java 
implementations ruled the xmlrpc world.

Of course I could update to better C++ xmlrpc-clients when I
want to use newer xmlrpc-versions and I personally don't want
to veto against this development trend. I just wanted to point 
out that implementations *exist* which really depend on particular 
specification details.


GreetinX
Steffen
-- 
Steffen Schwigon <sc...@webit.de>

Re: XmlRpcServer found un-streamy

Posted by Hannes Wallnoefer <ha...@helma.at>.
Timothy Peierls wrote:

>John Wilson wrote:
>
>>Personally I hate it but I think it would be unwise to 
>>break the spec. 
>>
>
>Agreed. You can bet there's some client out there that relies
>on Content-length because the spec said it would be there (and
>that it would be correct).
>
Yes. For a moment I had thought we could still have universal 
compatibility (well, close to universal ;-) without Content-Length 
header, but since this is obviously not the case I agree it's a bad idea 
to go that way.

Hannes




Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Daniel Rall" <dl...@finemaltcoding.com>
To: <rp...@xml.apache.org>; <da...@userland.com>
Sent: Thursday, February 14, 2002 6:44 PM
Subject: Re: XmlRpcServer found un-streamy


[snip]
> The XML-RPC spec apparently does not take HTTP/1.1 into consideration
> (I believe it came while HTTP/1.1 was still an emergent technology
> when the spec was published).
[snip]
> I think the spec needs adjustment for HTTP/1.1, so I'm CC'ing this
> mail to UserLand (as they appear to maintain the XML-RPC spec).  If no
> one has any issues with it, I will also be so bold as to create a
> branch in the CVS repository for adding HTTP/1.1 support to the client
> and server, banking on the eventuality of spec support.

Dan,

I think you may be new to XML-RPC. Please forgive me if you are not and you
already know this....


XML-RPC is a trademark of Dave Winer's Userland company. The spec is
copyrighted by Userland and is licensed under terms which allow people who
implement the spec to use the term XML-RPC and for people to change or
extend the spec in any way they like as long as they don't call it XML-RPC.
Dave is adamant that the spec is frozen. By frozen I understand that he
means that not a dot or comma of the spec can be changed and that
"clarifications" are not permitted. Dave has held this position for a very
long time against quite intense pressure (and I have done my share of
pressurising!). The spec has vagueness, ambiguity and contradicts the XML
spec in at least one place. However if we want to implement XML-RPC we have
to live with it. In practice there are common spec violations (allowing non
USASCII characters in strings so that Hannes can spell his name correctly,
for example). However there are certainly quite a few XML-RPC
implementations around which use HTTP 1.1 in the header but will not support
chunked transfers, just as there are XML-RPC servers who return a
Content-Length header to an HTTP 0.9 request.

Probably a better forum for us to discuss the
http://groups.yahoo.com/group/xml-rpc/ mailing list. You will find many
instances of people arguing for extensions, changes and clarifications in
the archive ;)

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Daniel Rall" <dl...@finemaltcoding.com>
To: <rp...@xml.apache.org>; <da...@userland.com>
Sent: Thursday, February 14, 2002 6:44 PM
Subject: Re: XmlRpcServer found un-streamy


[snip]
> The XML-RPC spec apparently does not take HTTP/1.1 into consideration
> (I believe it came while HTTP/1.1 was still an emergent technology
> when the spec was published).
[snip]
> I think the spec needs adjustment for HTTP/1.1, so I'm CC'ing this
> mail to UserLand (as they appear to maintain the XML-RPC spec).  If no
> one has any issues with it, I will also be so bold as to create a
> branch in the CVS repository for adding HTTP/1.1 support to the client
> and server, banking on the eventuality of spec support.

Dan,

I think you may be new to XML-RPC. Please forgive me if you are not and you
already know this....


XML-RPC is a trademark of Dave Winer's Userland company. The spec is
copyrighted by Userland and is licensed under terms which allow people who
implement the spec to use the term XML-RPC and for people to change or
extend the spec in any way they like as long as they don't call it XML-RPC.
Dave is adamant that the spec is frozen. By frozen I understand that he
means that not a dot or comma of the spec can be changed and that
"clarifications" are not permitted. Dave has held this position for a very
long time against quite intense pressure (and I have done my share of
pressurising!). The spec has vagueness, ambiguity and contradicts the XML
spec in at least one place. However if we want to implement XML-RPC we have
to live with it. In practice there are common spec violations (allowing non
USASCII characters in strings so that Hannes can spell his name correctly,
for example). However there are certainly quite a few XML-RPC
implementations around which use HTTP 1.1 in the header but will not support
chunked transfers, just as there are XML-RPC servers who return a
Content-Length header to an HTTP 0.9 request.

Probably a better forum for us to discuss the
http://groups.yahoo.com/group/xml-rpc/ mailing list. You will find many
instances of people arguing for extensions, changes and clarifications in
the archive ;)

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by Daniel Rall <dl...@finemaltcoding.com>.
[This message is a followup to a thread started by my original post
regarding the performance of the server provided by the Apache XML-RPC
Project (formerly the from Helma).]

Timothy Peierls <ti...@peierls.net> writes:

> The spec is not exactly clear about whether comments and PIs
> are allowed, so you can be sure that there is or will be an
> implementation out there that uses a trailing PI to send
> sideband information for clients that care about it, leaving
> other clients to ignore it. Innocent clients that rely on the 
> document close tag will break, and the rogue implementation 
> author can claim with some justification that the spec doesn't 
> disallow the trailing PIs. So clients cannot in general use 
> the closing tag in lieu of the Content-length.
>
> At any rate, there seems to be general agreement that servers
> can't omit the Content-length header, so ultimately, yes, the
> point is moot.

The XML-RPC spec apparently does not take HTTP/1.1 into consideration
(I believe it came while HTTP/1.1 was still an emergent technology
when the spec was published).

http://www.w3.org/Protocols/rfc2616/rfc2616.html

I spoke with Greg Stein about how Apache httpd handles large responses
(hopefully I don't misrepresent what he said here too badly ;-).
httpd apprently writes the Content-Length header if the response is
smaller than a certain threshold.  If the response size exceeds this
threshold, one of two things happens based on the client's HTTP
protocol support.  If the client does not support HTTP/1.1, the
Content-Length header is omitted and the response is streamed to the
client, terminated by the close of the socket connection (ignoring
Keep-Alive headers because of the great size of the response).  If
HTTP/1.1 is supported, Chunked Transfer Coding is used (see RFC 2616,
section 3.6.1):

  "All HTTP/1.1 applications MUST be able to receive and decode the
  'chunked' transfer-coding, [...]"

When chunking, the server says it's sending X bytes, dump X bytes to
the response stream, and repeats as necessary.  I would assume that
Keep-Alives would work in this situation, since there would be no need
to close the socket to signal end of stream.  This nicely reduces the
size of the RAM buffers necessary for sending large responses.
Section 4.4 (Message Length) provides a nice summary of implementation
requirements.

I think the spec needs adjustment for HTTP/1.1, so I'm CC'ing this
mail to UserLand (as they appear to maintain the XML-RPC spec).  If no
one has any issues with it, I will also be so bold as to create a
branch in the CVS repository for adding HTTP/1.1 support to the client
and server, banking on the eventuality of spec support.

                             Thanks, Dan

Re: XmlRpcServer found un-streamy

Posted by Daniel Rall <dl...@finemaltcoding.com>.
[This message is a followup to a thread started by my original post
regarding the performance of the server provided by the Apache XML-RPC
Project (formerly the from Helma).]

Timothy Peierls <ti...@peierls.net> writes:

> The spec is not exactly clear about whether comments and PIs
> are allowed, so you can be sure that there is or will be an
> implementation out there that uses a trailing PI to send
> sideband information for clients that care about it, leaving
> other clients to ignore it. Innocent clients that rely on the 
> document close tag will break, and the rogue implementation 
> author can claim with some justification that the spec doesn't 
> disallow the trailing PIs. So clients cannot in general use 
> the closing tag in lieu of the Content-length.
>
> At any rate, there seems to be general agreement that servers
> can't omit the Content-length header, so ultimately, yes, the
> point is moot.

The XML-RPC spec apparently does not take HTTP/1.1 into consideration
(I believe it came while HTTP/1.1 was still an emergent technology
when the spec was published).

http://www.w3.org/Protocols/rfc2616/rfc2616.html

I spoke with Greg Stein about how Apache httpd handles large responses
(hopefully I don't misrepresent what he said here too badly ;-).
httpd apprently writes the Content-Length header if the response is
smaller than a certain threshold.  If the response size exceeds this
threshold, one of two things happens based on the client's HTTP
protocol support.  If the client does not support HTTP/1.1, the
Content-Length header is omitted and the response is streamed to the
client, terminated by the close of the socket connection (ignoring
Keep-Alive headers because of the great size of the response).  If
HTTP/1.1 is supported, Chunked Transfer Coding is used (see RFC 2616,
section 3.6.1):

  "All HTTP/1.1 applications MUST be able to receive and decode the
  'chunked' transfer-coding, [...]"

When chunking, the server says it's sending X bytes, dump X bytes to
the response stream, and repeats as necessary.  I would assume that
Keep-Alives would work in this situation, since there would be no need
to close the socket to signal end of stream.  This nicely reduces the
size of the RAM buffers necessary for sending large responses.
Section 4.4 (Message Length) provides a nice summary of implementation
requirements.

I think the spec needs adjustment for HTTP/1.1, so I'm CC'ing this
mail to UserLand (as they appear to maintain the XML-RPC spec).  If no
one has any issues with it, I will also be so bold as to create a
branch in the CVS repository for adding HTTP/1.1 support to the client
and server, banking on the eventuality of spec support.

                             Thanks, Dan

Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
John Wilson wrote:
> > > If you are sending well formed XML you can
> > > tell when the response has ended without it.

> From: "Timothy Peierls" <ti...@peierls.net>
> > Can you? Aren't comments and PIs allowed after the top level
> > tag is closed? Or is that an old, fixed XML spec bug?

John Wilson wrote:
> As the XML-RPC implementation never sends PIs the point is 
> somewhat moot ;)

The spec is not exactly clear about whether comments and PIs
are allowed, so you can be sure that there is or will be an
implementation out there that uses a trailing PI to send
sideband information for clients that care about it, leaving
other clients to ignore it. Innocent clients that rely on the 
document close tag will break, and the rogue implementation 
author can claim with some justification that the spec doesn't 
disallow the trailing PIs. So clients cannot in general use 
the closing tag in lieu of the Content-length.

At any rate, there seems to be general agreement that servers
can't omit the Content-length header, so ultimately, yes, the
point is moot.

--tim

Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
John Wilson wrote:
> > > If you are sending well formed XML you can
> > > tell when the response has ended without it.

> From: "Timothy Peierls" <ti...@peierls.net>
> > Can you? Aren't comments and PIs allowed after the top level
> > tag is closed? Or is that an old, fixed XML spec bug?

John Wilson wrote:
> As the XML-RPC implementation never sends PIs the point is 
> somewhat moot ;)

The spec is not exactly clear about whether comments and PIs
are allowed, so you can be sure that there is or will be an
implementation out there that uses a trailing PI to send
sideband information for clients that care about it, leaving
other clients to ignore it. Innocent clients that rely on the 
document close tag will break, and the rogue implementation 
author can claim with some justification that the spec doesn't 
disallow the trailing PIs. So clients cannot in general use 
the closing tag in lieu of the Content-length.

At any rate, there seems to be general agreement that servers
can't omit the Content-length header, so ultimately, yes, the
point is moot.

--tim

Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message ----- 
From: "Timothy Peierls" <ti...@peierls.net>
To: <rp...@xml.apache.org>
Sent: Wednesday, February 13, 2002 11:09 PM
Subject: Re: XmlRpcServer found un-streamy


> John Wilson wrote:
> > If you are sending well formed XML you can
> > tell when the response has ended without it.
> 
> Can you? Aren't comments and PIs allowed after the top level
> tag is closed? Or is that an old, fixed XML spec bug?

As the XML-RPC implementation never sends PIs the point is somewhat moot ;)

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message ----- 
From: "Timothy Peierls" <ti...@peierls.net>
To: <rp...@xml.apache.org>
Sent: Wednesday, February 13, 2002 11:09 PM
Subject: Re: XmlRpcServer found un-streamy


> John Wilson wrote:
> > If you are sending well formed XML you can
> > tell when the response has ended without it.
> 
> Can you? Aren't comments and PIs allowed after the top level
> tag is closed? Or is that an old, fixed XML spec bug?

As the XML-RPC implementation never sends PIs the point is somewhat moot ;)

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by Hannes Wallnoefer <ha...@helma.at>.
Timothy Peierls wrote:

>John Wilson wrote:
>
>>Personally I hate it but I think it would be unwise to 
>>break the spec. 
>>
>
>Agreed. You can bet there's some client out there that relies
>on Content-length because the spec said it would be there (and
>that it would be correct).
>
Yes. For a moment I had thought we could still have universal 
compatibility (well, close to universal ;-) without Content-Length 
header, but since this is obviously not the case I agree it's a bad idea 
to go that way.

Hannes




Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
John Wilson wrote:
> If you are sending well formed XML you can
> tell when the response has ended without it.

Can you? Aren't comments and PIs allowed after the top level
tag is closed? Or is that an old, fixed XML spec bug?


> Personally I hate it but I think it would be unwise to 
> break the spec. 

Agreed. You can bet there's some client out there that relies
on Content-length because the spec said it would be there (and
that it would be correct).

--tim

Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
John Wilson wrote:
> If you are sending well formed XML you can
> tell when the response has ended without it.

Can you? Aren't comments and PIs allowed after the top level
tag is closed? Or is that an old, fixed XML spec bug?


> Personally I hate it but I think it would be unwise to 
> break the spec. 

Agreed. You can bet there's some client out there that relies
on Content-length because the spec said it would be there (and
that it would be correct).

--tim

Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Hannes Wallnoefer" <ha...@helma.at>
To: <rp...@xml.apache.org>
Sent: Wednesday, February 13, 2002 10:49 AM
Subject: Re: XmlRpcServer found un-streamy


> Yes, this  is very unefficient for large resonses. The reason for doing
> it this way is that the XML-RPC says: "The Content-Length must be
> specified and must be correct." So we need the whole response to be
> packed before we can start sending headers.
>
> Now I don't think this is really needed for the XML-RPC clients out
> there. Most that I'm aware of use a full HTTP client and should do fine
> without (as long as HTTP keep-alive is disabled, because with keep-alive
> the client needs Content-Length to know when it has read the whole
> response). If this is the case, I think it would be a good idea to offer
> methods for an optional stream-based interface to XmlRpcServer.

It's one of the few things the spec is clear on. Personally I hate it but I
think it would be unwise to break the spec. (especially in a prestigious
implementation like the Apache one). Does the HTTP spec *require*
Content-Length with keep-alive? If you are sending well formed XML you can
tell when the response has ended without it.

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by John Wilson <tu...@wilson.co.uk>.
----- Original Message -----
From: "Hannes Wallnoefer" <ha...@helma.at>
To: <rp...@xml.apache.org>
Sent: Wednesday, February 13, 2002 10:49 AM
Subject: Re: XmlRpcServer found un-streamy


> Yes, this  is very unefficient for large resonses. The reason for doing
> it this way is that the XML-RPC says: "The Content-Length must be
> specified and must be correct." So we need the whole response to be
> packed before we can start sending headers.
>
> Now I don't think this is really needed for the XML-RPC clients out
> there. Most that I'm aware of use a full HTTP client and should do fine
> without (as long as HTTP keep-alive is disabled, because with keep-alive
> the client needs Content-Length to know when it has read the whole
> response). If this is the case, I think it would be a good idea to offer
> methods for an optional stream-based interface to XmlRpcServer.

It's one of the few things the spec is clear on. Personally I hate it but I
think it would be unwise to break the spec. (especially in a prestigious
implementation like the Apache one). Does the HTTP spec *require*
Content-Length with keep-alive? If you are sending well formed XML you can
tell when the response has ended without it.

John Wilson
The Wilson Partnership
http://www.wilson.co.uk


Re: XmlRpcServer found un-streamy

Posted by Timothy Peierls <ti...@peierls.net>.
Daniel Rall wrote:
>> That means the same data is buffered in memory (in various 
>> forms) a minimum of 4 times (!!). If the response contains 
>> large quantities of data, imagine the repercussions...

I've observed significant overhead when returning large result
objects. It used to be worse, before the intermediate values were
cleared in XmlRpc.Worker, but it's still wasteful.

So far, the response to this has centered on how the possibilities for
streaming are limited by the need to produce the response body in
memory before writing the Content-length header. However, there are
other ways things could be improved.

1. It might be reasonable to perform two passes over the result
object, one to compute the content length, and then one to stream it
out.

2. In any event, why not have XmlWriter write directly to an
OutputStreamWriter that wraps a ByteArrayOutputStream (maybe itself
wrapped in a BufferedWriter)? This would eliminate the need for a
StringBuffer, which is one of the problem areas that Dan is worried
about.

3. And, at the risk of being a one-note Johnny, I'll point out that
were we to use List and Map instead of Vector and Hashtable, server
side code that, for example, currently constructs a Vector in memory
just to hold the results of iterating over some internal data
structure might instead be able to use a custom List implementation
that would effectively stream those results directly from the List's
Iterator without having to form an intermediate array or Vector.

Of these, #2 seems to me the easiest to contemplate.

--tim

Re: XmlRpcServer found un-streamy

Posted by Steffen Schwigon <sc...@webit.de>.
Hannes Wallnoefer wrote:
> 
> Yes, this  is very unefficient for large resonses. The reason for doing
> it this way is that the XML-RPC says: "The Content-Length must be
> specified and must be correct." So we need the whole response to be
> packed before we can start sending headers.
> 
> Now I don't think this is really needed for the XML-RPC clients out
> there. Most that I'm aware of use a full HTTP client and should do fine
> without 

There is at least on implementation that uses the Content-length:
header. My own xmlrpc-client. :-), a quick (and you may call it 
therefore dirty) C++ client from the times when only java 
implementations ruled the xmlrpc world.

Of course I could update to better C++ xmlrpc-clients when I
want to use newer xmlrpc-versions and I personally don't want
to veto against this development trend. I just wanted to point 
out that implementations *exist* which really depend on particular 
specification details.


GreetinX
Steffen
-- 
Steffen Schwigon <sc...@webit.de>

Re: XmlRpcServer found un-streamy

Posted by Hannes Wallnoefer <ha...@helma.at>.
Yes, this  is very unefficient for large resonses. The reason for doing 
it this way is that the XML-RPC says: "The Content-Length must be 
specified and must be correct." So we need the whole response to be 
packed before we can start sending headers.

Now I don't think this is really needed for the XML-RPC clients out 
there. Most that I'm aware of use a full HTTP client and should do fine 
without (as long as HTTP keep-alive is disabled, because with keep-alive 
the client needs Content-Length to know when it has read the whole 
response). If this is the case, I think it would be a good idea to offer 
methods for an optional stream-based interface to XmlRpcServer.

Hannes


Daniel Rall wrote:

>When the XmlRpcServer class responds to requests, all the data in the
>response is first buffered in memory in whatever form is reasonable
>for output (such as a Hashtable), then written to a StringBuffer, then
>converted to a String, then a converted to byte[].  That means the
>same data is buffered in memory (in various forms) a minimum of 4
>times (!!).  If the response contains large quantities of data,
>imagine the repurcussions...
>
>The current performance for large data sets is just not sufficient.  I
>would like to make the API more "streamy".  Velocity's template
>processing API
><http://jakarta.apache.org/velocity/api/org/apache/velocity/app/Velocity.html#mergeTemplate(java.lang.String,%20java.lang.String,%20org.apache.velocity.context.Context,%20java.io.Writer)>
>is a good example of how I would like the XML-RPC server to act.
>
>Thoughts, comments?
>
>                             Thanks, Dan
>





Re: XmlRpcServer found un-streamy

Posted by Hannes Wallnoefer <ha...@helma.at>.
Yes, this  is very unefficient for large resonses. The reason for doing 
it this way is that the XML-RPC says: "The Content-Length must be 
specified and must be correct." So we need the whole response to be 
packed before we can start sending headers.

Now I don't think this is really needed for the XML-RPC clients out 
there. Most that I'm aware of use a full HTTP client and should do fine 
without (as long as HTTP keep-alive is disabled, because with keep-alive 
the client needs Content-Length to know when it has read the whole 
response). If this is the case, I think it would be a good idea to offer 
methods for an optional stream-based interface to XmlRpcServer.

Hannes


Daniel Rall wrote:

>When the XmlRpcServer class responds to requests, all the data in the
>response is first buffered in memory in whatever form is reasonable
>for output (such as a Hashtable), then written to a StringBuffer, then
>converted to a String, then a converted to byte[].  That means the
>same data is buffered in memory (in various forms) a minimum of 4
>times (!!).  If the response contains large quantities of data,
>imagine the repurcussions...
>
>The current performance for large data sets is just not sufficient.  I
>would like to make the API more "streamy".  Velocity's template
>processing API
><http://jakarta.apache.org/velocity/api/org/apache/velocity/app/Velocity.html#mergeTemplate(java.lang.String,%20java.lang.String,%20org.apache.velocity.context.Context,%20java.io.Writer)>
>is a good example of how I would like the XML-RPC server to act.
>
>Thoughts, comments?
>
>                             Thanks, Dan
>