Re: [INTERFACES] Re: [PATCHES] Re: Fixes and enhancements to JDBC driver(take 2) - Mailing list pgsql-jdbc

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