Thread: Re: [INTERFACES] Re: [PATCHES] Re: Fixes and enhancements to JDBC driver(take 2)

Re: [INTERFACES] Re: [PATCHES] Re: Fixes and enhancements to JDBC driver(take 2)

From
Bruce Momjian
Date:
Can someone comment on this?

---------------------------------------------------------------------------


Peter,

What is the final resolution of this for 7.1?  I noticed that you have
essentially commented out the byte array pooling code in current
sources.  Given that 7.1 is rapidly coming to a close, is this something
that is expected to get fixed and reenabled for 7.1, or is this going to
wait for 7.2.

I don't like the way the code has been left at this moment.  Even though
much of the code has been commented out, it still does stuff like
allocate 512 SimpleObjectPool objects per PG_Stream (via the
BytePoolDim1) that are then never used.

So if this is going to get fixed for 7.1, it needs to be done soon,
preferably by beta4, otherwise I would recommend taking all of this code
completely out of source control until it can be done correctly for 7.2
(instead of commenting out certain pieces).  The reason I suggest this
is that it is confusing for anyone looking at the source code seeing all
of this stuff that isn't being used currently.

Finally, I will make a plug for my version of a fix to this overall
problem that I sent out last weekend (Gunner did you finally get that
attachment, and have you looked at that code?).  I can redo that fix
against current sources and send a proper patch file if anyone thinks
that is the best course of action.

thanks,
--Barry


Bruce Momjian wrote:
>
> Gunnar, everyone, can people check the CVS or snapshot and make sure you
> are happy.  This has been a confusing item for me, and I want to make
> sure we have it working as intended.
>
> Thanks.
>
> [ Charset ISO-8859-1 unsupported, converting... ]
> > Quoting Gunnar R|nning <gunnar@candleweb.no>:
> >
> > > Barry Lind <barry@xythos.com> writes:
> > >
> > > > Gunner,
> > > >
> > > > Do your fixes address the concerns I raised on 12/21?  (I have
> > > included
> > > > that email to the list below).  To summarize the three major
> > > > concerns/bugs were:
> > > > 1) Code incorrectly deallocates when a new statement is executed,
> > > even
> > > > though the byte[]s are still being used.
> > > > 2) The cache isn't limited in size, resulting in essentially memory
> > > > leaks for long lived connections in a connection pool.
> > > > 3) The implementation is limited to a max 256 byte byte[], whereas my
> > > > queries have many values that exceed this size, and the current
> > > > implementation doesn't lend itself well (because of #2) to cache
> > > things
> > > > upto 8K in size.
> > >
> > > The original patch that I supplied was a proof of concept on what kind
> > > of
> > > performance improvements that could be made by reusing byte arrays.
> > > This
> > > was unfortunately committed before anybody but me had done any testing
> > > at
> > > all on it.
> > >
> > > The most serious problem with this code was your issue 1). Number 2) and
> > > 3)
> > > should be easy to handle has config parameters. The reason for
> > > hardcoding
> > > 3) to 256 was simply because I found this to be the most optimal value
> > > for
> > > the web application I was doing the testing on.
> > >
> > > Eventually, it should be configurable whether to use the byte[] caching
> > > implementation or not, as the perfomance of memory allocation may vary
> > > greatly depending on VM and OS implementations.
> >
> > Now we use ANT this is extremely easy to do. Also, I have made some changes, in
> > that your supplied classes have moved to a new package org.postgresql.core, and
> > extend an interface ObjectPool. I did this so that it would be easier to change
> > the implementations without having to hack the main code too much.
> >
> > Although your patch is in there, I've disabled the part that frees the arrays
> > (as that was the bit causing problems). Hopefully...
> >
> > >
> > > If you go back to the October archives of pgsql-general you will find a
> > > pointer to my second shot at an implementation - this one fix your issue
> > > 1)
> > > but not the others.
> > >
> > > I would like to see what you have been working on as well, so we can
> > > come up with the best of breeds solution.
> >
> > In cvs as of yesterday...
> >
> > Peter
> >
> > --
> > Peter Mount peter@retep.org.uk
> > PostgreSQL JDBC Driver: http://www.retep.org.uk/postgres/
> > RetepPDF PDF library for Java: http://www.retep.org.uk/pdf/
> >
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026



