Обсуждение: Re: [PATCHES] Re: [INTERFACES] Patch for JDBC timestamp problems

What was the conclusion of this discussion?  Do we leave it static?

> Barry Lind wrote:
> >
> > Attached is a set of patches for a couple of bugs dealing with
> > timestamps in JDBC.
> >
> > Bug#1) Incorrect timestamp stored in DB if client timezone different
> > than DB.
> >
> > The buggy implementation of setTimestamp() in PreparedStatement simply
> > used the toString() method of the java.sql.Timestamp object to convert
> > to a string to send to the database.  The format of this is yyyy-MM-dd
> > hh:mm:ss.SSS which doesn't include any timezone information.  Therefore
> > the DB assumes its timezone since none is specified.  That is OK if the
> > timezone of the client and server are the same, however if they are
> > different the wrong timestamp is received by the server.  For example if
> > the client is running in timezone GMT and wants to send the timestamp
> > for noon to a server running in PST (GMT-8 hours), then the server will
> > receive 2000-01-12 12:00:00.0 and interprete it as 2000-01-12
> > 12:00:00-08 which is 2000-01-12 04:00:00 in GMT.  The fix is to send a
> > format to the server that includes the timezone offset.  For simplicity
> > sake the fix uses a SimpleDateFormat object with its timezone set to GMT
> > so that '+00' can be used as the timezone for postgresql.  This is done
> > as SimpleDateFormat doesn't support formating timezones in the way
> > postgresql expects.
> >
> > Bug#2) Incorrect handling of partial seconds in getting timestamps from
> > the DB
> >
> > When the SimpleDateFormat object parses a string with a format like
> > yyyy-MM-dd hh:mm:ss.SS it expects the fractional seconds to be three
> > decimal places (time precision in java is miliseconds = three decimal
> > places).  This seems like a bug in java to me, but it is unlikely to be
> > fixed anytime soon, so the postgresql code needed modification to
> > support the java behaviour.  So for example a string of '2000-01-12
> > 12:00:00.12-08' coming from the database was being converted to a
> > timestamp object with a value of 2000-01-12 12:00:00.012GMT-08:00.  The
> > fix was to check for a '.' in the string and if one is found append on
> > an extra zero to the fractional seconds part.
> >
> > Bug#3) Performance problems
> >
> > In fixing the above two bugs, I noticed some things that could be
> > improved.  In PreparedStatement.setTimestamp(),
> > PreparedStatement.setDate(), ResultSet.getTimestamp(), and
> > ResultSet.getDate() these methods were creating a new SimpleDateFormat
> > object everytime they were called.  To avoid this unnecessary object
> > creation overhead, I changed the code to use static variables for
> > keeping a single instance of the needed formating objects.
> > Also the code used the + operator for string concatenation.  As everyone
> > should know this is very inefficient and the use of StringBuffers is
> > prefered.
> >
>
> While the java spec says that a+b+c should be converted into
> a.concat(b.toString()).concat(c.toString())
>  probably every single java compiler (including javac) uses
> StringBuffers.  The only case where it is an advantage to use your own
> stringBuffer is in a case like:
>
> StringBuffer sb = new StringBuffer("blah");
> sb.append(a+b+c);
>
> Since that would create a temporary StringBuffer to calculate a+b+c just
> to append to the original sb it it might be better to explictly append
> a,b,and c.
>
>
> Using static SimpleDateFormats will probably not cause threading
> issues.  Common sense says that if the set methods are never called on
> them there will be no state change that my cause sync problems.  But the
> spec doesn't garuntee it.  Personally I would have no problem using
> static SimpleDateFormats if this were my code.
>
>
>
> --
> Joseph Shraibman
> jks@selectacast.net
> Increase signal to noise ratio.  http://www.targabot.com
>


--
  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

No, it cannot be static.

