Thread: Re: Re: JDBC Performance
Gunnar, Your new JDBC driver (postgresql.jar, 29-Sept-2000, 14:47, 187K) caused the following error. Using these tables... ------------------------------------------------------------------------ >>> CREATE TABLE servers ( pid INT4 PRIMARY KEY, tableid INT2, host TEXT, port INT4); >>> CREATE TABLE classes ( tableid INT2, classname TEXT, tablename TEXT); >>> CREATE TABLE persistent ( pid INT4 PRIMARY KEY, tableid INT2); >>> CREATE TABLE test ( pid INT4 PRIMARY KEY, tableid INT2, my_string TEXT, my_long INT8, my_double FLOAT8, ref INT8); >>> CREATE TABLE pids ( next_lpid INT4); >>> CREATE TABLE test2 ( pid INT4 PRIMARY KEY, tableid INT2, one INT4, two INT2, three INT2, name TEXT, four FLOAT4, five FLOAT8, six INT8); ------------------------------------------------------------------------ I run this select statement... SELECT host, port FROM Servers WHERE PID=1; Bad Integer int4 at org.postgresql.jdbc2.ResultSet.getInt(ResultSet.java:261) at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:748) at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:789) at com.idisys.odb.ODBManager.loadMain(ODBManager.java:655) at com.idisys.odb.ODBManager.load(ODBManager.java:584) at com.idisys.odb.ODBManager.getObject(ODBManager.java:790) at com.idisys.odb.ODBManager.getServer(ODBManager.java:814) at com.idisys.odb.Reference.getServer(Reference.java:27) at com.idisys.odb.Reference.getURL(Reference.java:39) at com.idisys.odb.Test.test(Test.java:319) at com.idisys.odb.Test.main(Test.java:124) - Keith -----Original Message----- From: Gunnar R|nning <gunnar@candleweb.no> To: Peter Mount <peter@retep.org.uk> Cc: kientzle@acm.org <kientzle@acm.org>; PostgreSQL general mailing list <pgsql-general@postgresql.org>; Keith L. Musser <kmusser@idisys.com> Date: Friday, September 29, 2000 9:08 AM Subject: Re: [GENERAL] Re: JDBC Performance >Peter Mount <peter@retep.org.uk> writes: > >> >> Email them to me, as the modifications will break when I commit my changes >> (delayed due to stress related illness), and there's a lot of changes in >> there. I'm about to resume work in a few minutes. >> > >Okay, I wrapped up the modifications now. I'm appending the patch against >the current CVS. You can also find the patch and a precompiled version of >the driver at : > >http://www.candleweb.no/~gunnar/projects/pgsql/ > >The interesting part is the replacement of new byte[] with an allocByte() >method called that uses a pool of different byte arrays. I first tried >using the JDK 1.2 datastructures to implement the pooling, but they had too >much overhead so I created a couple of simple and dirty implementations >instead. > >I also added ReceiveString() methods that can take byte[] array as >parameter. All the ReceiveString methods in Connection now uses one shared >byte array instead of forcing ReceiveString to allocate a new one on each >call. > >Comments and test results from others are very welcome. > >Maybe I will look into doing the custom char conversion this weekend, as >the default implementation provided by Sun appears to be the current >bottleneck. As Tim Kientzle wrote in another mail, this implementation is >instatiating a new converter object every time you do a conversion. This is >is also pointed out has a bottleneck by OptimizeIT. > >Regards, > > Gunnar > >? postgresql.jar >? lazy_result.diff >? bytecache.diff >? org/postgresql/DriverClass.java >Index: org/postgresql/Connection.java >=================================================================== >RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/Co nnection.java,v >retrieving revision 1.6 >diff -c -r1.6 Connection.java >*** org/postgresql/Connection.java 2000/09/12 05:09:54 1.6 >--- org/postgresql/Connection.java 2000/09/29 12:54:12 >*************** >*** 81,86 **** >--- 81,91 ---- > // The PID an cancellation key we get from the backend process > public int pid; > public int ckey; >+ >+ // This receive_sbuf should be used by the different methods >+ // that call pg_stream.ReceiveString() in this Connection, so >+ // so we avoid uneccesary new allocations. >+ byte receive_sbuf[] = new byte[8192]; > > /** > * This is called by Class.forName() from within org.postgresql.Driver >*************** >*** 165,171 **** > // "User authentication failed" > // > throw new SQLException(pg_stream.ReceiveString >! (4096, getEncoding())); > > case 'R': > // Get the type of request >--- 170,176 ---- > // "User authentication failed" > // > throw new SQLException(pg_stream.ReceiveString >! (receive_sbuf, 4096, getEncoding())); > > case 'R': > // Get the type of request >*************** >*** 236,242 **** > case 'E': > case 'N': > throw new SQLException(pg_stream.ReceiveString >! (4096, getEncoding())); > default: > throw new PSQLException("postgresql.con.setup"); > } >--- 241,247 ---- > case 'E': > case 'N': > throw new SQLException(pg_stream.ReceiveString >! (receive_sbuf, 4096, getEncoding())); > default: > throw new PSQLException("postgresql.con.setup"); > } >*************** >*** 248,254 **** > break; > case 'E': > case 'N': >! throw new SQLException(pg_stream.ReceiveString(4096)); > default: > throw new PSQLException("postgresql.con.setup"); > } >--- 253,259 ---- > break; > case 'E': > case 'N': >! throw new SQLException(pg_stream.ReceiveString(receive_sbuf, 4096, getEncoding())); > default: > throw new PSQLException("postgresql.con.setup"); > } >*************** >*** 306,312 **** > //currentDateStyle=i+1; // this is the index of the format > //} > } >! > /** > * Send a query to the backend. Returns one of the ResultSet > * objects. >--- 311,317 ---- > //currentDateStyle=i+1; // this is the index of the format > //} > } >! > /** > * Send a query to the backend. Returns one of the ResultSet > * objects. >*************** >*** 322,328 **** > { > // added Oct 7 1998 to give us thread safety. > synchronized(pg_stream) { >! > Field[] fields = null; > Vector tuples = new Vector(); > byte[] buf = null; >--- 327,339 ---- > { > // added Oct 7 1998 to give us thread safety. > synchronized(pg_stream) { >! // Deallocate all resources in the stream associated >! // with a previous request. >! // This will let the driver reuse byte arrays that has already >! // been allocated instead of allocating new ones in order >! // to gain performance improvements. >! pg_stream.deallocate(); >! > Field[] fields = null; > Vector tuples = new Vector(); > byte[] buf = null; >*************** >*** 352,359 **** > try > { > pg_stream.SendChar('Q'); >! buf = sql.getBytes(); >! pg_stream.Send(buf); > pg_stream.SendChar(0); > pg_stream.flush(); > } catch (IOException e) { >--- 363,369 ---- > try > { > pg_stream.SendChar('Q'); >! pg_stream.Send(sql.getBytes()); > pg_stream.SendChar(0); > pg_stream.flush(); > } catch (IOException e) { >*************** >*** 370,376 **** > { > case 'A': // Asynchronous Notify > pid = pg_stream.ReceiveInteger(4); >! msg = pg_stream.ReceiveString(8192); > break; > case 'B': // Binary Data Transfer > if (fields == null) >--- 380,387 ---- > { > case 'A': // Asynchronous Notify > pid = pg_stream.ReceiveInteger(4); >! msg = pg_stream.ReceiveString(receive_sbuf, 8192, >! getEncoding()); > break; > case 'B': // Binary Data Transfer > if (fields == null) >*************** >*** 381,387 **** > tuples.addElement(tup); > break; > case 'C': // Command Status >! recv_status = pg_stream.ReceiveString(8192); > > // Now handle the update count correctly. > if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")) { >--- 392,400 ---- > tuples.addElement(tup); > break; > case 'C': // Command Status >! recv_status = >! pg_stream.ReceiveString(receive_sbuf, 8192, >! getEncoding()); > > // Now handle the update count correctly. > if(recv_status.startsWith("INSERT") || recv_status.startsWith("UPDATE") || recv_status.startsWith("DELETE")) { >*************** >*** 423,429 **** > tuples.addElement(tup); > break; > case 'E': // Error Message >! msg = pg_stream.ReceiveString(4096); > final_error = new SQLException(msg); > hfr = true; > break; >--- 436,443 ---- > tuples.addElement(tup); > break; > case 'E': // Error Message >! msg = pg_stream.ReceiveString(receive_sbuf, 4096, >! getEncoding()); > final_error = new SQLException(msg); > hfr = true; > break; >*************** >*** 438,447 **** > hfr = true; > break; > case 'N': // Error Notification >! addWarning(pg_stream.ReceiveString(4096)); > break; > case 'P': // Portal Name >! String pname = pg_stream.ReceiveString(8192); > break; > case 'T': // MetaData Field Description > if (fields != null) >--- 452,465 ---- > hfr = true; > break; > case 'N': // Error Notification >! addWarning(pg_stream.ReceiveString(receive_sbuf, >! 4096, >! getEncoding())); > break; > case 'P': // Portal Name >! String pname = >! pg_stream.ReceiveString(receive_sbuf, 8192, >! getEncoding()); > break; > case 'T': // MetaData Field Description > if (fields != null) >*************** >*** 461,466 **** >--- 479,486 ---- > } > } > >+ >+ > /** > * Receive the field descriptions from the back end > * >*************** >*** 474,480 **** > > for (i = 0 ; i < nf ; ++i) > { >! String typname = pg_stream.ReceiveString(8192); > int typid = pg_stream.ReceiveIntegerR(4); > int typlen = pg_stream.ReceiveIntegerR(2); > int typmod = pg_stream.ReceiveIntegerR(4); >--- 494,501 ---- > > for (i = 0 ; i < nf ; ++i) > { >! String typname = pg_stream.ReceiveString(receive_sbuf, 8192, >! getEncoding()); > int typid = pg_stream.ReceiveIntegerR(4); > int typlen = pg_stream.ReceiveIntegerR(2); > int typmod = pg_stream.ReceiveIntegerR(4); >Index: org/postgresql/PG_Stream.java >=================================================================== >RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/PG _Stream.java,v >retrieving revision 1.2 >diff -c -r1.2 PG_Stream.java >*** org/postgresql/PG_Stream.java 2000/09/12 04:58:47 1.2 >--- org/postgresql/PG_Stream.java 2000/09/29 12:54:12 >*************** >*** 22,28 **** >--- 22,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. >*************** >*** 70,76 **** > */ > public void SendInteger(int val, int siz) throws IOException > { >! byte[] buf = new byte[siz]; > > while (siz-- > 0) > { >--- 74,80 ---- > */ > public void SendInteger(int val, int siz) throws IOException > { >! byte[] buf = bytePoolDim1.allocByte(siz); > > while (siz-- > 0) > { >*************** >*** 94,100 **** > */ > public void SendIntegerReverse(int val, int siz) throws IOException > { >! byte[] buf = new byte[siz]; > int p=0; > while (siz-- > 0) > { >--- 98,104 ---- > */ > public void SendIntegerReverse(int val, int siz) throws IOException > { >! byte[] buf = bytePoolDim1.allocByte(siz); > int p=0; > while (siz-- > 0) > { >*************** >*** 236,258 **** > return n; > } > >! public String ReceiveString(int maxsize) throws SQLException { >! return ReceiveString(maxsize, null); >! } >! > /** > * 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 encoding the charset encoding to use. >- * @param maxsiz maximum length of string in bytes > * @return string from back end > * @exception SQLException if an I/O error occurs > */ > public String ReceiveString(int maxsiz, String encoding) throws SQLException > { >! byte[] rst = new byte[maxsiz]; > int s = 0; > > try >--- 240,292 ---- > return n; > } > >! > /** > * 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 maxsiz maximum length of string >+ * @return string from back end >+ * @exception SQLException if an I/O error occurs >+ */ >+ public String ReceiveString(int maxsiz) throws SQLException >+ { >+ byte[] rst = bytePoolDim1.allocByte(maxsiz); >+ return ReceiveString(rst, maxsiz, null); >+ } >+ >+ /** >+ * 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 maxsiz maximum length of string > * @param encoding the charset encoding to use. > * @return string from back end > * @exception SQLException if an I/O error occurs > */ > public String ReceiveString(int maxsiz, String encoding) throws SQLException > { >! 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 >*************** >*** 262,270 **** > int c = pg_input.read(); > if (c < 0) > throw new PSQLException("postgresql.stream.eof"); >! else if (c == 0) >! break; >! else > rst[s++] = (byte)c; > } > if (s >= maxsiz) >--- 296,305 ---- > int c = pg_input.read(); > if (c < 0) > throw new PSQLException("postgresql.stream.eof"); >! else if (c == 0) { >! rst[s] = 0; >! break; >! } else > rst[s++] = (byte)c; > } > if (s >= maxsiz) >*************** >*** 299,305 **** > { > int i, bim = (nf + 7)/8; > byte[] bitmask = Receive(bim); >! byte[][] answer = new byte[nf][0]; > > int whichbit = 0x80; > int whichbyte = 0; >--- 334,340 ---- > { > int i, bim = (nf + 7)/8; > byte[] bitmask = Receive(bim); >! byte[][] answer = bytePoolDim2.allocByte(nf); > > int whichbit = 0x80; > int whichbyte = 0; >*************** >*** 337,343 **** > */ > private byte[] Receive(int siz) throws SQLException > { >! byte[] answer = new byte[siz]; > Receive(answer,0,siz); > return answer; > } >--- 372,378 ---- > */ > private byte[] Receive(int siz) throws SQLException > { >! byte[] answer = bytePoolDim1.allocByte(siz); > Receive(answer,0,siz); > return answer; > } >*************** >*** 395,398 **** >--- 430,581 ---- > pg_input.close(); > 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]; >+ ObjectPool inusemap[] = new ObjectPool[maxsize]; >+ byte binit[][] = new byte[maxsize][0]; >+ >+ public BytePoolDim1(){ >+ for(int i = 0; i < maxsize; i++){ >+ binit[i] = new byte[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]; >+ ObjectPool inusemap[] = new ObjectPool[maxsize]; >+ >+ 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(); >+ } >+ } >+ } >+ >
"Keith L. Musser" <kmusser@idisys.com> writes:
> Gunnar,
>
> Your new JDBC driver (postgresql.jar, 29-Sept-2000, 14:47, 187K) caused
> the following error.
>
Thanks, I will look into the problem. The regression tests that Peter Mount
talking about would have been nice to have to catch things like this.
You cannot by any chance send me a copy of your source code, so I have
better chance of understanding and debugging the problem ?
Regards,
Gunnar
"Keith L. Musser" <kmusser@idisys.com> writes:
> Gunnar,
>
> Your new JDBC driver (postgresql.jar, 29-Sept-2000, 14:47, 187K) caused
> the following error.
>
>
> SELECT host, port FROM Servers WHERE PID=1;
> Bad Integer int4
> at org.postgresql.jdbc2.ResultSet.getInt(ResultSet.java:261)
> at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:748)
> at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:789)
OK, I found the problem. I will post a fixed version later today. The
problem was that getObject() executed Field.getSQLType() which in turn
executed Connection.ExecSQL(). I modified ExecSQL to deallocate the cached
byte arrays on entry, because I believed it only were called by
Statement.execute() methods. I guess I should move the deallocation into
the Statement classes instead, as that is were it really belongs.
I interpret the JDBC spec. to say that only one ResultSet will be open
per. Statement, but one Connection canm have several statements with one
result set each.
Regards,
Gunnar
[feel stupid replying to myself...]
Gunnar R|nning <gunnar@candleweb.no> writes:
> > at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:789)
>
> OK, I found the problem. I will post a fixed version later today. The
> problem was that getObject() executed Field.getSQLType() which in turn
> executed Connection.ExecSQL(). I modified ExecSQL to deallocate the cached
> byte arrays on entry, because I believed it only were called by
> Statement.execute() methods. I guess I should move the deallocation into
> the Statement classes instead, as that is were it really belongs.
>
> I interpret the JDBC spec. to say that only one ResultSet will be open
> per. Statement, but one Connection canm have several statements with one
> result set each.
>
This does of course imply that arrays should be cached on a
ResultSet/Statement basis instead of on PGStream as it is done now. Do
anybody have good suggestions on how to implement this ?
Approach 1:
The cache is now only per Connection, maybe we should a global pool of free
byte arrays instead ?
Cons :
This would probably mean that we need to add more
synchronization to ensure safe access by concurrent threads and could
therefore lead to poorer performance and concurrency.
Pros : Possibly lower memory consumption and higher performance in some
cases(when you find free byte arrays in the global pool). If your
application is not pooling connections, but recreating connections it would
also benefit performance wise from this approach.
Approach 2:
Another solution would be have the cache be per connection but associate a
pool of used byte arrays to each resultset/statement and deallocate these
on resultset.close()/statement.close().
Pros: Faster for the typical web application that uses pooled connections,
because this approach would require less synchronization.
Cons: Higher memory consumption.
Either of these two approaches would probably require some reorganization
of how the driver works.
Any other suggestions or comments ?
Regards,
Gunnar
On 29 Sep 2000, Gunnar R|nning wrote: > "Keith L. Musser" <kmusser@idisys.com> writes: > > > Gunnar, > > > > Your new JDBC driver (postgresql.jar, 29-Sept-2000, 14:47, 187K) caused > > the following error. > > > > > > > SELECT host, port FROM Servers WHERE PID=1; > > Bad Integer int4 > > at org.postgresql.jdbc2.ResultSet.getInt(ResultSet.java:261) > > at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:748) > > at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:789) > > OK, I found the problem. I will post a fixed version later today. The > problem was that getObject() executed Field.getSQLType() which in turn > executed Connection.ExecSQL(). I modified ExecSQL to deallocate the cached > byte arrays on entry, because I believed it only were called by > Statement.execute() methods. I guess I should move the deallocation into > the Statement classes instead, as that is were it really belongs. > > I interpret the JDBC spec. to say that only one ResultSet will be open > per. Statement, but one Connection canm have several statements with one > result set each. That is true, but you have to be careful of some of the DatabaseMetaData methods as well, as some of them do issue their own queries to get their results. It would be a pain to create new Statements just for them. This area also has problems when transactions are being used, but without nested transactions, it's a known problem :-( Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
On 29 Sep 2000, Gunnar R|nning wrote: > [feel stupid replying to myself...] > > Gunnar R|nning <gunnar@candleweb.no> writes: > > > > at org.postgresql.jdbc2.ResultSet.getObject(ResultSet.java:789) > > > > OK, I found the problem. I will post a fixed version later today. The > > problem was that getObject() executed Field.getSQLType() which in turn > > executed Connection.ExecSQL(). I modified ExecSQL to deallocate the cached > > byte arrays on entry, because I believed it only were called by > > Statement.execute() methods. I guess I should move the deallocation into > > the Statement classes instead, as that is were it really belongs. > > > > I interpret the JDBC spec. to say that only one ResultSet will be open > > per. Statement, but one Connection canm have several statements with one > > result set each. > > > > This does of course imply that arrays should be cached on a > ResultSet/Statement basis instead of on PGStream as it is done now. Do > anybody have good suggestions on how to implement this ? > > Approach 1: > The cache is now only per Connection, maybe we should a global pool of free > byte arrays instead ? > Cons : > This would probably mean that we need to add more > synchronization to ensure safe access by concurrent threads and could > therefore lead to poorer performance and concurrency. Our concurrency is done by locking against the pg_Stream object. As there is only one per connection this works (as it prevents the network protocol from getting messed up). The pool of free byte arrays don't need to be locked in this way. As I see it, we would need a single static class that holds two stacks, one of free arrays, and the other of arrays in use. The methods used to move an array from one stack to another need to be syncronized so they are atomic. The only thing to think of here, is how to deal in reaping arrays that are no longer used, but who's Connection's have died abnormally. > Pros : Possibly lower memory consumption and higher performance in some > cases(when you find free byte arrays in the global pool). If your > application is not pooling connections, but recreating connections it would > also benefit performance wise from this approach. > > Approach 2: > Another solution would be have the cache be per connection but associate a > pool of used byte arrays to each resultset/statement and deallocate these > on resultset.close()/statement.close(). Some people don't call close() unfortunately. Bad practice as you should. I like this idea as it keeps things simple in the source - the close() methods are supposed to free up any resources used, and this is what's being done. > Pros: Faster for the typical web application that uses pooled connections, > because this approach would require less synchronization. > Cons: Higher memory consumption. How many people use pooled connections? I don't, but then most of my Java stuff is application based, not web/servlet based. Approach 1 does have some advantages for pooled connections but would be a problem for users who use single connections per VM. Even worse, would be those using applets (some still use JDBC direct from applets), and the limiting factor there is the size of the driver. > Either of these two approaches would probably require some reorganization > of how the driver works. Anything of this nature always does. Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
Peter Mount <peter@retep.org.uk> writes:
> Our concurrency is done by locking against the pg_Stream object. As there
> is only one per connection this works (as it prevents the network protocol
> from getting messed up).
>
> The pool of free byte arrays don't need to be locked in this way. As I see
> it, we would need a single static class that holds two stacks, one of free
> arrays, and the other of arrays in use. The methods used to move an array
> from one stack to another need to be syncronized so they are atomic.
>
> The only thing to think of here, is how to deal in reaping arrays that are
> no longer used, but who's Connection's have died abnormally.
I agree that a global stack for free byte arrays could be preferable, as
long synchronization doesn't add to much overhead. But because of the
problem you mention with connections dying abnormally or other error
situtations we need to store the used byte arrays in a place where that
allows the garbage collector to cleanup after us. I think the ResultSet
or Statement would be the right place to store these.
>
> How many people use pooled connections? I don't, but then most of my Java
> stuff is application based, not web/servlet based.
>
> Approach 1 does have some advantages for pooled connections but would be a
> problem for users who use single connections per VM. Even worse, would be
> those using applets (some still use JDBC direct from applets), and the
> limiting factor there is the size of the driver.
Pooled connections are very common in the web world, but I would think that
it makes sense for many applications to reuse connections as well. Maybe I
should add some configuration parameters, so the user can tune the
implementation or even choose implementation to suite a particular
application ?
Regards,
Gunnar
On 2 Oct 2000, Gunnar R|nning wrote: > Peter Mount <peter@retep.org.uk> writes: > > > Our concurrency is done by locking against the pg_Stream object. As there > > is only one per connection this works (as it prevents the network protocol > > from getting messed up). > > > > The pool of free byte arrays don't need to be locked in this way. As I see > > it, we would need a single static class that holds two stacks, one of free > > arrays, and the other of arrays in use. The methods used to move an array > > from one stack to another need to be syncronized so they are atomic. > > > > The only thing to think of here, is how to deal in reaping arrays that are > > no longer used, but who's Connection's have died abnormally. > > I agree that a global stack for free byte arrays could be preferable, as > long synchronization doesn't add to much overhead. But because of the > problem you mention with connections dying abnormally or other error > situtations we need to store the used byte arrays in a place where that > allows the garbage collector to cleanup after us. I think the ResultSet > or Statement would be the right place to store these. > > > > > How many people use pooled connections? I don't, but then most of my Java > > stuff is application based, not web/servlet based. > > > > Approach 1 does have some advantages for pooled connections but would be a > > problem for users who use single connections per VM. Even worse, would be > > those using applets (some still use JDBC direct from applets), and the > > limiting factor there is the size of the driver. > > Pooled connections are very common in the web world, but I would think that > it makes sense for many applications to reuse connections as well. Maybe I > should add some configuration parameters, so the user can tune the > implementation or even choose implementation to suite a particular > application ? The best place for this is in org.postgresql.Driver, as that's where you can add the DriverParameter's code (some old stubs exist, under there commented out). We must also look at the javax.sql stuff to see how its done there also. Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
Peter Mount <peter@retep.org.uk> writes: > > Anything of this nature always does. > Okay, I have some new code in place that hopefully should work better. I couldn't produce a full patch using cvs diff -c this time since I have created new files and anonymous cvs usage doesn't allow you to adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz The new files that should be added are : ? org/postgresql/PGStatement.java ? org/postgresql/ObjectPool.java ? org/postgresql/ObjectPoolFactory.java There is now a global static pool of free byte arrays and used byte arrays connected to a statement object. This is the role of the new PGStatement class. Access to the global free array is synchronized, while we rely on the PG_Stream synchronization for the used array. My measurements show that the perfomance boost on this code is not quite as big as my last shot, but it is still an improvement. Maybe some of the difference is due to the new synchronization on the global array. I think I will look into choosing between on a connection level and global level. I would also appreciate it if anybody would test the new driver. My test platform is RedHat Linux 6.2, and since the effect of these changes could be very dependent on the actual platform and JVM tests from other platforms would be very nice to see. I have a precompiled version at : http://www.candleweb.no/~gunnar/projects/pgsql/postgresql.jar. I have also started experimented with improving the performance of the various conversions. The problem here is ofcourse related handle the various encodings. One thing I found to speed up ResultSet.getInt() a lot was to do custom conversion on the byte array into int instead of going through the getString() to do the conversion. But I'm unsure if this is portable, can we assume that a digit never can be represented by more than one byte ? It works fine in my iso-latin-8859-1 environment, but what about other environments ? Maybe we could provide different ResultSet implementations depending on the encoding used or delegate some methods of the result set to an "converter class". Check the org/postgresql/jdbc2/FastResultSet.java in the tarball above to see the modified getInt() method. Regards, Gunnar
Applied, and new files added. > Peter Mount <peter@retep.org.uk> writes: > > > > > Anything of this nature always does. > > > > Okay, I have some new code in place that hopefully should work better. I > couldn't produce a full patch using cvs diff -c this time since I have > created new files and anonymous cvs usage doesn't allow you to > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > The new files that should be added are : > > ? org/postgresql/PGStatement.java > ? org/postgresql/ObjectPool.java > ? org/postgresql/ObjectPoolFactory.java > > There is now a global static pool of free byte arrays and used byte arrays > connected to a statement object. This is the role of the new PGStatement > class. Access to the global free array is synchronized, while we rely on > the PG_Stream synchronization for the used array. > > My measurements show that the perfomance boost on this code is not quite as > big as my last shot, but it is still an improvement. Maybe some of the > difference is due to the new synchronization on the global array. I think I > will look into choosing between on a connection level and global level. > > > I would also appreciate it if anybody would test the new driver. My test > platform is RedHat Linux 6.2, and since the effect of these changes could > be very dependent on the actual platform and JVM tests from other platforms > would be very nice to see. I have a precompiled version at : > > http://www.candleweb.no/~gunnar/projects/pgsql/postgresql.jar. > > > I have also started experimented with improving the performance of the > various conversions. The problem here is ofcourse related handle the > various encodings. One thing I found to speed up ResultSet.getInt() a lot > was to do custom conversion on the byte array into int instead of going > through the getString() to do the conversion. But I'm unsure if this is > portable, can we assume that a digit never can be represented by more than > one byte ? It works fine in my iso-latin-8859-1 environment, but what about > other environments ? Maybe we could provide different ResultSet > implementations depending on the encoding used or delegate some methods of > the result set to an "converter class". > > Check the org/postgresql/jdbc2/FastResultSet.java in the tarball above to > see the modified getInt() method. > > Regards, > > Gunnar > -- 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 <pgman@candle.pha.pa.us> writes:
> Applied, and new files added.
>
Scares me ;-) I just wanted to get some feedback and testers for this code
before actually having it applied. I can also imagine Peter might have some
comments on structure etc. since he got a lot better overview of the JDBC
code as whole.
Well did anyone review the changes or test them out ? The application I'm
testing is hardly representative for all applications, so there may still
be some bugs left in there.
Regards,
Gunnar
On 5 Oct 2000, Gunnar R|nning wrote: > Peter Mount <peter@retep.org.uk> writes: > > > > > Anything of this nature always does. > > > > Okay, I have some new code in place that hopefully should work better. I > couldn't produce a full patch using cvs diff -c this time since I have > created new files and anonymous cvs usage doesn't allow you to > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz Got it. Don't worry about the diff, I'll merge it in manually. At this stage, it will be easier, and less prone to generating bugs with the changes I've got here. > The new files that should be added are : > > ? org/postgresql/PGStatement.java > ? org/postgresql/ObjectPool.java > ? org/postgresql/ObjectPoolFactory.java > > There is now a global static pool of free byte arrays and used byte arrays > connected to a statement object. This is the role of the new PGStatement > class. Access to the global free array is synchronized, while we rely on > the PG_Stream synchronization for the used array. Good. That should keep the thread safety intact. > My measurements show that the perfomance boost on this code is not quite as > big as my last shot, but it is still an improvement. Maybe some of the > difference is due to the new synchronization on the global array. I think I > will look into choosing between on a connection level and global level. > > > I would also appreciate it if anybody would test the new driver. My test > platform is RedHat Linux 6.2, and since the effect of these changes could > be very dependent on the actual platform and JVM tests from other platforms > would be very nice to see. I have a precompiled version at : SuSE 6.4 (got 7.0 to install shortly). As soon as I recover my VMWare licence, I'll be able to get the other OS's up and running as well. > I have also started experimented with improving the performance of the > various conversions. The problem here is ofcourse related handle the > various encodings. One thing I found to speed up ResultSet.getInt() a lot > was to do custom conversion on the byte array into int instead of going > through the getString() to do the conversion. But I'm unsure if this is > portable, can we assume that a digit never can be represented by more than > one byte ? It works fine in my iso-latin-8859-1 environment, but what about > other environments ? Maybe we could provide different ResultSet > implementations depending on the encoding used or delegate some methods of > the result set to an "converter class". We would have to have a rethink on how ResultSet is implemented. Currently we have a common parent class org.postgresql.ResultSet which is common to both jdbc1 & jdbc2 drivers. However, this class is abstract because we then extend it for each of the two drivers because of the extra methods. However, remember I'm planning implementing Scrollable ResultSet's. I was going to do it by splitting the org.postgresql.jdbc2.ResultSet class, two, one for the current non-scrollable version, and one for the scrollable one. ie: org.postgresql.ResultSet -+--> org.postgresql.jdbc1.ResultSet (JDBC1 only) | | org.postgresql.jdbc2.ResultSet (JDBC2 only, common | conversions here) | +-------------+------------------------+ | | org.postgresql.jdbc2.NSResultSet org.postgresql.jdbc2.ScrResultSet Current Non Scrollable ResultSet New Scrollable ResultSet Implementation Now the conversion stuff would have to go in the org.postgresql.ResultSet class if it's to be used in both drivers, or in org.postgresql.jdbc2.ResultSet class for jdbc2 only. The problem I don't want to see is having conversion code duplicated, which we still have in places, ResultSet being a good example. Does this make sense? > Check the org/postgresql/jdbc2/FastResultSet.java in the tarball above to > see the modified getInt() method. Will do. PS: Sorry for the late reply, but my illness has knocked me out for the last few days, so I've only just got to sit down to read my mail. Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
On Sun, 8 Oct 2000, Bruce Momjian wrote: > Applied, and new files added. Eeek! you beat me to it again. At this rate, my appendments/changes won't get into CVS ;-) Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
On 9 Oct 2000, Gunnar R|nning wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > Applied, and new files added. > > > > Scares me ;-) I just wanted to get some feedback and testers for this code > before actually having it applied. I can also imagine Peter might have some > comments on structure etc. since he got a lot better overview of the JDBC > code as whole. There's going to be some structure changes happening, mainly with the conversion/optimisation work Gunnar's working on, but also with the Scrollable ResultSet and regression testing stuff I'm working on at the moment. > Well did anyone review the changes or test them out ? The application I'm > testing is hardly representative for all applications, so there may still > be some bugs left in there. Which is why I need to get the regression stuff sorted out as soon as I can. Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
> On 5 Oct 2000, Gunnar R|nning wrote: > > > Peter Mount <peter@retep.org.uk> writes: > > > > > > > > Anything of this nature always does. > > > > > > > Okay, I have some new code in place that hopefully should work better. I > > couldn't produce a full patch using cvs diff -c this time since I have > > created new files and anonymous cvs usage doesn't allow you to > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > Got it. Don't worry about the diff, I'll merge it in manually. At this > stage, it will be easier, and less prone to generating bugs with the > changes I've got here. OK, I can back it out in 5 minutes. It is very easy. Would you like it backed out? -- 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
On Mon, 9 Oct 2000, Bruce Momjian wrote: > > On 5 Oct 2000, Gunnar R|nning wrote: > > > > > Peter Mount <peter@retep.org.uk> writes: > > > > > > > > > > > Anything of this nature always does. > > > > > > > > > > Okay, I have some new code in place that hopefully should work better. I > > > couldn't produce a full patch using cvs diff -c this time since I have > > > created new files and anonymous cvs usage doesn't allow you to > > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > > > Got it. Don't worry about the diff, I'll merge it in manually. At this > > stage, it will be easier, and less prone to generating bugs with the > > changes I've got here. > > OK, I can back it out in 5 minutes. It is very easy. Would you like it > backed out? If you could please, as it would make like easier for me to import my changes, and also as Gunnar said, it's still being tested at the moment. Thank, Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
> On Mon, 9 Oct 2000, Bruce Momjian wrote: > > > > On 5 Oct 2000, Gunnar R|nning wrote: > > > > > > > Peter Mount <peter@retep.org.uk> writes: > > > > > > > > > > > > > > Anything of this nature always does. > > > > > > > > > > > > > Okay, I have some new code in place that hopefully should work better. I > > > > couldn't produce a full patch using cvs diff -c this time since I have > > > > created new files and anonymous cvs usage doesn't allow you to > > > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > > > > > Got it. Don't worry about the diff, I'll merge it in manually. At this > > > stage, it will be easier, and less prone to generating bugs with the > > > changes I've got here. > > > > OK, I can back it out in 5 minutes. It is very easy. Would you like it > > backed out? > > If you could please, as it would make like easier for me to import my > changes, and also as Gunnar said, it's still being tested at the moment. OK, backed out. Thanks for the heads-up. -- 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> writes: > > couldn't produce a full patch using cvs diff -c this time since I have > > created new files and anonymous cvs usage doesn't allow you to > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > Got it. Don't worry about the diff, I'll merge it in manually. At this > stage, it will be easier, and less prone to generating bugs with the > changes I've got here. Hmm. I just grabbed the latest source from CVS and it seems to be that my first version and not the latest one who is merged in(or maybe it was Bruce who did that...). Anyway the version in CVS is buggy and should be replaced with the version from October 5 pointed to in the mail quoted above. The current changes in CVS needs to be backed out one way or the other. If you need any help, I can create a new version based on the current CVS image. regards, Gunnar
On 18 Oct 2000, Gunnar R|nning wrote: > Peter Mount <peter@retep.org.uk> writes: > > > > > couldn't produce a full patch using cvs diff -c this time since I have > > > created new files and anonymous cvs usage doesn't allow you to > > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > > > Got it. Don't worry about the diff, I'll merge it in manually. At this > > stage, it will be easier, and less prone to generating bugs with the > > changes I've got here. > > Hmm. I just grabbed the latest source from CVS and it seems to be that my > first version and not the latest one who is merged in(or maybe it was Bruce > who did that...). Anyway the version in CVS is buggy and should be replaced > with the version from October 5 pointed to in the mail quoted above. The > current changes in CVS needs to be backed out one way or the other. Hmmm, I'll check it here, as I thought I had the October 5th one in. > If you need any help, I can create a new version based on the > current CVS image. It shouldn't be that difficult. I'll let you know if there's a problem. Peter -- Peter T Mount peter@retep.org.uk http://www.retep.org.uk PostgreSQL JDBC Driver http://www.retep.org.uk/postgres/ Java PDF Generator http://www.retep.org.uk/pdf/
I am just going through my mailbox, and see that I applied this on October 8: --------------------------------------------------------------------------- revision 1.7 date: 2000/10/08 19:37:54; author: momjian; state: Exp; lines: +37-17 Okay, I have some new code in place that hopefully should work better. I couldn't produce a full patch using cvs diff -c this time since I have created new files and anonymous cvs usage doesn't allow you to adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz --------------------------------------------------------------------------- Can you confirm that it is OK now? Your email from October 18th below seems to indicate it is not correct. Can I get a diff against the current CVS source that I can apply? Thanks. Sorry for the confusion. --------------------------------------------------------------------------- > Peter Mount <peter@retep.org.uk> writes: > > > > > couldn't produce a full patch using cvs diff -c this time since I have > > > created new files and anonymous cvs usage doesn't allow you to > > > adds. I'm supplying the modified src/interfaces/jdbc as a tarball at : > > > http://www.candleweb.no/~gunnar/projects/pgsql/postgres-jdbc-2000-10-05.tgz > > > > Got it. Don't worry about the diff, I'll merge it in manually. At this > > stage, it will be easier, and less prone to generating bugs with the > > changes I've got here. > > Hmm. I just grabbed the latest source from CVS and it seems to be that my > first version and not the latest one who is merged in(or maybe it was Bruce > who did that...). Anyway the version in CVS is buggy and should be replaced > with the version from October 5 pointed to in the mail quoted above. The > current changes in CVS needs to be backed out one way or the other. > > If you need any help, I can create a new version based on the > current CVS image. > > regards, > > Gunnar > > -- 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 <pgman@candle.pha.pa.us> writes:
> Can you confirm that it is OK now? Your email from October 18th below
> seems to indicate it is not correct. Can I get a diff against the
> current CVS source that I can apply? Thanks. Sorry for the confusion.
It wasn't okay before I left to Kenya on 10 december. I notified Peter
about this before I went down there and I just came back two days ago, so
I'm not yet up to speed on the modifications.
... cvs update ...
The cvs update confirms that it is still _not_ okay. It is the first buggy
version of the code that is in there still. I will supply a new patch based
on the current CVS. Hopefully I will get some time tonight and on Sunday to
complete the patch.
regards,
Gunnar
Thanks you so much. I am sorry for the mixup. We will get it in there ASAP when it arrives. > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > > Can you confirm that it is OK now? Your email from October 18th below > > seems to indicate it is not correct. Can I get a diff against the > > current CVS source that I can apply? Thanks. Sorry for the confusion. > > > It wasn't okay before I left to Kenya on 10 december. I notified Peter > about this before I went down there and I just came back two days ago, so > I'm not yet up to speed on the modifications. > > ... cvs update ... > > The cvs update confirms that it is still _not_ okay. It is the first buggy > version of the code that is in there still. I will supply a new patch based > on the current CVS. Hopefully I will get some time tonight and on Sunday to > complete the patch. > > regards, > > Gunnar > > -- 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