--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: [INTERFACES] Re: [PATCHES] Re: Fixes and enhancements to JDBC driver(take 2)

From
Peter T Mount
Date:
Quoting Bruce Momjian <pgman@candle.pha.pa.us>:

> Can someone comment on this?
>
> ---------------------------------------------------------------------------
>
>
> Peter,
>
> What is the final resolution of this for 7.1?  I noticed that you have
> essentially commented out the byte array pooling code in current
> sources.  Given that 7.1 is rapidly coming to a close, is this
> something
> that is expected to get fixed and reenabled for 7.1, or is this going
> to wait for 7.2.

I commented it out to make sure that when 7.1 was released the driver would at
least work (with it enabled we would have one broken driver).

I've got a lot of work to do before 7.1 is released, and commenting out the
code was IMHO the best way to ensure that we had something that worked.

As for waiting for 7.2, I'd say 7.1.1. Unless it's something major (like the
change from postgresql.Driver to org.postgresql.Driver) JDBC doesn't
necessarily follow the postgresql versions. As it is, getting this feature is
now a bug fix anyhow...

> I don't like the way the code has been left at this moment.  Even
> though
> much of the code has been commented out, it still does stuff like
> allocate 512 SimpleObjectPool objects per PG_Stream (via the
> BytePoolDim1) that are then never used.

Yes, although all it takes is for me to comment out that bit as well before 7.1
is released.

> So if this is going to get fixed for 7.1, it needs to be done soon,
> preferably by beta4, otherwise I would recommend taking all of this
> code
> completely out of source control until it can be done correctly for 7.2
> (instead of commenting out certain pieces).  The reason I suggest this
> is that it is confusing for anyone looking at the source code seeing
> all
> of this stuff that isn't being used currently.

If it goes out of the source, it will be lost. Believe me it will. With the
ammount of commits made to JDBC recently, its easy for one copy of the source
to get out of sync.

The jdbc driver is under continual development. The CVS is meant to be dynamic,
not contain just the "working" copy. If it was, then no software project would
ever get completed.

> Finally, I will make a plug for my version of a fix to this overall
> problem that I sent out last weekend (Gunner did you finally get that
> attachment, and have you looked at that code?).  I can redo that fix
> against current sources and send a proper patch file if anyone thinks
> that is the best course of action.

I've not seen this...

Peter