Bruce Momjian wrote:
>
> What was the conclusion of this discussion?  Do we leave it static?
>
> > Barry Lind wrote:
> > >
> > > Attached is a set of patches for a couple of bugs dealing with
> > > timestamps in JDBC.
> > >
> > > Bug#1) Incorrect timestamp stored in DB if client timezone different
> > > than DB.
> > >
> > > The buggy implementation of setTimestamp() in PreparedStatement simply
> > > used the toString() method of the java.sql.Timestamp object to convert
> > > to a string to send to the database.  The format of this is yyyy-MM-dd
> > > hh:mm:ss.SSS which doesn't include any timezone information.  Therefore
> > > the DB assumes its timezone since none is specified.  That is OK if the
> > > timezone of the client and server are the same, however if they are
> > > different the wrong timestamp is received by the server.  For example if
> > > the client is running in timezone GMT and wants to send the timestamp
> > > for noon to a server running in PST (GMT-8 hours), then the server will
> > > receive 2000-01-12 12:00:00.0 and interprete it as 2000-01-12
> > > 12:00:00-08 which is 2000-01-12 04:00:00 in GMT.  The fix is to send a
> > > format to the server that includes the timezone offset.  For simplicity
> > > sake the fix uses a SimpleDateFormat object with its timezone set to GMT
> > > so that '+00' can be used as the timezone for postgresql.  This is done
> > > as SimpleDateFormat doesn't support formating timezones in the way
> > > postgresql expects.
> > >
> > > Bug#2) Incorrect handling of partial seconds in getting timestamps from
> > > the DB
> > >
> > > When the SimpleDateFormat object parses a string with a format like
> > > yyyy-MM-dd hh:mm:ss.SS it expects the fractional seconds to be three
> > > decimal places (time precision in java is miliseconds = three decimal
> > > places).  This seems like a bug in java to me, but it is unlikely to be
> > > fixed anytime soon, so the postgresql code needed modification to
> > > support the java behaviour.  So for example a string of '2000-01-12
> > > 12:00:00.12-08' coming from the database was being converted to a
> > > timestamp object with a value of 2000-01-12 12:00:00.012GMT-08:00.  The
> > > fix was to check for a '.' in the string and if one is found append on
> > > an extra zero to the fractional seconds part.
> > >
> > > Bug#3) Performance problems
> > >
> > > In fixing the above two bugs, I noticed some things that could be
> > > improved.  In PreparedStatement.setTimestamp(),
> > > PreparedStatement.setDate(), ResultSet.getTimestamp(), and
> > > ResultSet.getDate() these methods were creating a new SimpleDateFormat
> > > object everytime they were called.  To avoid this unnecessary object
> > > creation overhead, I changed the code to use static variables for
> > > keeping a single instance of the needed formating objects.
> > > Also the code used the + operator for string concatenation.  As everyone
> > > should know this is very inefficient and the use of StringBuffers is
> > > prefered.
> > >
> >
> > While the java spec says that a+b+c should be converted into
> > a.concat(b.toString()).concat(c.toString())
> >  probably every single java compiler (including javac) uses
> > StringBuffers.  The only case where it is an advantage to use your own
> > stringBuffer is in a case like:
> >
> > StringBuffer sb = new StringBuffer("blah");
> > sb.append(a+b+c);
> >
> > Since that would create a temporary StringBuffer to calculate a+b+c just
> > to append to the original sb it it might be better to explictly append
> > a,b,and c.
> >
> >
> > Using static SimpleDateFormats will probably not cause threading
> > issues.  Common sense says that if the set methods are never called on
> > them there will be no state change that my cause sync problems.  But the
> > spec doesn't garuntee it.  Personally I would have no problem using
> > static SimpleDateFormats if this were my code.
> >
> >
> >
> > --
> > Joseph Shraibman
> > jks@selectacast.net
> > Increase signal to noise ratio.  http://www.targabot.com
> >
>
> --
>   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

--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio.  http://www.targabot.com

Joseph Shraibman wrote:
>
> No, it cannot be static.
>

Well actually there could be static variables, but they could not be
used directly.  It might pay just to keep them around to clone() so we
don't have to reconstruct them for every method that uses them.  But the
performance gain would be minimal and would add a bit of confusion to
the code, so I don't suggest it.


--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio.  http://www.targabot.com

OK, so you are saying I should apply the patch that removes the the
static.  I will do it now.� Thanks.  That clears up a few open emails
for me.


> Joseph Shraibman wrote:
> >
> > No, it cannot be static.
> >
>
> Well actually there could be static variables, but they could not be
> used directly.  It might pay just to keep them around to clone() so we
> don't have to reconstruct them for every method that uses them.  But the
> performance gain would be minimal and would add a bit of confusion to
> the code, so I don't suggest it.
>
>
> --
> Joseph Shraibman
> jks@selectacast.net
> Increase signal to noise ratio.  http://www.targabot.com
>


--
  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 Wed, 24 Jan 2001, Joseph Shraibman wrote:

> > What was the conclusion of this discussion?  Do we leave it static?
>
> No, it cannot be static.

As I see it we have three possible solutions to this problems.