>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
> >
> > Gunnar, everyone, can people check the CVS or snapshot and make sure
> you
> > are happy.  This has been a confusing item for me, and I want to make
> > sure we have it working as intended.
> >
> > Thanks.
> >
> > [ Charset ISO-8859-1 unsupported, converting... ]
> > > Quoting Gunnar R|nning <gunnar@candleweb.no>:
> > >
> > > > Barry Lind <barry@xythos.com> writes:
> > > >
> > > > > Gunner,
> > > > >
> > > > > Do your fixes address the concerns I raised on 12/21?  (I have
> > > > included
> > > > > that email to the list below).  To summarize the three major
> > > > > concerns/bugs were:
> > > > > 1) Code incorrectly deallocates when a new statement is
> executed,
> > > > even
> > > > > though the byte[]s are still being used.
> > > > > 2) The cache isn't limited in size, resulting in essentially
> memory
> > > > > leaks for long lived connections in a connection pool.
> > > > > 3) The implementation is limited to a max 256 byte byte[],
> whereas my
> > > > > queries have many values that exceed this size, and the current
> > > > > implementation doesn't lend itself well (because of #2) to
> cache
> > > > things
> > > > > upto 8K in size.
> > > >
> > > > The original patch that I supplied was a proof of concept on what
> kind
> > > > of
> > > > performance improvements that could be made by reusing byte
> arrays.
> > > > This
> > > > was unfortunately committed before anybody but me had done any
> testing
> > > > at
> > > > all on it.
> > > >
> > > > The most serious problem with this code was your issue 1). Number
> 2) and
> > > > 3)
> > > > should be easy to handle has config parameters. The reason for
> > > > hardcoding
> > > > 3) to 256 was simply because I found this to be the most optimal
> value
> > > > for
> > > > the web application I was doing the testing on.
> > > >
> > > > Eventually, it should be configurable whether to use the byte[]
> caching
> > > > implementation or not, as the perfomance of memory allocation may
> vary
> > > > greatly depending on VM and OS implementations.
> > >
> > > Now we use ANT this is extremely easy to do. Also, I have made some
> changes, in
> > > that your supplied classes have moved to a new package
> org.postgresql.core, and
> > > extend an interface ObjectPool. I did this so that it would be
> easier to change
> > > the implementations without having to hack the main code too much.
> > >
> > > Although your patch is in there, I've disabled the part that frees
> the arrays
> > > (as that was the bit causing problems). Hopefully...
> > >
> > > >
> > > > If you go back to the October archives of pgsql-general you will
> find a
> > > > pointer to my second shot at an implementation - this one fix your
> issue
> > > > 1)
> > > > but not the others.
> > > >
> > > > I would like to see what you have been working on as well, so we
> can
> > > > come up with the best of breeds solution.
> > >
> > > In cvs as of yesterday...
> > >
> > > Peter
> > >
> > > --
> > > Peter Mount peter@retep.org.uk
> > > PostgreSQL JDBC Driver: http://www.retep.org.uk/postgres/
> > > RetepPDF PDF library for Java: http://www.retep.org.uk/pdf/
> > >
> >
> > --
> >   Bruce Momjian                        |  http://candle.pha.pa.us
> >   pgman@candle.pha.pa.us               |  (610) 853-3000
> >   +  If your life is a hard drive,     |  830 Blythe Avenue
> >   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania
> 19026
>
>
>
> --
>   Bruce Momjian                        |  http://candle.pha.pa.us
>   pgman@candle.pha.pa.us               |  (610) 853-3000
>   +  If your life is a hard drive,     |  830 Blythe Avenue
>   +  Christ can be your backup.        |  Drexel Hill, Pennsylvania
> 19026
>



--
Peter Mount peter@retep.org.uk
PostgreSQL JDBC Driver: http://www.retep.org.uk/postgres/
RetepPDF PDF library for Java: http://www.retep.org.uk/pdf/

Re: [INTERFACES] Re: [PATCHES] Re: Fixes and enhancements to JDBC driver(take 2)

From
Richard Bullington-McGuire
Date:
On Fri, 2 Feb 2001, Peter T Mount wrote:

> > What is the final resolution of this for 7.1?  I noticed that you have
> > essentially commented out the byte array pooling code in current
> > sources.  Given that 7.1 is rapidly coming to a close, is this
> > something
> > that is expected to get fixed and reenabled for 7.1, or is this going
> > to wait for 7.2.
>
> I commented it out to make sure that when 7.1 was released the driver
> would at least work (with it enabled we would have one broken driver).

At Microstate, we fixed the byte pooling code when it was still in
PG_Stream. We patched this and submitted our patch to pgsql-patches, and
it was accepted.  It did have some nasty off-by-one errors. It does not
look like the fixes we did made it into the new, separate
org.postgresql.core.BytePoolDim[12] classes.

We've been using this code on an application with many tens of thousands
of queries per day for more than a month without any problems.

I've attached the production PG_Stream.java class that we've been using.

 --
 Richard Bullington-McGuire  <rbulling@microstate.com>
 Chief Technology Officer, The Microstate Corporation
 Phone: 703-796-6446  URL: http://www.microstate.com/
 PGP key IDs:    RSA: 0x93862305   DH/DSS: 0xDAC3028E


Attachment
Here is a context diff of the current CVS and the user-supplied version
for review.

> On Fri, 2 Feb 2001, Peter T Mount wrote:
>
> > > What is the final resolution of this for 7.1?  I noticed that you have
> > > essentially commented out the byte array pooling code in current
> > > sources.  Given that 7.1 is rapidly coming to a close, is this
> > > something
> > > that is expected to get fixed and reenabled for 7.1, or is this going
> > > to wait for 7.2.
> >
> > I commented it out to make sure that when 7.1 was released the driver
> > would at least work (with it enabled we would have one broken driver).
>
> At Microstate, we fixed the byte pooling code when it was still in
> PG_Stream. We patched this and submitted our patch to pgsql-patches, and
> it was accepted.  It did have some nasty off-by-one errors. It does not
> look like the fixes we did made it into the new, separate
> org.postgresql.core.BytePoolDim[12] classes.
>
> We've been using this code on an application with many tens of thousands
> of queries per day for more than a month without any problems.
>
> I've attached the production PG_Stream.java class that we've been using.
>
>  --
>  Richard Bullington-McGuire  <rbulling@microstate.com>
>  Chief Technology Officer, The Microstate Corporation
>  Phone: 703-796-6446  URL: http://www.microstate.com/
>  PGP key IDs:    RSA: 0x93862305   DH/DSS: 0xDAC3028E
>
Content-Description:

[ Attachment, skipping... ]


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
*** /bjm/0    Fri Feb  2 12:51:43 2001
--- PG_Stream.java    Thu Jan 18 13:16:00 2001
***************
*** 6,11 ****
--- 6,12 ----
  import java.util.*;
  import java.sql.*;
  import org.postgresql.*;