a) Just stop it being static, each PreparedStatement gets a new
   instantiation (I think this is what we've done for now).
b) static ThreadLocal, each Thread gets one instantiation.
c) An Object Pool (possibly using SoftReferences to stop memory bloat),
   slightly more difficult to code, a lot less instantiations, much
   better performance.

If people agree that either 'b' or 'c' is a better solution, I'll go ahead
and implement it.

Michael Stephenson      mstephenson@openworld.co.uk
Developer   -    Web Applications    -   Open World
Tel: +44 1225 444 950         Fax: +44 1225 336 738

Michael Stephenson wrote:
>
> On Wed, 24 Jan 2001, Joseph Shraibman wrote:
>
> > > What was the conclusion of this discussion?  Do we leave it static?
> >
> > No, it cannot be static.
>
> As I see it we have three possible solutions to this problems.
>
> a) Just stop it being static, each PreparedStatement gets a new
>    instantiation (I think this is what we've done for now).
> b) static ThreadLocal, each Thread gets one instantiation.

But I think some people are still using java 1.1.x and they don't have
ThreadLocal.

> c) An Object Pool (possibly using SoftReferences to stop memory bloat),
>    slightly more difficult to code, a lot less instantiations, much
>    better performance.

But there are so many different ones used and you would have to make a
pool for each one.

>
> If people agree that either 'b' or 'c' is a better solution, I'll go ahead
> and implement it.

I don't agree.  Part of java performance is memory usage.  Introducing
memory pools means keeping objects around forever even if they are used
once.  In tests I've done to see if using static variables are usful
with string parsing I've found that gain is minimal.  Any call to
postgress will be disk and network bound and trying to introduce a pool
will only complicate things.

--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio.  http://www.targabot.com

> > b) static ThreadLocal, each Thread gets one instantiation.
>
> But I think some people are still using java 1.1.x and they don't have
> ThreadLocal.

Then they can stick with using one instantiation per object, this is
really pulling our performance down, do some tests.

> > c) An Object Pool (possibly using SoftReferences to stop memory bloat),
> >    slightly more difficult to code, a lot less instantiations, much
> >    better performance.
>
> But there are so many different ones used and you would have to make a
> pool for each one.

SimpleDateFormat has a HUGE instantiation penalty, bigger than any other
object in the jdk I can think of.

> > If people agree that either 'b' or 'c' is a better solution, I'll go ahead
> > and implement it.
>
> I don't agree.  Part of java performance is memory usage.  Introducing
> memory pools means keeping objects around forever even if they are used
> once.

This is why I would use SoftReferences, this means when memory use is too
high the virtual machine will just garbage collect them.

Again, this is a 1.2 or greater solution.

> In tests I've done to see if using static variables are usful
> with string parsing I've found that gain is minimal.

Strings have a low cost of instantiation, on my machine they take 0.0028
compared to 0.58ms (Sun JDK1.2.2 linux, P2 733).

This *IS* a big cost. Using SoftReferences in an object pool my initial
tests are going in the same order of magnitude as the String
instantiations (the main cost is the synchronisation).

> Any call to
> postgress will be disk and network bound and trying to introduce a pool
> will only complicate things.

There are already other Object pools for other things.

Michael Stephenson      mstephenson@openworld.co.uk
Developer   -    Web Applications    -   Open World
Tel: +44 1225 444 950         Fax: +44 1225 336 738

Quoting Joseph Shraibman <jks@selectacast.net>:

> Michael Stephenson wrote:
> >
> > On Wed, 24 Jan 2001, Joseph Shraibman wrote:
> >
> > > > What was the conclusion of this discussion?  Do we leave it
> static?
> > >
> > > No, it cannot be static.
> >
> > As I see it we have three possible solutions to this problems.
> >
> > a) Just stop it being static, each PreparedStatement gets a new
> >    instantiation (I think this is what we've done for now).
> > b) static ThreadLocal, each Thread gets one instantiation.
>
> But I think some people are still using java 1.1.x and they don't have
> ThreadLocal.

True, except the 1.1 & 1.2 implementations are different packages, so you
simply don't do ThreadLocal in 1.1.x

> > c) An Object Pool (possibly using SoftReferences to stop memory
> bloat),
> >    slightly more difficult to code, a lot less instantiations, much
> >    better performance.
>
> But there are so many different ones used and you would have to make a
> pool for each one.
>
> >
> > If people agree that either 'b' or 'c' is a better solution, I'll go
> ahead
> > and implement it.
>
> I don't agree.  Part of java performance is memory usage.  Introducing
> memory pools means keeping objects around forever even if they are used
> once.  In tests I've done to see if using static variables are usful
> with string parsing I've found that gain is minimal.  Any call to
> postgress will be disk and network bound and trying to introduce a pool
> will only complicate things.
>
> --
> Joseph Shraibman
> jks@selectacast.net
> Increase signal to noise ratio.  http://www.targabot.com
>