+ import org.postgresql.core.*;
  import org.postgresql.util.*;

  /**
***************
*** 22,31 ****
    private Socket connection;
    private InputStream pg_input;
    private BufferedOutputStream pg_output;
!
      BytePoolDim1 bytePoolDim1 = new BytePoolDim1();
      BytePoolDim2 bytePoolDim2 = new BytePoolDim2();
!
    /**
     * Constructor:  Connect to the PostgreSQL back end and return
     * a stream connection.
--- 23,32 ----
    private Socket connection;
    private InputStream pg_input;
    private BufferedOutputStream pg_output;
!
      BytePoolDim1 bytePoolDim1 = new BytePoolDim1();
      BytePoolDim2 bytePoolDim2 = new BytePoolDim2();
!
    /**
     * Constructor:  Connect to the PostgreSQL back end and return
     * a stream connection.
***************
*** 37,52 ****
    public PG_Stream(String host, int port) throws IOException
    {
      connection = new Socket(host, port);
!
      // Submitted by Jason Venner <jason@idiom.com> adds a 10x speed
      // improvement on FreeBSD machines (caused by a bug in their TCP Stack)
      connection.setTcpNoDelay(true);
!
      // Buffer sizes submitted by Sverre H Huseby <sverrehu@online.no>
      pg_input = new BufferedInputStream(connection.getInputStream(), 8192);
      pg_output = new BufferedOutputStream(connection.getOutputStream(), 8192);
    }
!
    /**
     * Sends a single character to the back end
     *
--- 38,53 ----
    public PG_Stream(String host, int port) throws IOException
    {
      connection = new Socket(host, port);
!
      // Submitted by Jason Venner <jason@idiom.com> adds a 10x speed
      // improvement on FreeBSD machines (caused by a bug in their TCP Stack)
      connection.setTcpNoDelay(true);
!
      // Buffer sizes submitted by Sverre H Huseby <sverrehu@online.no>
      pg_input = new BufferedInputStream(connection.getInputStream(), 8192);
      pg_output = new BufferedOutputStream(connection.getOutputStream(), 8192);
    }
!
    /**
     * Sends a single character to the back end
     *
***************
*** 59,69 ****
        //byte b[] = new byte[1];
        //b[0] = (byte)val;
        //pg_output.write(b);
!
        // Optimised version by Sverre H. Huseby Aug 22 1999 Applied Sep 13 1999
        pg_output.write((byte)val);
    }
!
    /**
     * Sends an integer to the back end
     *
--- 60,70 ----
        //byte b[] = new byte[1];
        //b[0] = (byte)val;
        //pg_output.write(b);
!
        // Optimised version by Sverre H. Huseby Aug 22 1999 Applied Sep 13 1999
        pg_output.write((byte)val);
    }
!
    /**
     * Sends an integer to the back end
     *
***************
*** 74,80 ****
    public void SendInteger(int val, int siz) throws IOException
    {
      byte[] buf = bytePoolDim1.allocByte(siz);
!
      while (siz-- > 0)
        {
      buf[siz] = (byte)(val & 0xff);
--- 75,81 ----
    public void SendInteger(int val, int siz) throws IOException
    {
      byte[] buf = bytePoolDim1.allocByte(siz);
!
      while (siz-- > 0)
        {
      buf[siz] = (byte)(val & 0xff);
***************
*** 82,88 ****
        }
      Send(buf);
    }
!
    /**
     * Sends an integer to the back end in reverse order.
     *
--- 83,89 ----
        }
      Send(buf);
    }
!
    /**
     * Sends an integer to the back end in reverse order.
     *
***************
*** 106,112 ****
        }
      Send(buf);
    }
!
    /**
     * Send an array of bytes to the backend
     *
--- 107,113 ----
        }
      Send(buf);
    }
!
    /**
     * Send an array of bytes to the backend
     *
***************
*** 117,123 ****
    {
      pg_output.write(buf);
    }
!
    /**
     * Send an exact array of bytes to the backend - if the length
     * has not been reached, send nulls until it has.
--- 118,124 ----
    {
      pg_output.write(buf);
    }
!
    /**
     * Send an exact array of bytes to the backend - if the length
     * has not been reached, send nulls until it has.
***************
*** 130,136 ****
    {
      Send(buf,0,siz);
    }
!
    /**
     * Send an exact array of bytes to the backend - if the length
     * has not been reached, send nulls until it has.
--- 131,137 ----
    {
      Send(buf,0,siz);
    }
!
    /**
     * Send an exact array of bytes to the backend - if the length
     * has not been reached, send nulls until it has.
***************
*** 143,149 ****
    public void Send(byte buf[], int off, int siz) throws IOException
    {
      int i;
!
      pg_output.write(buf, off, ((buf.length-off) < siz ? (buf.length-off) : siz));
      if((buf.length-off) < siz)
        {
--- 144,150 ----
    public void Send(byte buf[], int off, int siz) throws IOException
    {
      int i;
!
      pg_output.write(buf, off, ((buf.length-off) < siz ? (buf.length-off) : siz));
      if((buf.length-off) < siz)
        {
***************
*** 153,159 ****
        }
        }
    }
!
    /**
     * Sends a packet, prefixed with the packet's length
     * @param buf buffer to send
--- 154,160 ----
        }
        }
    }
!
    /**
     * Sends a packet, prefixed with the packet's length
     * @param buf buffer to send
***************
*** 164,170 ****
      SendInteger(buf.length+4,4);
      Send(buf);
    }
!
    /**
     * Receives a single character from the backend
     *
--- 165,171 ----
      SendInteger(buf.length+4,4);
      Send(buf);
    }
!
    /**
     * Receives a single character from the backend
     *
***************
*** 174,180 ****
    public int ReceiveChar() throws SQLException
    {
      int c = 0;
!
      try
        {
      c = pg_input.read();
--- 175,181 ----
    public int ReceiveChar() throws SQLException
    {
      int c = 0;
!
      try
        {
      c = pg_input.read();
***************
*** 184,190 ****
        }
        return c;
    }
!
    /**
     * Receives an integer from the backend
     *
--- 185,191 ----
        }
        return c;
    }
!
    /**
     * Receives an integer from the backend
     *
***************
*** 195,207 ****
    public int ReceiveInteger(int siz) throws SQLException
    {
      int n = 0;
!
      try
        {
      for (int i = 0 ; i < siz ; i++)
        {
          int b = pg_input.read();
!
          if (b < 0)
            throw new PSQLException("postgresql.stream.eof");
          n = n | (b << (8 * i)) ;
--- 196,208 ----
    public int ReceiveInteger(int siz) throws SQLException
    {
      int n = 0;
!
      try
        {
      for (int i = 0 ; i < siz ; i++)
        {
          int b = pg_input.read();
!
          if (b < 0)
            throw new PSQLException("postgresql.stream.eof");
          n = n | (b << (8 * i)) ;
***************
*** 211,217 ****
        }
        return n;
    }
!
    /**
     * Receives an integer from the backend
     *
--- 212,218 ----
        }
        return n;
    }
!
    /**
     * Receives an integer from the backend
     *
***************
*** 222,234 ****
    public int ReceiveIntegerR(int siz) throws SQLException
    {
      int n = 0;
!
      try
        {
      for (int i = 0 ; i < siz ; i++)
        {
          int b = pg_input.read();
!
          if (b < 0)
            throw new PSQLException("postgresql.stream.eof");
          n = b | (n << 8);
--- 223,235 ----
    public int ReceiveIntegerR(int siz) throws SQLException
    {
      int n = 0;
!
      try
        {
      for (int i = 0 ; i < siz ; i++)
        {
          int b = pg_input.read();
!
          if (b < 0)
            throw new PSQLException("postgresql.stream.eof");
          n = b | (n << 8);
***************
*** 270,293 ****
      byte[] rst = bytePoolDim1.allocByte(maxsiz);
      return ReceiveString(rst, maxsiz, encoding);
    }
!
    /**
     * Receives a null-terminated string from the backend.  Maximum of
     * maxsiz bytes - if we don't see a null, then we assume something
     * has gone wrong.
     *
!    * @param rst byte array to read the String into. rst.length must
!    *        equal to or greater than maxsize.
     * @param maxsiz maximum length of string in bytes
     * @param encoding the charset encoding to use.
     * @return string from back end
     * @exception SQLException if an I/O error occurs
     */
!   public String ReceiveString(byte rst[], int maxsiz, String encoding)
        throws SQLException
    {
      int s = 0;
!
      try
        {
      while (s < maxsiz)
--- 271,294 ----
      byte[] rst = bytePoolDim1.allocByte(maxsiz);
      return ReceiveString(rst, maxsiz, encoding);
    }
!
    /**
     * Receives a null-terminated string from the backend.  Maximum of
     * maxsiz bytes - if we don't see a null, then we assume something
     * has gone wrong.
     *
!    * @param rst byte array to read the String into. rst.length must
!    *        equal to or greater than maxsize.
     * @param maxsiz maximum length of string in bytes
     * @param encoding the charset encoding to use.
     * @return string from back end
     * @exception SQLException if an I/O error occurs
     */
!   public String ReceiveString(byte rst[], int maxsiz, String encoding)
        throws SQLException
    {
      int s = 0;
!
      try
        {
      while (s < maxsiz)
***************
*** 318,324 ****
        }
        return v;
    }
!
    /**
     * Read a tuple from the back end.  A tuple is a two dimensional
     * array of bytes
--- 319,325 ----
        }
        return v;
    }
!
    /**
     * Read a tuple from the back end.  A tuple is a two dimensional
     * array of bytes
***************
*** 334,343 ****
      int i, bim = (nf + 7)/8;
      byte[] bitmask = Receive(bim);
      byte[][] answer = bytePoolDim2.allocByte(nf);
!
      int whichbit = 0x80;
      int whichbyte = 0;
!
      for (i = 0 ; i < nf ; ++i)
        {
      boolean isNull = ((bitmask[whichbyte] & whichbit) == 0);
--- 335,344 ----
      int i, bim = (nf + 7)/8;
      byte[] bitmask = Receive(bim);
      byte[][] answer = bytePoolDim2.allocByte(nf);
!
      int whichbit = 0x80;
      int whichbyte = 0;
!
      for (i = 0 ; i < nf ; ++i)
        {
      boolean isNull = ((bitmask[whichbyte] & whichbit) == 0);
***************
*** 347,367 ****
          ++whichbyte;
          whichbit = 0x80;
        }
!     if (isNull)
        answer[i] = null;
      else
        {
          int len = ReceiveIntegerR(4);
!         if (!bin)
            len -= 4;
!         if (len < 0)
            len = 0;
          answer[i] = Receive(len);
        }
        }
      return answer;
    }
!
    /**
     * Reads in a given number of bytes from the backend
     *
--- 348,368 ----
          ++whichbyte;
          whichbit = 0x80;
        }
!     if (isNull)
        answer[i] = null;
      else
        {
          int len = ReceiveIntegerR(4);
!         if (!bin)
            len -= 4;
!         if (len < 0)
            len = 0;
          answer[i] = Receive(len);
        }
        }
      return answer;
    }
!
    /**
     * Reads in a given number of bytes from the backend
     *
***************
*** 375,381 ****
      Receive(answer,0,siz);
      return answer;
    }
!
    /**
     * Reads in a given number of bytes from the backend
     *
--- 376,382 ----
      Receive(answer,0,siz);
      return answer;
    }
!
    /**
     * Reads in a given number of bytes from the backend
     *
***************
*** 387,394 ****
    public void Receive(byte[] b,int off,int siz) throws SQLException
    {
      int s = 0;
!
!     try
        {
      while (s < siz)
        {
--- 388,395 ----
    public void Receive(byte[] b,int off,int siz) throws SQLException
    {
      int s = 0;
!
!     try
        {
      while (s < siz)
        {
***************
*** 401,407 ****
        throw new PSQLException("postgresql.stream.ioerror",e);
        }
    }
!
    /**
     * This flushes any pending output to the backend. It is used primarily
     * by the Fastpath code.
--- 402,408 ----
        throw new PSQLException("postgresql.stream.ioerror",e);
        }
    }
!
    /**
     * This flushes any pending output to the backend. It is used primarily
     * by the Fastpath code.
***************
*** 415,421 ****
        throw new PSQLException("postgresql.stream.flush",e);
      }
    }
!
    /**
     * Closes the connection
     *
--- 416,422 ----
        throw new PSQLException("postgresql.stream.flush",e);
      }
    }
!
    /**
     * Closes the connection
     *
***************
*** 430,577 ****
      connection.close();
    }

-   /**
-    * Deallocate all resources that has been associated with any previous
-    * query.
-    */
-   public void deallocate(){
-       bytePoolDim1.deallocate();
-       bytePoolDim2.deallocate();
-   }
- }
-
- /**
-  * A simple and fast object pool implementation that can pool objects
-  * of any type. This implementation is not thread safe, it is up to the users
-  * of this class to assure thread safety.
-  */
- class ObjectPool {
-     int cursize = 0;
-     int maxsize = 16;
-     Object arr[] = new Object[maxsize];
-
-     public void add(Object o){
-     if(cursize >= maxsize){
-         Object newarr[] = new Object[maxsize*2];
-         System.arraycopy(arr, 0, newarr, 0, maxsize);
-         maxsize = maxsize * 2;
-         arr = newarr;
-     }
-     arr[cursize++] = o;
-     }
-
-     public Object remove(){
-     return arr[--cursize];
-     }
-     public boolean isEmpty(){
-     return cursize == 0;
-     }
-     public int size(){
-     return cursize;
-     }
-     public void addAll(ObjectPool pool){
-     int srcsize = pool.size();
-     if(srcsize == 0)
-         return;
-     int totalsize = srcsize + cursize;
-     if(totalsize > maxsize){
-         Object newarr[] = new Object[totalsize*2];
-         System.arraycopy(arr, 0, newarr, 0, cursize);
-         maxsize = maxsize = totalsize * 2;
-         arr = newarr;
-     }
-     System.arraycopy(pool.arr, 0, arr, cursize, srcsize);
-     cursize = totalsize;
-     }
-     public void clear(){
-         cursize = 0;
-     }
  }