--
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/

Peter T Mount wrote:
>
> Quoting Joseph Shraibman <jks@selectacast.net>:
>
> > Michael Stephenson wrote:
> > >
> > > On Wed, 24 Jan 2001, Joseph Shraibman wrote:
> > >
> > > > > What was the conclusion of this discussion?  Do we leave it
> > static?
> > > >
> > > > No, it cannot be static.
> > >
> > > As I see it we have three possible solutions to this problems.
> > >
> > > a) Just stop it being static, each PreparedStatement gets a new
> > >    instantiation (I think this is what we've done for now).
> > > b) static ThreadLocal, each Thread gets one instantiation.
> >
> > But I think some people are still using java 1.1.x and they don't have
> > ThreadLocal.
>
> True, except the 1.1 & 1.2 implementations are different packages, so you
> simply don't do ThreadLocal in 1.1.x
>

Umm, not exactly.  You can use jdbc 2 with java 1.1.x by downloading a
seperate package.  I'm not sure how many people do this, but I imagine
there are a few.



--
Joseph Shraibman
jks@selectacast.net
Increase signal to noise ratio.  http://www.targabot.com

At 09:39 30/01/01 +0000, Michael Stephenson wrote:
> > > b) static ThreadLocal, each Thread gets one instantiation.
> >
> > But I think some people are still using java 1.1.x and they don't have
> > ThreadLocal.
>
>Then they can stick with using one instantiation per object, this is
>really pulling our performance down, do some tests.

Yes, and as they are different classes (only built as required) this isn't
a problem.


> > > c) An Object Pool (possibly using SoftReferences to stop memory bloat),
> > >    slightly more difficult to code, a lot less instantiations, much
> > >    better performance.
> >
> > But there are so many different ones used and you would have to make a
> > pool for each one.
>
>SimpleDateFormat has a HUGE instantiation penalty, bigger than any other
>object in the jdk I can think of.
>
> > > If people agree that either 'b' or 'c' is a better solution, I'll go
> ahead
> > > and implement it.
> >
> > I don't agree.  Part of java performance is memory usage.  Introducing
> > memory pools means keeping objects around forever even if they are used
> > once.
>
>This is why I would use SoftReferences, this means when memory use is too
>high the virtual machine will just garbage collect them.
>
>Again, this is a 1.2 or greater solution.
>
> > In tests I've done to see if using static variables are usful
> > with string parsing I've found that gain is minimal.
>
>Strings have a low cost of instantiation, on my machine they take 0.0028
>compared to 0.58ms (Sun JDK1.2.2 linux, P2 733).
>
>This *IS* a big cost. Using SoftReferences in an object pool my initial
>tests are going in the same order of magnitude as the String
>instantiations (the main cost is the synchronisation).
>
> > Any call to
> > postgress will be disk and network bound and trying to introduce a pool
> > will only complicate things.
>
>There are already other Object pools for other things.

Ok, have you got any quick examples of this? I would like to nail the
Timestamp problems once and for all, so now's the time to sort this out.

Peter

At 17:23 30/01/01 -0500, Joseph Shraibman wrote:
>Peter T Mount wrote:
> >
> > Quoting Joseph Shraibman <jks@selectacast.net>:
> >
> > > Michael Stephenson wrote:
> > > >
> > > > On Wed, 24 Jan 2001, Joseph Shraibman wrote:
> > > >
> > > > > > What was the conclusion of this discussion?  Do we leave it
> > > static?
> > > > >
> > > > > No, it cannot be static.
> > > >
> > > > As I see it we have three possible solutions to this problems.
> > > >
> > > > a) Just stop it being static, each PreparedStatement gets a new
> > > >    instantiation (I think this is what we've done for now).
> > > > b) static ThreadLocal, each Thread gets one instantiation.
> > >
> > > But I think some people are still using java 1.1.x and they don't have
> > > ThreadLocal.
> >
> > True, except the 1.1 & 1.2 implementations are different packages, so you
> > simply don't do ThreadLocal in 1.1.x
> >
>
>Umm, not exactly.  You can use jdbc 2 with java 1.1.x by downloading a
>seperate package.  I'm not sure how many people do this, but I imagine
>there are a few.

A subset maybe, but there are some bits that reference classes introduced
in JDK1.2.x and later, so would simply not work with 1.1.

Peter