- /**
-  * A simple and efficient class to pool one dimensional byte arrays
-  * of different sizes.
-  */
- class BytePoolDim1 {
-     int maxsize = 256;
-     ObjectPool notusemap[] = new ObjectPool[maxsize + 1];
-     ObjectPool inusemap[] = new ObjectPool[maxsize + 1];
-
-     public BytePoolDim1(){
-     for(int i = 0; i <= maxsize; i++){
-         inusemap[i] = new ObjectPool();
-         notusemap[i] = new ObjectPool();
-     }
-     }
-
-     public byte[] allocByte(int size){
-     if(size > maxsize){
-         return new byte[size];
-     }
-
-     ObjectPool not_usel = notusemap[size];
-     ObjectPool in_usel = inusemap[size];
-     byte b[] = null;
-
-     if(!not_usel.isEmpty()) {
-         Object o = not_usel.remove();
-         b = (byte[]) o;
-     } else
-         b = new byte[size];
-     in_usel.add(b);
-
-     return b;
-     }
-
-     public void deallocate(){
-     for(int i = 0; i <= maxsize; i++){
-         notusemap[i].addAll(inusemap[i]);
-         inusemap[i].clear();
-     }
-
-     }
- }
-
-
-
- /**
-  * A simple and efficient class to pool two dimensional byte arrays
-  * of different sizes.
-  */
- class BytePoolDim2 {
-     int maxsize = 32;
-     ObjectPool notusemap[] = new ObjectPool[maxsize + 1];
-     ObjectPool inusemap[] = new ObjectPool[maxsize + 1];
-
-     public BytePoolDim2(){
-     for(int i = 0; i <= maxsize; i++){
-         inusemap[i] = new ObjectPool();
-         notusemap[i] = new ObjectPool();
-     }
-     }
-
-     public byte[][] allocByte(int size){
-     if(size > maxsize){
-         return new byte[size][0];
-     }
-     ObjectPool not_usel = notusemap[size];
-     ObjectPool in_usel =  inusemap[size];
-
-     byte b[][] = null;
-
-     if(!not_usel.isEmpty()) {
-         Object o = not_usel.remove();
-         b = (byte[][]) o;
-     } else
-         b = new byte[size][0];
-     in_usel.add(b);
-     return b;
-     }
-
-     public void deallocate(){
-     for(int i = 0; i <= maxsize; i++){
-         notusemap[i].addAll(inusemap[i]);
-         inusemap[i].clear();
-     }
-     }
- }
--- 431,435 ----

At 09:30 02/02/01 -0500, Richard Bullington-McGuire wrote:
>On Fri, 2 Feb 2001, Peter T Mount wrote:
>
> > > What is the final resolution of this for 7.1?  I noticed that you have
> > > essentially commented out the byte array pooling code in current
> > > sources.  Given that 7.1 is rapidly coming to a close, is this
> > > something
> > > that is expected to get fixed and reenabled for 7.1, or is this going
> > > to wait for 7.2.
> >
> > I commented it out to make sure that when 7.1 was released the driver
> > would at least work (with it enabled we would have one broken driver).
>
>At Microstate, we fixed the byte pooling code when it was still in
>PG_Stream. We patched this and submitted our patch to pgsql-patches, and
>it was accepted.  It did have some nasty off-by-one errors. It does not
>look like the fixes we did made it into the new, separate
>org.postgresql.core.BytePoolDim[12] classes.


Hmmm, should have as I remember puting your fixes in. I remember this as I
had to apply the patch manually as I did it while moving the code out of
PG_Stream. I'll double check.



>We've been using this code on an application with many tens of thousands
>of queries per day for more than a month without any problems.
>
>I've attached the production PG_Stream.java class that we've been using.


Thanks.