Re: [PATCHES] Re: High Memory Usage Patch -- Disregard my last message

Hi all,

We're seeing one usage of a database use significant more memory than
we're used to and I'm hoping for some hints at what Postgres allocates
memory for. BTW, we're running PG 7.1 on Linux.

First of all we see some of the PG processes use as much as 80 MB
(according to top and ps). This seems to be allocated during a query
(SELECT). Unfortunately, we haven't been able to discover exactly what query
is doing this. Anyway, the first problem is this high memory allocation.
Considering the database is only about 80 MB total (according to disk usage
of the entire data directory), allocating 80 MB seems excessive.
The second significant problem is that this memory allocation does not go
down once the SELECT is completed. According to ps the process in IDLE yet
the memory usage remains the same. If it was only one process then that
would be fine but we see this happening with as many as 3 processes
simultaneously and the box only has 256 MB RAM (although we plan to up that
to 512 soon).

So, if anyone has any general idea as to when PG allocates memory (for
example, I assume and hope, the PG process wouldn't allocate enough memory
to have the entire result set in memory) I'd appreciate the info. Any other
clues are also welcome.
We have other, near identical, instances of this database which do not
show this behavior. The particular queries being done will differ between
them and one particular table is about 10 times larger (150,000 rows instead
of 15,000) in this db as compared to the others but I don't see either of
those factors causing this large a difference. Well, I don't know, how much
memory do semi-complex queries take?

Thanks,

--Rainer

"Rainer Mager" <rmager(at)vgkk(dot)com> writes:
> First of all we see some of the PG processes use as much as 80 MB
> (according to top and ps). This seems to be allocated during a query
> (SELECT). Unfortunately, we haven't been able to discover exactly what query
> is doing this.

If you can catch the thing while the memory usage is going up, you could
attach to the backend with gdb and examine debug_query_string to see
what the current query is. (Note: I think you need 7.1.1 or later to
have this variable available. You should be on 7.1.2 in any case, just
on general principles.) Otherwise, consider turning on query logging
and see if you can correlate log entries with the memory consumption.

> The second significant problem is that this memory allocation does not go
> down once the SELECT is completed. According to ps the process in IDLE yet
> the memory usage remains the same.

Very few Unix programs of any description will release memory back to
the OS before process exit. Postgres is no different. Start a fresh
backend if you need to cut down the memory usage.

There's not a lot more we can say about it until you can identify the
query that sucks up all the space. 7.1 in general is a lot better about
memory consumption than prior releases, but it sounds like you may have
found a leak that's not plugged yet. I would like to know about it when
you identify the problem query.

regards, tom lane

I've continued to work on this problem with limited results so far. I have
lowered the connection re-use setting in our connection pool but this
doesn't seem to have helped. I believe that one particular query ends up
allocating around 80MB of memory and if more than 2 or 3 of the connections
in the pool do this simultaneously then the total memory usage becomes too
high (number of connections * 80 MB). I haven't been able to determine the
exact query yet, I'm still working on it.

I have also discovered another problem with this system and running out of
memory. This one, I believe is a bug in the JDBC driver. Here's the
situation:

The Java program processes a large number (in this case, around 60,000) of
database actions (each consists of a few simple queries and a larger insert
or update), all in a single Java thread. For each of these 60,000 actions
there is a PreparedStatement created. In the PostgreSQL JDBC driver's
implementation of PreparedStatement the following exists during the object
instantiation:

// We use ThreadLocal for SimpleDateFormat's because they are not
that
// thread safe, so each calling thread has its own object.
private ThreadLocal tl_df = new ThreadLocal(); // setDate()
SimpleDateFormat
private ThreadLocal tl_tsdf = new ThreadLocal(); // setTimestamp()
SimpleDateFormat

So, you can see that every single PreparedStatement allocates 2 ThreadLocal
objects. The interesting part is that each of these objects persists for the
entire time the thread persists (in our case, until all 60,000 actions are
completed). This is stated in the Sun Javadocs for ThreadLocal:

Each thread holds an implicit reference to its copy of a
ThreadLocal as long as the thread is alive and the
ThreadLocal object is accessible; after a thread goes
away, all of its copies of ThreadLocal variables are
subject to garbage collection (unless other references
to these copies exist).

So, this means that ANY APPLICATION that uses PreparedStatements in a thread
that 1) either does a lot of PreparedStatements or 2) never dies (i.e., a
main thread) will ALWAYS eventually have an out of memory error. Simply put,
this is a MEMORY LEAK. I imagine that the leak is very small, the
ThreadLocal object only contains one member variable, maybe 64 bytes or less
(depending on the VM implementation). So, our 60,000 PreparedStatements of 2
ThreadLocals each times 64 bytes (my wild guess) is 7.5MB.

The good news is that ThreadLocal is ONLY used in PreparedStatemnt and not
in any other parts of the JDBC driver.

I'll work on a patch but if someone has already done this I would be
grateful.

--Rainer

> -----Original Message-----
> From: pgsql-admin-owner(at)postgresql(dot)org
> [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Tom Lane
> Sent: Thursday, June 14, 2001 10:03 AM
> To: Rainer Mager
> Cc: PostgreSQL Admin
> Subject: Re: [ADMIN] High memory usage
>
> If you can catch the thing while the memory usage is going up, you could
> attach to the backend with gdb and examine debug_query_string to see
> what the current query is. (Note: I think you need 7.1.1 or later to
> have this variable available. You should be on 7.1.2 in any case, just
> on general principles.) Otherwise, consider turning on query logging
> and see if you can correlate log entries with the memory consumption.

Attached is my patch to the official 7.1.2 PreparedStatement.java class.
This seems to work quite well for me in a test case. To try to reproduce the
seen problem I will need to test all night. I'll report tomorrow.

BTW, this is my first attempt at making a unified diff so I might have done
something wrong. If this diff doesn't apply please tell me.

--Rainer

> -----Original Message-----
> From: pgsql-admin-owner(at)postgresql(dot)org
> [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Rainer Mager
> Sent: Wednesday, June 20, 2001 9:08 AM
> To: Tom Lane
> Cc: PostgreSQL Admin
> Subject: RE: [ADMIN] High memory usage
>
>
> I'll work on a patch but if someone has already done this I would be
> grateful.

Hi,

Here is a query that demonstrates the problem. Running this takes about
60MB until it is done at which time it is freed (I was wrong when I said
otherwise earlier). Interestingly, the same amount of memory is used when
doing an EXPLAIN on this query. Also it happens to return 0 rows. Please
excuse the weird characters in the middle this is a Japanese (UTF8)
database. Also please excuse Outlook breaking the query, it is just one long
line.

SELECT DISTINCT product.product_id FROM product, pr_prop_str alias_table_0,
pr_prop_str alias_table_1, pr_prop_str alias_table_2, pr_prop_str
alias_table_3, pr_prop_str alias_table_4, pr_prop_str alias_table_5,
pr_prop_str alias_table_6, pr_prop_str alias_table_7, pr_prop_str
alias_table_8 WHERE product.product_id = alias_table_0.product_id AND
product.product_id = alias_table_1.product_id AND product.product_id =
alias_table_2.product_id AND product.product_id = alias_table_3.product_id
AND product.product_id = alias_table_4.product_id AND product.product_id =
alias_table_5.product_id AND product.product_id = alias_table_6.product_id
AND product.product_id = alias_table_7.product_id AND product.product_id =
alias_table_8.product_id AND ( alias_table_0.pr_property_id = 147 AND
alias_table_0.str = '3E362cb' ) AND ( alias_table_1.pr_property_id = 18 AND
alias_table_1.str > '000999999' ) AND ( alias_table_2.pr_property_id = 18
AND alias_table_2.str < '004999999' ) AND ( alias_table_3.pr_property_id =
51 AND alias_table_3.str = '蛬Oウ縷C~O縷Cウ縷C~I縷Cォ' ) AND (
alias_table_4.pr_property_id = 115 AND alias_table_4.str = '1' ) AND (
alias_table_5.pr_property_id = 68 AND alias_table_5.str = '05' ) AND (
alias_table_6.pr_property_id = 113 AND alias_table_6.str < '030001' ) AND
( alias_table_7.pr_property_id = 57 AND alias_table_7.str < '19980101' ) AND
( alias_table_8.pr_property_id = 158 AND alias_table_8.str = '1' );

So, why is so much memory used and are there any settings I can change to
reduce that (possibly at the expense of performance)? The problem is that
sometimes this query will be running 4 or 5 times simultaneously and using
all of the box's memory.

Thanks,

--Rainer

> -----Original Message-----
> From: pgsql-admin-owner(at)postgresql(dot)org
> [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Tom Lane
> Sent: Thursday, June 14, 2001 10:03 AM
> To: Rainer Mager
> Cc: PostgreSQL Admin
> Subject: Re: [ADMIN] High memory usage
>
>
> There's not a lot more we can say about it until you can identify the
> query that sucks up all the space. 7.1 in general is a lot better about
> memory consumption than prior releases, but it sounds like you may have
> found a leak that's not plugged yet. I would like to know about it when
> you identify the problem query.
>
> regards, tom lane

More info on this...

First, this type of query seems to use exponential amounts of memory. I
tried a join with 10 pieces (the one below with 9 used ~60MB) and it went up
immediately to ~ 390MB usage and has been sitting there at 100% CPU usage
for more than 10 minutes. This is on a database with almost no data in it.
Similarly, a query with only 8 joins is very quick and has much lower memory
usage.

Best regards,

--Rainer

> -----Original Message-----
> From: Rainer Mager [mailto:rmager(at)vgkk(dot)com]
> Sent: Wednesday, June 20, 2001 4:19 PM
> To: Tom Lane
> Cc: PostgreSQL Admin
> Subject: RE: [ADMIN] High memory usage
>
>
> Hi,
>
> Here is a query that demonstrates the problem. Running this
> takes about 60MB until it is done at which time it is freed (I
> was wrong when I said otherwise earlier). Interestingly, the same
> amount of memory is used when doing an EXPLAIN on this query.
> Also it happens to return 0 rows. Please excuse the weird
> characters in the middle this is a Japanese (UTF8) database. Also
> please excuse Outlook breaking the query, it is just one long line.
>
>
> SELECT DISTINCT product.product_id FROM product, pr_prop_str
> alias_table_0, pr_prop_str alias_table_1, pr_prop_str
> alias_table_2, pr_prop_str alias_table_3, pr_prop_str
> alias_table_4, pr_prop_str alias_table_5, pr_prop_str
> alias_table_6, pr_prop_str alias_table_7, pr_prop_str
> alias_table_8 WHERE product.product_id = alias_table_0.product_id
> AND product.product_id = alias_table_1.product_id AND
> product.product_id = alias_table_2.product_id AND
> product.product_id = alias_table_3.product_id AND
> product.product_id = alias_table_4.product_id AND
> product.product_id = alias_table_5.product_id AND
> product.product_id = alias_table_6.product_id AND
> product.product_id = alias_table_7.product_id AND
> product.product_id = alias_table_8.product_id AND (
> alias_table_0.pr_property_id = 147 AND alias_table_0.str =
> '3E362cb' ) AND ( alias_table_1.pr_property_id = 18 AND
> alias_table_1.str > '000999999' ) AND (
> alias_table_2.pr_property_id = 18 AND alias_table_2.str <
> '004999999' ) AND ( alias_table_3.pr_property_id = 51 AND
> alias_table_3.str = '蛬Oウ縷C~O縷Cウ縷C~I縷Cォ' ) AND (
> alias_table_4.pr_property_id = 115 AND alias_table_4.str = '1' )
> AND ( alias_table_5.pr_property_id = 68 AND alias_table_5.str =
> '05' ) AND ( alias_table_6.pr_property_id = 113 AND
> alias_table_6.str < '030001' ) AND ( alias_table_7.pr_property_id
> = 57 AND alias_table_7.str < '19980101' ) AND (
> alias_table_8.pr_property_id = 158 AND alias_table_8.str = '1' );

Ranier -
Can you explain in words what this query is supposed to be doing?

I'm guessing, from the DISTINCT, and the use of multiple occurances of
the same table, that the result you want can be gotten at in some other
way, that lets the backend be smarter about how it does it. Since it _is_
releasing the memory, i.e., it's not a new leak, I'm guessing that Tom
just got a whole lot less interested ;-) But helping you use PostgreSQL
better is part of what the community does, as well.

Hmm, you mention that _planning_ this query sucks up the memory, as well.
My guess is it's an interaction of the optimizer with the plan for this
query, which might have many, nearly identical cost plans, since 8 of
the 9 tables are actually the same table.

Ross

SELECT DISTINCT product.product_id
FROM product,
pr_prop_str alias_table_0,
pr_prop_str alias_table_1,
pr_prop_str alias_table_2,
pr_prop_str alias_table_3,
pr_prop_str alias_table_4,
pr_prop_str alias_table_5,
pr_prop_str alias_table_6,
pr_prop_str alias_table_7,
pr_prop_str alias_table_8
WHERE product.product_id = alias_table_0.product_id
AND product.product_id = alias_table_1.product_id
AND product.product_id = alias_table_2.product_id
AND product.product_id = alias_table_3.product_id
AND product.product_id = alias_table_4.product_id
AND product.product_id = alias_table_5.product_id
AND product.product_id = alias_table_6.product_id
AND product.product_id = alias_table_7.product_id
AND product.product_id = alias_table_8.product_id
AND ( alias_table_0.pr_property_id = 147
AND alias_table_0.str = '3E362cb' )
AND ( alias_table_1.pr_property_id = 18
AND alias_table_1.str > '000999999' )
AND ( alias_table_2.pr_property_id = 18
AND alias_table_2.str < '004999999' )
AND ( alias_table_3.pr_property_id = 51
AND alias_table_3.str = '?$Bi_O?$B%&e_C~O?$Be_C?$B%&e_C~I?$Be_C?$B%)' )
AND ( alias_table_4.pr_property_id = 115
AND alias_table_4.str = '1' )
AND ( alias_table_5.pr_property_id = 68
AND alias_table_5.str = '05' )
AND ( alias_table_6.pr_property_id = 113
AND alias_table_6.str < '030001' )
AND ( alias_table_7.pr_property_id = 57
AND alias_table_7.str < '19980101' )
AND ( alias_table_8.pr_property_id = 158
AND alias_table_8.str = '1' );

On Wed, Jun 20, 2001 at 04:18:52PM +0900, Rainer Mager wrote:
> Hi,
>
> Here is a query that demonstrates the problem. Running this takes about
> 60MB until it is done at which time it is freed (I was wrong when I said
> otherwise earlier). Interestingly, the same amount of memory is used when
> doing an EXPLAIN on this query. Also it happens to return 0 rows. Please
> excuse the weird characters in the middle this is a Japanese (UTF8)
> database. Also please excuse Outlook breaking the query, it is just one long
> line.
>
>

Thanks for the reply.

I can try to explain the query but it is being generated semi-automatically
so it is not hard coded for a particular purpose. Before going into the
explanation, though, I have a litte bit of new information. First, it is
ONLY the join condition that matters, the other parameters do not make a
difference in terms of memory usage. That is, the following, simplified
query, uses the same amount of memory. Also having or removing the DISTINCT
makes no difference. Also, for some VERY odd reason, adding a 10th
constraint caused the EXPLAIN to take significantly LESS memory but the
actual query still took much MORE memory.

> SELECT DISTINCT product.product_id
> FROM product,
> pr_prop_str alias_table_0,
> pr_prop_str alias_table_1,
> pr_prop_str alias_table_2,
> pr_prop_str alias_table_3,
> pr_prop_str alias_table_4,
> pr_prop_str alias_table_5,
> pr_prop_str alias_table_6,
> pr_prop_str alias_table_7,
> pr_prop_str alias_table_8
> WHERE product.product_id = alias_table_0.product_id
> AND product.product_id = alias_table_1.product_id
> AND product.product_id = alias_table_2.product_id
> AND product.product_id = alias_table_3.product_id
> AND product.product_id = alias_table_4.product_id
> AND product.product_id = alias_table_5.product_id
> AND product.product_id = alias_table_6.product_id
> AND product.product_id = alias_table_7.product_id
> AND product.product_id = alias_table_8.product_id;

Obviously this query isn't particularly interesting by itself but it does,
perhaps, simplify the problem. If you create a table called 'product' with
'product_id' and a table called 'pr_prop_str' with 'product_id', then you
can test the above query. For me, even with minimal data in these tables the
query still took ~60MB. As for an explanation of the full query:

What is happening is that a 'product' is being searched for that fulfills a
number of criteria that are specified in the pr_prop_str (product properties
strings) table. So we join all the product IDs to make sure the product has
all of the required properties. Then we add in the particular property
conditions. Each property has an ID (for example, the first pr_property_id
is 147, that might coorespond to a model number or something like that) that
we use in conjunction with the particular requirement (in this example, the
model number must be '3E362cb').

I hope that makes sense.

--Rainer

"Rainer Mager" <rmager(at)vgkk(dot)com> writes:
> First, this type of query seems to use exponential amounts of memory.

Hmm. What does EXPLAIN show as the plan for the query? How long does
it take to do the EXPLAIN? If the EXPLAIN alone takes a lot of time/
space, then you need to reduce the size of the planner's search space
with an explicit JOIN --- see
http://www.ca.postgresql.org/users-lounge/docs/7.1/postgres/explicit-joins.html

regards, tom lane

Patch format looks good to me. I will wait to see what others say about
its content.

> Attached is my patch to the official 7.1.2 PreparedStatement.java class.
> This seems to work quite well for me in a test case. To try to reproduce the
> seen problem I will need to test all night. I'll report tomorrow.
>
> BTW, this is my first attempt at making a unified diff so I might have done
> something wrong. If this diff doesn't apply please tell me.
>
> --Rainer
>
> > -----Original Message-----
> > From: pgsql-admin-owner(at)postgresql(dot)org
> > [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Rainer Mager
> > Sent: Wednesday, June 20, 2001 9:08 AM
> > To: Tom Lane
> > Cc: PostgreSQL Admin
> > Subject: RE: [ADMIN] High memory usage
> >
> >
> > I'll work on a patch but if someone has already done this I would be
> > grateful.

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

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

This looks very promising. I'll have to look over it and try a few things
out but I think it puts me on the right track.

Thanks!

--Rainer

> -----Original Message-----
> From: Tom Lane [mailto:tgl(at)sss(dot)pgh(dot)pa(dot)us]
> Sent: Thursday, June 21, 2001 10:56 AM
> To: Rainer Mager
> Cc: PostgreSQL Admin
> Subject: Re: [ADMIN] High memory usage
>
>
> "Rainer Mager" <rmager(at)vgkk(dot)com> writes:
> > First, this type of query seems to use exponential amounts of memory.
>
> Hmm. What does EXPLAIN show as the plan for the query? How long does
> it take to do the EXPLAIN? If the EXPLAIN alone takes a lot of time/
> space, then you need to reduce the size of the planner's search space
> with an explicit JOIN --- see
> http://www.ca.postgresql.org/users-lounge/docs/7.1/postgres/explic
> it-joins.html
>
> regards, tom lane

"Ross J. Reedstrom" <reedstrm(at)rice(dot)edu> writes:
> My guess is it's an interaction of the optimizer with the plan for this
> query, which might have many, nearly identical cost plans, since 8 of
> the 9 tables are actually the same table.

Yes, I suspect the same. A large fraction of the possible subplans
would have exactly identical costs, which would keep the planner from
discarding any of them (normally, it drops clearly-inferior subplans
instantly, which does a great deal to limit exponential search
behavior). It doesn't help any that the WHERE conditions are all
so similar, either.

I have a strong suspicion that the database schema is poorly thought
out, but lacking any concrete info, it's hard to offer suggestions
for improvement.

regards, tom lane

JDBC folks, can you comment on this patch? Thanks.

> Attached is my patch to the official 7.1.2 PreparedStatement.java class.
> This seems to work quite well for me in a test case. To try to reproduce the
> seen problem I will need to test all night. I'll report tomorrow.
>
> BTW, this is my first attempt at making a unified diff so I might have done
> something wrong. If this diff doesn't apply please tell me.
>
> --Rainer
>
> > -----Original Message-----
> > From: pgsql-admin-owner(at)postgresql(dot)org
> > [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Rainer Mager
> > Sent: Wednesday, June 20, 2001 9:08 AM
> > To: Tom Lane
> > Cc: PostgreSQL Admin
> > Subject: RE: [ADMIN] High memory usage
> >
> >
> > I'll work on a patch but if someone has already done this I would be
> > grateful.

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

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

my two cents worth...

1) what is the problem that this is trying to solve, I assume from the
subject that it is some sort of high memory usage?
2) I am trying to understand how a statement would ever be used by more than
one thread at a time? I would think that it would be impossible to share
statements across threads.
3) The double locking method used is alledgedly unsafe on SMP machines
http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double_p.html.

Dave

----- Original Message -----
From: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: "Rainer Mager" <rmager(at)vgkk(dot)com>
Cc: "PostgreSQL jdbc list" <pgsql-jdbc(at)postgresql(dot)org>; "PostgreSQL-patches"
<pgsql-patches(at)postgresql(dot)org>
Sent: Thursday, June 21, 2001 6:23 PM
Subject: [JDBC] Re: [ADMIN] High memory usage [PATCH]

>
> JDBC folks, can you comment on this patch? Thanks.
>
>
> > Attached is my patch to the official 7.1.2 PreparedStatement.java class.
> > This seems to work quite well for me in a test case. To try to reproduce
the
> > seen problem I will need to test all night. I'll report tomorrow.
> >
> > BTW, this is my first attempt at making a unified diff so I might have
done
> > something wrong. If this diff doesn't apply please tell me.
> >
> > --Rainer
> >
> > > -----Original Message-----
> > > From: pgsql-admin-owner(at)postgresql(dot)org
> > > [mailto:pgsql-admin-owner(at)postgresql(dot)org]On Behalf Of Rainer Mager
> > > Sent: Wednesday, June 20, 2001 9:08 AM
> > > To: Tom Lane
> > > Cc: PostgreSQL Admin
> > > Subject: RE: [ADMIN] High memory usage
> > >
> > >
> > > I'll work on a patch but if someone has already done this I would be
> > > grateful.
>
> [ Attachment, skipping... ]
>
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
> + If your life is a hard drive, | 830 Blythe Avenue
> + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
>

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

> --- PreparedStatement.java Sat Feb 17 01:45:00 2001
> +++ PreparedStatement.java.new Wed Jun 20 12:34:57 2001
> @@ -39,10 +39,12 @@
> // Some performance caches
> private StringBuffer sbuf = new StringBuffer();
>
> - // We use ThreadLocal for SimpleDateFormat's because they are not
that
> - // thread safe, so each calling thread has its own object.
> - private ThreadLocal tl_df = new ThreadLocal(); // setDate()
SimpleDateFormat
> - private ThreadLocal tl_tsdf = new ThreadLocal(); //
setTimestamp() SimpleDateFormat
> + // Because SimpleDateFormat is not thread safe we create one for each
> + // PreparedStatemnt here AND synchronize on it for each usage.
> + // We can NOT use ThreadLocal because they are not freed until the
thread
> + // completes. This would be a memory leak for long running threads.
> + private SimpleDateFormat df = null;
> + private SimpleDateFormat tsdf = null;
>
> /**
> * Constructor for the PreparedStatement class.
> @@ -90,9 +92,6 @@
> * New in 7.1 - overides Statement.close() to dispose of a few
local objects
> */
> public void close() throws SQLException {
> - // free the ThreadLocal caches
> - tl_df.set(null);
> -
> super.close();
> }
>
> @@ -332,14 +331,18 @@
> */
> public void setDate(int parameterIndex, java.sql.Date x) throws
SQLException
> {
> - SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
> - if(df==null) {
> + // The df DateFormat is initialized here to delay creation until needed.
> + if( df == null ) {
> + synchronized( this ) {
> + if( df == null ) {
> df = new SimpleDateFormat("''yyyy-MM-dd''");
> - tl_df.set(df);
> + }
> + }
> }
>
> - set(parameterIndex, df.format(x));
> -
> + // We must synchronize here because SimpleDateFormat is not thread safe.
> + synchronized( df ) {
> + set( parameterIndex, df.format(x) );
> // The above is how the date should be handled.
> //
> // However, in JDK's prior to 1.1.6 (confirmed with the
> @@ -351,6 +354,7 @@
> //
> //set(parameterIndex, df.format(new
java.util.Date(x.getTime()+86400000)));
> }
> + }
>
> /**
> * Set a parameter to a java.sql.Time value. The driver converts
> @@ -375,17 +379,22 @@
> */
> public void setTimestamp(int parameterIndex, Timestamp x) throws
SQLException
> {
> - SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
> - if(df==null) {
> - df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> - tl_tsdf.set(df);
> + // The tsdf DateFormat is initialized here to delay creation until
needed.
> + if( tsdf == null ) {
> + synchronized( this ) {
> + if( tsdf == null ) {
> + tsdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> + tsdf.setTimeZone(TimeZone.getTimeZone("GMT"));
> + }
> + }
> }
> - df.setTimeZone(TimeZone.getTimeZone("GMT"));
>
> + // We must synchronize here because SimpleDateFormat is not thread safe.
> + synchronized( tsdf ) {
> // Use the shared StringBuffer
> synchronized(sbuf) {
> sbuf.setLength(0);
> -
sbuf.append("'").append(df.format(x)).append('.').append(x.getNanos()/100000
00).append("+00'");
> +
sbuf.append("'").append(tsdf.format(x)).append('.').append(x.getNanos()/1000
0000).append("+00'");
> set(parameterIndex, sbuf.toString());
> }
>
> @@ -393,6 +402,7 @@
> // to be identical. Pays to read the docs ;-)
> //set(parameterIndex,"'"+x.toString()+"'");
> }
> + }
>
> /**
> * When a very large ASCII value is input to a LONGVARCHAR parameter,
>

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

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/users-lounge/docs/faq.html
>

Good questions, I'll answer what I can...

> 1) what is the problem that this is trying to solve, I assume from the
> subject that it is some sort of high memory usage?

Yes it involves memory usage and a memory leak. Basically, the way the code
was before the patch, each instantiation of a PreparedStatement created 2
new ThreadLocal objects. According to Sun's Javadocs, these objects are not
freed until the thread is completed. For a single thread app or main thread
of an application (in our case a server) the thread NEVER goes away.
Therefore these ThreadLocal objects just keep adding up and using memory,
forever.

The patch removes the use of these ThreadLocals.

> 2) I am trying to understand how a statement would ever be used
> by more than
> one thread at a time? I would think that it would be impossible to share
> statements across threads.

This I can't really answer. The code appeared to be written to be thread
safe so I attempted to continue that. Whether thread safe for a Statement is
necessary or not I can't directly say. Although, I agree with you, I can't
imagine a need to use a single Statement in multiple threads.

> 3) The double locking method used is alledgedly unsafe on SMP machines
> http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double_p.html.

Interesting. I wasn't aware of this. If question #2 is answered such that
thread safe isn't necessary, then this problem goes away pretty easily. If
thread safety is needed then this would have to be rewritten, I can look
into doing this if you like.

> Dave

--Rainer

* "Dave Cramer" <Dave(at)micro-automation(dot)net> wrote:
|
| my two cents worth...
|
| 1) what is the problem that this is trying to solve, I assume from the
| subject that it is some sort of high memory usage?

I would like to know as well.

| 2) I am trying to understand how a statement would ever be used by more than
| one thread at a time? I would think that it would be impossible to share
| statements across threads.

Quite easy. Just the same way as you share any other object across threads
and according to the JDBC spec requires that the driver shoudl be thread
safe.

| 3) The double locking method used is alledgedly unsafe on SMP machines
| http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double_p.html.

Yup, double checked locking is not going to guarantee that you get
properly initialized objects with the current Java spec.

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

On the thread safe issue: I am having a hard time understanding how one
would share a statement across threads. I would expect that threadsafe
just means don't use static buffers so that I can have multiple
statements in multiple threads. I don't think it is possible to have two
threads concurrently doing a query using one statement. This being said,
I don't thing it is necessary to go to great lengths to lock within the
statement as long as all the members within the statement are
non-static.

Comments?

-----Original Message-----
From: gunnar(at)polygnosis(dot)com [mailto:gunnar(at)polygnosis(dot)com]
Sent: June 22, 2001 7:42 AM
To: Dave Cramer
Cc: Bruce Momjian; Rainer Mager; PostgreSQL jdbc list
Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]

* "Dave Cramer" <Dave(at)micro-automation(dot)net> wrote:
|
| my two cents worth...
|
| 1) what is the problem that this is trying to solve, I assume from the
| subject that it is some sort of high memory usage?

I would like to know as well.

| 2) I am trying to understand how a statement would ever be used by
more than
| one thread at a time? I would think that it would be impossible to
share
| statements across threads.

Quite easy. Just the same way as you share any other object across
threads
and according to the JDBC spec requires that the driver shoudl be thread

safe.

| 3) The double locking method used is alledgedly unsafe on SMP machines
| http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double_p.html.

Yup, double checked locking is not going to guarantee that you get
properly initialized objects with the current Java spec.

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

* "Dave Cramer" <davec(at)sentricity(dot)com> wrote:
|
| On the thread safe issue: I am having a hard time understanding how one
| would share a statement across threads. I would expect that threadsafe
| just means don't use static buffers so that I can have multiple
| statements in multiple threads. I don't think it is possible to have two
| threads concurrently doing a query using one statement. This being said,
| I don't thing it is necessary to go to great lengths to lock within the
| statement as long as all the members within the statement are
| non-static.
|
| Comments?

According to the spec. you should be able to reference the same statement
from several threads at the same time. I never do this myself but there
might be scenarios where this is feasible, ie. another thread calls
Statement.cancel() to cancel the execution of a long running statement.

I definitaly think the JDBC driver for PostgreSQL needs more attention from
the community. It seems like Peter Mount has very little time to spend on it,
unfortunately that is the case with me as well(as long as I don't get paid
for it...). Among other things I would love to see better support for
BLOBs, but I for now we will have to go with MySQL :-(

regards,

Gunnar

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
|

| Interesting. I wasn't aware of this. If question #2 is answered such that
| thread safe isn't necessary, then this problem goes away pretty easily. If
| thread safety is needed then this would have to be rewritten, I can look
| into doing this if you like.

Thread safety is required by the spec. Do you have "JDBC API tutorial and
reference, 2 ed." from Addison Wesley ? This book contains a section for
JDBC driver writers and explains this issue.

regards,

Gunnar

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

I am going to have a look at the spec in more detail to see how they
expect the driver to be used within threads

I am in a similar situation wrt using the driver in server and will
check if the memory usage is better.

Dave

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Rainer Mager
Sent: June 22, 2001 12:56 AM
To: Dave Cramer; Bruce Momjian
Cc: PostgreSQL jdbc list
Subject: RE: [JDBC] Re: [ADMIN] High memory usage [PATCH]

Good questions, I'll answer what I can...

> 1) what is the problem that this is trying to solve, I assume from the
> subject that it is some sort of high memory usage?

Yes it involves memory usage and a memory leak. Basically, the way the
code
was before the patch, each instantiation of a PreparedStatement created
2
new ThreadLocal objects. According to Sun's Javadocs, these objects are
not
freed until the thread is completed. For a single thread app or main
thread
of an application (in our case a server) the thread NEVER goes away.
Therefore these ThreadLocal objects just keep adding up and using
memory,
forever.

The patch removes the use of these ThreadLocals.

> 2) I am trying to understand how a statement would ever be used
> by more than
> one thread at a time? I would think that it would be impossible to
share
> statements across threads.

This I can't really answer. The code appeared to be written to be thread
safe so I attempted to continue that. Whether thread safe for a
Statement is
necessary or not I can't directly say. Although, I agree with you, I
can't
imagine a need to use a single Statement in multiple threads.

> 3) The double locking method used is alledgedly unsafe on SMP machines
> http://www.javaworld.com/javaworld/jw-02-2001/jw-0209-double_p.html.

Interesting. I wasn't aware of this. If question #2 is answered such
that
thread safe isn't necessary, then this problem goes away pretty easily.
If
thread safety is needed then this would have to be rewritten, I can look
into doing this if you like.

> Dave

--Rainer

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

[snip]

Wouldn't it be more elegant simply to make the ThreadLocal's static (as
I'd have thought was probably the intention of the original author), this
would remove the possiblity of any memory leaks in a single threaded
environment, and have the same affect ultimately in a multithreaded
environment?

Michael Stephenson

Yes, this is the correct approach, great advice!

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Michael Stephenson
Sent: June 22, 2001 10:42 AM
To: PostgreSQL jdbc list
Subject: [JDBC] Re: Re: [ADMIN] High memory usage [PATCH]

[snip]

Wouldn't it be more elegant simply to make the ThreadLocal's static (as
I'd have thought was probably the intention of the original author),
this
would remove the possiblity of any memory leaks in a single threaded
environment, and have the same affect ultimately in a multithreaded
environment?

Michael Stephenson

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://www.postgresql.org/search.mpl

Here is a patch which inspired by Michael Stephens that should work

Dave

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar Rønning
Sent: June 22, 2001 10:14 AM
To: Rainer Mager
Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]

* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
|

| Interesting. I wasn't aware of this. If question #2 is answered such
that
| thread safe isn't necessary, then this problem goes away pretty
easily. If
| thread safety is needed then this would have to be rewritten, I can
look
| into doing this if you like.

Thread safety is required by the spec. Do you have "JDBC API tutorial
and
reference, 2 ed." from Addison Wesley ? This book contains a section for

JDBC driver writers and explains this issue.

regards,

Gunnar

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

> Here is a patch which inspired by Michael Stephens that should work
>
> Dave
>
>
> -----Original Message-----
> From: pgsql-jdbc-owner(at)postgresql(dot)org
> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
> Sent: June 22, 2001 10:14 AM
> To: Rainer Mager
> Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>
> * "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> |
>
> | Interesting. I wasn't aware of this. If question #2 is answered such
> that
> | thread safe isn't necessary, then this problem goes away pretty
> easily. If
> | thread safety is needed then this would have to be rewritten, I can
> look
> | into doing this if you like.
>
> Thread safety is required by the spec. Do you have "JDBC API tutorial
> and
> reference, 2 ed." from Addison Wesley ? This book contains a section for
>
> JDBC driver writers and explains this issue.
>
> regards,
>
> Gunnar
>
> --
> Gunnar R?nning - gunnar(at)polygnosis(dot)com
> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

[ Attachment, skipping... ]

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

Patch applied. Thanks.

> Here is a patch which inspired by Michael Stephens that should work
>
> Dave
>
>
> -----Original Message-----
> From: pgsql-jdbc-owner(at)postgresql(dot)org
> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
> Sent: June 22, 2001 10:14 AM
> To: Rainer Mager
> Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>
> * "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> |
>
> | Interesting. I wasn't aware of this. If question #2 is answered such
> that
> | thread safe isn't necessary, then this problem goes away pretty
> easily. If
> | thread safety is needed then this would have to be rewritten, I can
> look
> | into doing this if you like.
>
> Thread safety is required by the spec. Do you have "JDBC API tutorial
> and
> reference, 2 ed." from Addison Wesley ? This book contains a section for
>
> JDBC driver writers and explains this issue.
>
> regards,
>
> Gunnar
>
> --
> Gunnar R?nning - gunnar(at)polygnosis(dot)com
> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

[ Attachment, skipping... ]

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

Since this patch got applied before I got around to commenting on it, I
have submitted a new patch to address my issues with the original patch.

The problem with the patch as applied is that is always creates the
SimpleDateFormat objects in the constructor of the PreparedStatement
regardless of whether or not they will ever be used in the
PreparedStatement. I have reverted back to the old behavior that only
creates them if necessary in the setDate and setTimestamp methods.

I also removed the close() method. It's only purpose was to free these
two SimpleDateFormat objects. I think it is much better to leave these
two objects cached on the thread so that other PreparedStatements can
use them. (This was the intention of a patch I submitted back in
February where I was trying to remove as many object creations as
possible to improve performance. That patch as written needed to get
pulled because of the problem that SimpleDataFormat objects are not
thread safe. Peter then added the ThreadLocal code to try to solve the
performance problem, but introduced the memory leak that originated this
email thread.) I think the cost of at most two SimpleDateFormat objects
being cached on each thead is worth the benefits of less object creation
and subsequent garbage collection.

thanks,
--Barry

Bruce Momjian wrote:

> Patch applied. Thanks.
>
>
>>Here is a patch which inspired by Michael Stephens that should work
>>
>>Dave
>>
>>
>>-----Original Message-----
>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>Sent: June 22, 2001 10:14 AM
>>To: Rainer Mager
>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>
>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>|
>>
>>| Interesting. I wasn't aware of this. If question #2 is answered such
>>that
>>| thread safe isn't necessary, then this problem goes away pretty
>>easily. If
>>| thread safety is needed then this would have to be rewritten, I can
>>look
>>| into doing this if you like.
>>
>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>and
>>reference, 2 ed." from Addison Wesley ? This book contains a section for
>>
>>JDBC driver writers and explains this issue.
>>
>>regards,
>>
>> Gunnar
>>
>>--
>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>
>>
>
> [ Attachment, skipping... ]
>
>

Barry,

My patch was attempting to maintain some sort of thread safety. I do not
think the if (df==null) test is thread-safe. It would need to be
synchronized.

However, as I have mentioned in a couple of previous posts; I'm not sure
thread-safety is worthwhile. The driver appears to be thread safe in
that multiple threads can each use different instances of connections,
and statements, and resultset's however I don't think it is thread safe
in the sense that multiple threads could use the same instance of the
above objects. The JDBC guide suggests that the driver s/b threadsafe in
this sense (multiple threads....same object). The guide suggests that
one thread may instigate the retrieval of a result set, and another
would display it.

Where this is leading is: What kind of thread safety are we trying to
achieve?

If it's only one instance per thread then, I would have to agree that
Barry's patch s/b applied.

P.S.

Is there a formal process within the postgres group for making these
kind of decisions.

If not I would like to suggest adopting the apache groups +1,-1 voting
approach.

-----Original Message-----
From: Barry Lind [mailto:blind(at)xythos(dot)com]
Sent: June 25, 2001 12:37 AM
To: pgsql-patches(at)postgresql(dot)org
Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
Subject: Re: [ADMIN] High memory usage [PATCH]

Since this patch got applied before I got around to commenting on it, I
have submitted a new patch to address my issues with the original patch.

The problem with the patch as applied is that is always creates the
SimpleDateFormat objects in the constructor of the PreparedStatement
regardless of whether or not they will ever be used in the
PreparedStatement. I have reverted back to the old behavior that only
creates them if necessary in the setDate and setTimestamp methods.

I also removed the close() method. It's only purpose was to free these
two SimpleDateFormat objects. I think it is much better to leave these
two objects cached on the thread so that other PreparedStatements can
use them. (This was the intention of a patch I submitted back in
February where I was trying to remove as many object creations as
possible to improve performance. That patch as written needed to get
pulled because of the problem that SimpleDataFormat objects are not
thread safe. Peter then added the ThreadLocal code to try to solve the
performance problem, but introduced the memory leak that originated this

email thread.) I think the cost of at most two SimpleDateFormat objects

being cached on each thead is worth the benefits of less object creation

and subsequent garbage collection.

thanks,
--Barry

Bruce Momjian wrote:

> Patch applied. Thanks.
>
>
>>Here is a patch which inspired by Michael Stephens that should work
>>
>>Dave
>>
>>
>>-----Original Message-----
>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>Sent: June 22, 2001 10:14 AM
>>To: Rainer Mager
>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>
>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>|
>>
>>| Interesting. I wasn't aware of this. If question #2 is answered such
>>that
>>| thread safe isn't necessary, then this problem goes away pretty
>>easily. If
>>| thread safety is needed then this would have to be rewritten, I can
>>look
>>| into doing this if you like.
>>
>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>and
>>reference, 2 ed." from Addison Wesley ? This book contains a section
for
>>
>>JDBC driver writers and explains this issue.
>>
>>regards,
>>
>> Gunnar
>>
>>--
>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>
>>---------------------------(end of
broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to
majordomo(at)postgresql(dot)org
>>
>>
>
> [ Attachment, skipping... ]
>
>

Barry, let me back out Dave's change until we can all agree on it, OK?
It is easy to do.

> Barry,
>
> My patch was attempting to maintain some sort of thread safety. I do not
> think the if (df==null) test is thread-safe. It would need to be
> synchronized.
>
> However, as I have mentioned in a couple of previous posts; I'm not sure
> thread-safety is worthwhile. The driver appears to be thread safe in
> that multiple threads can each use different instances of connections,
> and statements, and resultset's however I don't think it is thread safe
> in the sense that multiple threads could use the same instance of the
> above objects. The JDBC guide suggests that the driver s/b threadsafe in
> this sense (multiple threads....same object). The guide suggests that
> one thread may instigate the retrieval of a result set, and another
> would display it.
>
>
> Where this is leading is: What kind of thread safety are we trying to
> achieve?
>
> If it's only one instance per thread then, I would have to agree that
> Barry's patch s/b applied.
>
> P.S.
>
> Is there a formal process within the postgres group for making these
> kind of decisions.
>
> If not I would like to suggest adopting the apache groups +1,-1 voting
> approach.
>
> -----Original Message-----
> From: Barry Lind [mailto:blind(at)xythos(dot)com]
> Sent: June 25, 2001 12:37 AM
> To: pgsql-patches(at)postgresql(dot)org
> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Since this patch got applied before I got around to commenting on it, I
> have submitted a new patch to address my issues with the original patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only
> creates them if necessary in the setDate and setTimestamp methods.
>
> I also removed the close() method. It's only purpose was to free these
> two SimpleDateFormat objects. I think it is much better to leave these
> two objects cached on the thread so that other PreparedStatements can
> use them. (This was the intention of a patch I submitted back in
> February where I was trying to remove as many object creations as
> possible to improve performance. That patch as written needed to get
> pulled because of the problem that SimpleDataFormat objects are not
> thread safe. Peter then added the ThreadLocal code to try to solve the
> performance problem, but introduced the memory leak that originated this
>
> email thread.) I think the cost of at most two SimpleDateFormat objects
>
> being cached on each thead is worth the benefits of less object creation
>
> and subsequent garbage collection.
>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
>
> > Patch applied. Thanks.
> >
> >
> >>Here is a patch which inspired by Michael Stephens that should work
> >>
> >>Dave
> >>
> >>
> >>-----Original Message-----
> >>From: pgsql-jdbc-owner(at)postgresql(dot)org
> >>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
> >>Sent: June 22, 2001 10:14 AM
> >>To: Rainer Mager
> >>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> >>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
> >>
> >>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> >>|
> >>
> >>| Interesting. I wasn't aware of this. If question #2 is answered such
> >>that
> >>| thread safe isn't necessary, then this problem goes away pretty
> >>easily. If
> >>| thread safety is needed then this would have to be rewritten, I can
> >>look
> >>| into doing this if you like.
> >>
> >>Thread safety is required by the spec. Do you have "JDBC API tutorial
> >>and
> >>reference, 2 ed." from Addison Wesley ? This book contains a section
> for
> >>
> >>JDBC driver writers and explains this issue.
> >>
> >>regards,
> >>
> >> Gunnar
> >>
> >>--
> >>Gunnar R?nning - gunnar(at)polygnosis(dot)com
> >>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
> >>
> >>---------------------------(end of
> broadcast)---------------------------
> >>TIP 1: subscribe and unsubscribe commands go to
> majordomo(at)postgresql(dot)org
> >>
> >>
> >
> > [ Attachment, skipping... ]
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

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

Dave,

The patch I submitted is thread safe. The "if (df==null)" check is
dealing with a ThreadLocal variable. By definition a ThreadLocal
variable can't be used by any other thread since each thread has its own
copy. (Unless the code goes and hands the object off to some other
thread thus causing threading issues, which it doesn't in this case).
Thus I believe the patch I submitted is perfectly thread safe.

To your more general questions:

I think thread safety is very important for all of the JDBC objects.
There are no restrictions placed on what a user could do with these
objects. It is perfectly legal to create a Statement in one thread,
hand that statement off to two or more other threads to access. Now I
wouldn't recommend doing this, but the spec permits it.

As far as procedures and voting, I also believe that something needs to
be done for the JDBC code base. Since Peter has apparently disappeared
(I haven't seen a post from him in about two months, and the
jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
doesn't even have the 7.1 code yet) I think the core group needs to step
in and provide some direction for the JDBC code base. Whether that is
appointing someone new as the official/unofficial JDBC guru or adopting
some other process, I don't know. What I do know is that the JDBC code
base is suffering from lack of attention and direction.

Issues that I am concerned about in the JDBC code base are:
- get/setXXXStream methods are not really spec compliant
- the 'bytea' datatype (i.e. binary) isn't supported, and can't be
without backward compatibility problems because the get/setBytes methods
currently assume that binary means BLOB.
- performance/performance/performance - lots of work could/should be
done to improve performance. Some good work was started last year but
nothing came of it.
- updateable cursors - I beleive some work is being done here by
various parties on the list, but I have some serious concerns about
if/how such functionality can/should be supported.

thanks,
--Barry

Dave Cramer wrote:

> Barry,
>
> My patch was attempting to maintain some sort of thread safety. I do not
> think the if (df==null) test is thread-safe. It would need to be
> synchronized.
>
> However, as I have mentioned in a couple of previous posts; I'm not sure
> thread-safety is worthwhile. The driver appears to be thread safe in
> that multiple threads can each use different instances of connections,
> and statements, and resultset's however I don't think it is thread safe
> in the sense that multiple threads could use the same instance of the
> above objects. The JDBC guide suggests that the driver s/b threadsafe in
> this sense (multiple threads....same object). The guide suggests that
> one thread may instigate the retrieval of a result set, and another
> would display it.
>
>
> Where this is leading is: What kind of thread safety are we trying to
> achieve?
>
> If it's only one instance per thread then, I would have to agree that
> Barry's patch s/b applied.
>
> P.S.
>
> Is there a formal process within the postgres group for making these
> kind of decisions.
>
> If not I would like to suggest adopting the apache groups +1,-1 voting
> approach.
>
> -----Original Message-----
> From: Barry Lind [mailto:blind(at)xythos(dot)com]
> Sent: June 25, 2001 12:37 AM
> To: pgsql-patches(at)postgresql(dot)org
> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Since this patch got applied before I got around to commenting on it, I
> have submitted a new patch to address my issues with the original patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only
> creates them if necessary in the setDate and setTimestamp methods.
>
> I also removed the close() method. It's only purpose was to free these
> two SimpleDateFormat objects. I think it is much better to leave these
> two objects cached on the thread so that other PreparedStatements can
> use them. (This was the intention of a patch I submitted back in
> February where I was trying to remove as many object creations as
> possible to improve performance. That patch as written needed to get
> pulled because of the problem that SimpleDataFormat objects are not
> thread safe. Peter then added the ThreadLocal code to try to solve the
> performance problem, but introduced the memory leak that originated this
>
> email thread.) I think the cost of at most two SimpleDateFormat objects
>
> being cached on each thead is worth the benefits of less object creation
>
> and subsequent garbage collection.
>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
>
>
>>Patch applied. Thanks.
>>
>>
>>
>>>Here is a patch which inspired by Michael Stephens that should work
>>>
>>>Dave
>>>
>>>
>>>-----Original Message-----
>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>Sent: June 22, 2001 10:14 AM
>>>To: Rainer Mager
>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>
>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>|
>>>
>>>| Interesting. I wasn't aware of this. If question #2 is answered such
>>>that
>>>| thread safe isn't necessary, then this problem goes away pretty
>>>easily. If
>>>| thread safety is needed then this would have to be rewritten, I can
>>>look
>>>| into doing this if you like.
>>>
>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>and
>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>
> for
>
>>>JDBC driver writers and explains this issue.
>>>
>>>regards,
>>>
>>> Gunnar
>>>
>>>--
>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>
>>>---------------------------(end of
>>>
> broadcast)---------------------------
>
>>>TIP 1: subscribe and unsubscribe commands go to
>>>
> majordomo(at)postgresql(dot)org
>
>>>
>>[ Attachment, skipping... ]
>>
>>
>>
>
>

Barry Lind wrote:

> Since this patch got applied before I got around to commenting on it, I
> have submitted a new patch to address my issues with the original patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only
> creates them if necessary in the setDate and setTimestamp methods.
>
> I also removed the close() method. It's only purpose was to free these
> two SimpleDateFormat objects. I think it is much better to leave these
> two objects cached on the thread so that other PreparedStatements can
> use them. (This was the intention of a patch I submitted back in
> February where I was trying to remove as many object creations as
> possible to improve performance. That patch as written needed to get
> pulled because of the problem that SimpleDataFormat objects are not
> thread safe. Peter then added the ThreadLocal code to try to solve the
> performance problem, but introduced the memory leak that originated this
> email thread.) I think the cost of at most two SimpleDateFormat objects
> being cached on each thead is worth the benefits of less object creation
> and subsequent garbage collection.
>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
>
>> Patch applied. Thanks.
>>
>>
>>> Here is a patch which inspired by Michael Stephens that should work
>>>
>>> Dave
>>>
>>>
>>> -----Original Message-----
>>> From: pgsql-jdbc-owner(at)postgresql(dot)org
>>> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>> Sent: June 22, 2001 10:14 AM
>>> To: Rainer Mager
>>> Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>> Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>
>>> * "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>> |
>>>
>>> | Interesting. I wasn't aware of this. If question #2 is answered such
>>> that
>>> | thread safe isn't necessary, then this problem goes away pretty
>>> easily. If
>>> | thread safety is needed then this would have to be rewritten, I can
>>> look
>>> | into doing this if you like.
>>>
>>> Thread safety is required by the spec. Do you have "JDBC API tutorial
>>> and reference, 2 ed." from Addison Wesley ? This book contains a
>>> section for
>>>
>>> JDBC driver writers and explains this issue.
>>>
>>> regards,
>>> Gunnar
>>>
>>> --
>>> Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>
>>> ---------------------------(end of broadcast)---------------------------
>>> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>>
>>>
>>
>> [ Attachment, skipping... ]
>>
>>
>
>
> ------------------------------------------------------------------------
>
> *** ./interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java.orig Sun Jun 24 21:05:29 2001
> --- ./interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java Sun Jun 24 21:13:15 2001
> ***************
> *** 65,78 ****
> this.sql = sql;
> this.connection = connection;
>
> - // might just as well create it here, so we don't take the hit later
> -
> - SimpleDateFormat df = new SimpleDateFormat("''yyyy-MM-dd''");
> - tl_df.set(df);
> -
> - df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> - tl_tsdf.set(df);
> -
> for (i = 0; i < sql.length(); ++i)
> {
> int c = sql.charAt(i);
> --- 65,70 ----
> ***************
> *** 95,111 ****
> templateStrings[i] = (String)v.elementAt(i);
> }
>
> - /**
> - * New in 7.1 - overides Statement.close() to dispose of a few local objects
> - */
> - public void close() throws SQLException
> - {
> - // free the ThreadLocal caches
> - tl_df.set(null);
> - tl_tsdf.set(null);
> - super.close();
> - }
> -
> /**
> * A Prepared SQL query is executed and its ResultSet is returned
> *
> --- 87,92 ----
> ***************
> *** 343,348 ****
> --- 324,333 ----
> public void setDate(int parameterIndex, java.sql.Date x) throws SQLException
> {
> SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
> + if(df==null) {
> + df = new SimpleDateFormat("''yyyy-MM-dd''");
> + tl_df.set(df);
> + }
>
> set(parameterIndex, df.format(x));
>
> ***************
> *** 382,387 ****
> --- 367,376 ----
> public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException
> {
> SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
> + if(df==null) {
> + df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
> + tl_tsdf.set(df);
> + }
> df.setTimeZone(TimeZone.getTimeZone("GMT"));
>
> // Use the shared StringBuffer
>
> patch.txt
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> 7bit
>
>

Bruce,

I don't think you need to back out Dave's patch. It works fine as it
is. I just think it can be improved upon.

thanks,
--Barry

Bruce Momjian wrote:

> Barry, let me back out Dave's change until we can all agree on it, OK?
> It is easy to do.
>
>
>>Barry,
>>
>>My patch was attempting to maintain some sort of thread safety. I do not
>>think the if (df==null) test is thread-safe. It would need to be
>>synchronized.
>>
>>However, as I have mentioned in a couple of previous posts; I'm not sure
>>thread-safety is worthwhile. The driver appears to be thread safe in
>>that multiple threads can each use different instances of connections,
>>and statements, and resultset's however I don't think it is thread safe
>>in the sense that multiple threads could use the same instance of the
>>above objects. The JDBC guide suggests that the driver s/b threadsafe in
>>this sense (multiple threads....same object). The guide suggests that
>>one thread may instigate the retrieval of a result set, and another
>>would display it.
>>
>>
>>Where this is leading is: What kind of thread safety are we trying to
>>achieve?
>>
>>If it's only one instance per thread then, I would have to agree that
>>Barry's patch s/b applied.
>>
>>P.S.
>>
>>Is there a formal process within the postgres group for making these
>>kind of decisions.
>>
>>If not I would like to suggest adopting the apache groups +1,-1 voting
>>approach.
>>
>>-----Original Message-----
>>From: Barry Lind [mailto:blind(at)xythos(dot)com]
>>Sent: June 25, 2001 12:37 AM
>>To: pgsql-patches(at)postgresql(dot)org
>>Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
>>Subject: Re: [ADMIN] High memory usage [PATCH]
>>
>>Since this patch got applied before I got around to commenting on it, I
>>have submitted a new patch to address my issues with the original patch.
>>
>>The problem with the patch as applied is that is always creates the
>>SimpleDateFormat objects in the constructor of the PreparedStatement
>>regardless of whether or not they will ever be used in the
>>PreparedStatement. I have reverted back to the old behavior that only
>>creates them if necessary in the setDate and setTimestamp methods.
>>
>>I also removed the close() method. It's only purpose was to free these
>>two SimpleDateFormat objects. I think it is much better to leave these
>>two objects cached on the thread so that other PreparedStatements can
>>use them. (This was the intention of a patch I submitted back in
>>February where I was trying to remove as many object creations as
>>possible to improve performance. That patch as written needed to get
>>pulled because of the problem that SimpleDataFormat objects are not
>>thread safe. Peter then added the ThreadLocal code to try to solve the
>>performance problem, but introduced the memory leak that originated this
>>
>>email thread.) I think the cost of at most two SimpleDateFormat objects
>>
>>being cached on each thead is worth the benefits of less object creation
>>
>>and subsequent garbage collection.
>>
>>thanks,
>>--Barry
>>
>>
>>Bruce Momjian wrote:
>>
>>
>>>Patch applied. Thanks.
>>>
>>>
>>>
>>>>Here is a patch which inspired by Michael Stephens that should work
>>>>
>>>>Dave
>>>>
>>>>
>>>>-----Original Message-----
>>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>>Sent: June 22, 2001 10:14 AM
>>>>To: Rainer Mager
>>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>
>>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>>|
>>>>
>>>>| Interesting. I wasn't aware of this. If question #2 is answered such
>>>>that
>>>>| thread safe isn't necessary, then this problem goes away pretty
>>>>easily. If
>>>>| thread safety is needed then this would have to be rewritten, I can
>>>>look
>>>>| into doing this if you like.
>>>>
>>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>>and
>>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>>
>>for
>>
>>>>JDBC driver writers and explains this issue.
>>>>
>>>>regards,
>>>>
>>>> Gunnar
>>>>
>>>>--
>>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>>---------------------------(end of
>>>>
>>broadcast)---------------------------
>>
>>>>TIP 1: subscribe and unsubscribe commands go to
>>>>
>>majordomo(at)postgresql(dot)org
>>
>>>>
>>>[ Attachment, skipping... ]
>>>
>>>
>>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>
>>
>

> Bruce,
>
> I don't think you need to back out Dave's patch. It works fine as it
> is. I just think it can be improved upon.

OK, you just let me know.

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

Bruce, Barry,

How about checking for the existence of the date object in the
constructor? This would mean that it would only be created once per
thread.

Dave

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Bruce Momjian
Sent: June 25, 2001 2:50 PM
To: Barry Lind
Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
Subject: Re: [JDBC] RE: [ADMIN] High memory usage [PATCH]

> Bruce,
>
> I don't think you need to back out Dave's patch. It works fine as it
> is. I just think it can be improved upon.

OK, you just let me know.

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

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

I like that idea. Best of both worlds. I will redo my patch to do that
and resubmit.

thanks,
--Barry

Dave Cramer wrote:

> Bruce, Barry,
>
> How about checking for the existence of the date object in the
> constructor? This would mean that it would only be created once per
> thread.
>
> Dave
>
>
> -----Original Message-----
> From: pgsql-jdbc-owner(at)postgresql(dot)org
> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Bruce Momjian
> Sent: June 25, 2001 2:50 PM
> To: Barry Lind
> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> Subject: Re: [JDBC] RE: [ADMIN] High memory usage [PATCH]
>
>
>>Bruce,
>>
>>I don't think you need to back out Dave's patch. It works fine as it
>>is. I just think it can be improved upon.
>>
>
> OK, you just let me know.
>
>

Thanks.

>
> I like that idea. Best of both worlds. I will redo my patch to do that
> and resubmit.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
> > Bruce, Barry,
> >
> > How about checking for the existence of the date object in the
> > constructor? This would mean that it would only be created once per
> > thread.
> >
> > Dave
> >
> >
> > -----Original Message-----
> > From: pgsql-jdbc-owner(at)postgresql(dot)org
> > [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Bruce Momjian
> > Sent: June 25, 2001 2:50 PM
> > To: Barry Lind
> > Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> > Subject: Re: [JDBC] RE: [ADMIN] High memory usage [PATCH]
> >
> >
> >>Bruce,
> >>
> >>I don't think you need to back out Dave's patch. It works fine as it
> >>is. I just think it can be improved upon.
> >>
> >
> > OK, you just let me know.
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

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

Dave,

The patch I submitted is thread safe. The "if (df==null)" check is
dealing with a ThreadLocal variable. By definition a ThreadLocal
variable can't be used by any other thread since each thread has its own
copy. (Unless the code goes and hands the object off to some other
thread thus causing threading issues, which it doesn't in this case).
Thus I believe the patch I submitted is perfectly thread safe.

To your more general questions:

I think thread safety is very important for all of the JDBC objects.
There are no restrictions placed on what a user could do with these
objects. It is perfectly legal to create a Statement in one thread,
hand that statement off to two or more other threads to access. Now I
wouldn't recommend doing this, but the spec permits it.

As far as procedures and voting, I also believe that something needs to
be done for the JDBC code base. Since Peter has apparently disappeared
(I haven't seen a post from him in about two months, and the
jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
doesn't even have the 7.1 code yet) I think the core group needs to step
in and provide some direction for the JDBC code base. Whether that is
appointing someone new as the official/unofficial JDBC guru or adopting
some other process, I don't know. What I do know is that the JDBC code
base is suffering from lack of attention and direction.

Issues that I am concerned about in the JDBC code base are:
- get/setXXXStream methods are not really spec compliant
- the 'bytea' datatype (i.e. binary) isn't supported, and can't be
without backward compatibility problems because the get/setBytes methods
currently assume that binary means BLOB.
- performance/performance/performance - lots of work could/should be
done to improve performance. Some good work was started last year but
nothing came of it.
- updateable cursors - I beleive some work is being done here by
various parties on the list, but I have some serious concerns about
if/how such functionality can/should be supported.

thanks,
--Barry


> Dave Cramer wrote:
>
>> Barry,
>>
>> My patch was attempting to maintain some sort of thread safety. I do not
>> think the if (df==null) test is thread-safe. It would need to be
>> synchronized.
>>
>> However, as I have mentioned in a couple of previous posts; I'm not sure
>> thread-safety is worthwhile. The driver appears to be thread safe in
>> that multiple threads can each use different instances of connections,
>> and statements, and resultset's however I don't think it is thread safe
>> in the sense that multiple threads could use the same instance of the
>> above objects. The JDBC guide suggests that the driver s/b threadsafe in
>> this sense (multiple threads....same object). The guide suggests that
>> one thread may instigate the retrieval of a result set, and another
>> would display it.
>>
>>
>> Where this is leading is: What kind of thread safety are we trying to
>> achieve?
>> If it's only one instance per thread then, I would have to agree that
>> Barry's patch s/b applied.
>>
>> P.S.
>> Is there a formal process within the postgres group for making these
>> kind of decisions.
>> If not I would like to suggest adopting the apache groups +1,-1 voting
>> approach.
>>
>> -----Original Message-----
>> From: Barry Lind [mailto:blind(at)xythos(dot)com] Sent: June 25, 2001 12:37 AM
>> To: pgsql-patches(at)postgresql(dot)org
>> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
>> Subject: Re: [ADMIN] High memory usage [PATCH]
>>
>> Since this patch got applied before I got around to commenting on it,
>> I have submitted a new patch to address my issues with the original
>> patch.
>>
>> The problem with the patch as applied is that is always creates the
>> SimpleDateFormat objects in the constructor of the PreparedStatement
>> regardless of whether or not they will ever be used in the
>> PreparedStatement. I have reverted back to the old behavior that only
>> creates them if necessary in the setDate and setTimestamp methods.
>>
>> I also removed the close() method. It's only purpose was to free
>> these two SimpleDateFormat objects. I think it is much better to
>> leave these two objects cached on the thread so that other
>> PreparedStatements can use them. (This was the intention of a patch I
>> submitted back in February where I was trying to remove as many object
>> creations as possible to improve performance. That patch as written
>> needed to get pulled because of the problem that SimpleDataFormat
>> objects are not thread safe. Peter then added the ThreadLocal code to
>> try to solve the performance problem, but introduced the memory leak
>> that originated this
>>
>> email thread.) I think the cost of at most two SimpleDateFormat objects
>>
>> being cached on each thead is worth the benefits of less object creation
>>
>> and subsequent garbage collection.
>>
>> thanks,
>> --Barry
>>
>>
>> Bruce Momjian wrote:
>>
>>
>>> Patch applied. Thanks.
>>>
>>>
>>>
>>>> Here is a patch which inspired by Michael Stephens that should work
>>>>
>>>> Dave
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>> Sent: June 22, 2001 10:14 AM
>>>> To: Rainer Mager
>>>> Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>> Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>
>>>> * "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>> |
>>>>
>>>> | Interesting. I wasn't aware of this. If question #2 is answered such
>>>> that
>>>> | thread safe isn't necessary, then this problem goes away pretty
>>>> easily. If
>>>> | thread safety is needed then this would have to be rewritten, I can
>>>> look
>>>> | into doing this if you like.
>>>>
>>>> Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>> and reference, 2 ed." from Addison Wesley ? This book contains a
>>>> section
>>>>
>> for
>>
>>>> JDBC driver writers and explains this issue.
>>>>
>>>> regards,
>>>> Gunnar
>>>>
>>>> --
>>>> Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>> ---------------------------(end of
>>>>
>> broadcast)---------------------------
>>
>>>> TIP 1: subscribe and unsubscribe commands go to
>>>>
>> majordomo(at)postgresql(dot)org
>>
>>>>
>>> [ Attachment, skipping... ]
>>>
>>>
>>>
>>
>>
>
>

resending since I sent it from the wrong email address last time.

Since this patch got applied before I got around to commenting on it,

I have submitted a new patch to address my issues with the original
patch.

The problem with the patch as applied is that is always creates the
SimpleDateFormat objects in the constructor of the PreparedStatement
regardless of whether or not they will ever be used in the
PreparedStatement. I have reverted back to the old behavior that only
creates them if necessary in the setDate and setTimestamp methods.

I also removed the close() method. It's only purpose was to free
these two SimpleDateFormat objects. I think it is much better to
leave these two objects cached on the thread so that other
PreparedStatements can use them. (This was the intention of a patch I
submitted back in February where I was trying to remove as many object
creations as possible to improve performance. That patch as written
needed to get pulled because of the problem that SimpleDataFormat
objects are not thread safe. Peter then added the ThreadLocal code to
try to solve the performance problem, but introduced the memory leak
that originated this email thread.) I think the cost of at most two
SimpleDateFormat objects being cached on each thead is worth the
benefits of less object creation and subsequent garbage collection.

thanks,
--Barry

>> Bruce Momjian wrote:
>>
>>> Patch applied. Thanks.
>>>
>>>
>>>> Here is a patch which inspired by Michael Stephens that should work
>>>>
>>>> Dave
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>> [mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>> Sent: June 22, 2001 10:14 AM
>>>> To: Rainer Mager
>>>> Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>> Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>
>>>> * "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>> |
>>>>
>>>> | Interesting. I wasn't aware of this. If question #2 is answered such
>>>> that
>>>> | thread safe isn't necessary, then this problem goes away pretty
>>>> easily. If
>>>> | thread safety is needed then this would have to be rewritten, I can
>>>> look
>>>> | into doing this if you like.
>>>>
>>>> Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>> and reference, 2 ed." from Addison Wesley ? This book contains a
>>>> section for
>>>>
>>>> JDBC driver writers and explains this issue.
>>>>
>>>> regards,
>>>> Gunnar
>>>>
>>>> --
>>>> Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>> ---------------------------(end of
>>>> broadcast)---------------------------
>>>> TIP 1: subscribe and unsubscribe commands go to
>>>> majordomo(at)postgresql(dot)org
>>>>
>>>>
>>>
>>> [ Attachment, skipping... ]
>>>
>>>
>>
>>
>> ------------------------------------------------------------------------
>>
>> ***
>> ./interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java.orig
>> Sun Jun 24 21:05:29 2001
>> --- ./interfaces/jdbc/org/postgresql/jdbc2/PreparedStatement.java
>> Sun Jun 24 21:13:15 2001
>> ***************
>> *** 65,78 ****
>> this.sql = sql;
>> this.connection = connection;
>>
>> - // might just as well create it here, so we don't take the
>> hit later
>> - - SimpleDateFormat df = new
>> SimpleDateFormat("''yyyy-MM-dd''");
>> - tl_df.set(df);
>> -
>> - df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
>> - tl_tsdf.set(df);
>> - for (i = 0; i < sql.length(); ++i)
>> {
>> int c = sql.charAt(i);
>> --- 65,70 ----
>> ***************
>> *** 95,111 ****
>> templateStrings[i] = (String)v.elementAt(i);
>> }
>> - /**
>> - * New in 7.1 - overides Statement.close() to dispose of a
>> few local objects
>> - */
>> - public void close() throws SQLException
>> - {
>> - // free the ThreadLocal caches
>> - tl_df.set(null);
>> - tl_tsdf.set(null);
>> - super.close();
>> - }
>> - /**
>> * A Prepared SQL query is executed and its ResultSet is returned
>> *
>> --- 87,92 ----
>> ***************
>> *** 343,348 ****
>> --- 324,333 ----
>> public void setDate(int parameterIndex, java.sql.Date x) throws
>> SQLException
>> {
>> SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
>> + if(df==null) {
>> + df = new SimpleDateFormat("''yyyy-MM-dd''");
>> + tl_df.set(df);
>> + }
>> set(parameterIndex, df.format(x));
>> ***************
>> *** 382,387 ****
>> --- 367,376 ----
>> public void setTimestamp(int parameterIndex, Timestamp x) throws
>> SQLException
>> {
>> SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
>> + if(df==null) {
>> + df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
>> + tl_tsdf.set(df);
>> + }
>> df.setTimeZone(TimeZone.getTimeZone("GMT"));
>> // Use the shared StringBuffer
>>
>> patch.txt
>>
>> Content-Type:
>>
>> text/plain
>> Content-Encoding:
>>
>> 7bit
>>
>>
>
>

Barry,

My understanding of the problem is as follows:

The if (anyvar == null) check is flawed in an SMP environment; according
to the spec anyvar can be in the process of being created the jvm could
set anyvar to point to the memory it is going to assign it to but not
complete the creation of the object. You end up with a non-null, but
invalid anyvar. This is the crux of the double locking flaw. While I
hope most compiler writers would be smarter than that, apparently it is
possible according to the spec.

Well if we make the driver completely thread-safe we are going to take a
real performance hit. Personally I would prefer to code assuming it is
not completely thread-safe and have a lightweight driver.

I am willing to take on some of the work for the driver, but I think the
process s/b a group process. I have learned a lot in this particular
thread.
As I already mentioned I would like to see a voting procedure like the
apache group.

Regarding the catch-22 with Blob, etc. I think we need to make a harsh
decision here. Either break the existing code, or create another driver
codebase. If we don't do something we will be doomed to non-compliance.
This will hurt the driver in the not too distant future. There are a lot
of tools out there which the driver needs to be compatible with.

I had a look at the updateable cursors and it looks possible. Do you
know who is working on it?

Dave

-----Original Message-----
From: Barry Lind [mailto:barry(at)xythos(dot)com]
Sent: June 25, 2001 11:39 AM
To: Dave(at)micro-automation(dot)net
Cc: 'PostgreSQL jdbc list'
Subject: Re: [ADMIN] High memory usage [PATCH]

Dave,

The patch I submitted is thread safe. The "if (df==null)" check is
dealing with a ThreadLocal variable. By definition a ThreadLocal
variable can't be used by any other thread since each thread has its own

copy. (Unless the code goes and hands the object off to some other
thread thus causing threading issues, which it doesn't in this case).
Thus I believe the patch I submitted is perfectly thread safe.

To your more general questions:

I think thread safety is very important for all of the JDBC objects.
There are no restrictions placed on what a user could do with these
objects. It is perfectly legal to create a Statement in one thread,
hand that statement off to two or more other threads to access. Now I
wouldn't recommend doing this, but the spec permits it.

As far as procedures and voting, I also believe that something needs to
be done for the JDBC code base. Since Peter has apparently disappeared
(I haven't seen a post from him in about two months, and the
jdbc.postgresql.org website hasn't been updated for about 4 mounths - it

doesn't even have the 7.1 code yet) I think the core group needs to step

in and provide some direction for the JDBC code base. Whether that is
appointing someone new as the official/unofficial JDBC guru or adopting
some other process, I don't know. What I do know is that the JDBC code
base is suffering from lack of attention and direction.

Issues that I am concerned about in the JDBC code base are:
- get/setXXXStream methods are not really spec compliant
- the 'bytea' datatype (i.e. binary) isn't supported, and can't be
without backward compatibility problems because the get/setBytes methods

currently assume that binary means BLOB.
- performance/performance/performance - lots of work could/should be
done to improve performance. Some good work was started last year but
nothing came of it.
- updateable cursors - I beleive some work is being done here by
various parties on the list, but I have some serious concerns about
if/how such functionality can/should be supported.

thanks,
--Barry

Dave Cramer wrote:

> Barry,
>
> My patch was attempting to maintain some sort of thread safety. I do
not
> think the if (df==null) test is thread-safe. It would need to be
> synchronized.
>
> However, as I have mentioned in a couple of previous posts; I'm not
sure
> thread-safety is worthwhile. The driver appears to be thread safe in
> that multiple threads can each use different instances of connections,
> and statements, and resultset's however I don't think it is thread
safe
> in the sense that multiple threads could use the same instance of the
> above objects. The JDBC guide suggests that the driver s/b threadsafe
in
> this sense (multiple threads....same object). The guide suggests that
> one thread may instigate the retrieval of a result set, and another
> would display it.
>
>
> Where this is leading is: What kind of thread safety are we trying to
> achieve?
>
> If it's only one instance per thread then, I would have to agree that
> Barry's patch s/b applied.
>
> P.S.
>
> Is there a formal process within the postgres group for making these
> kind of decisions.
>
> If not I would like to suggest adopting the apache groups +1,-1 voting
> approach.
>
> -----Original Message-----
> From: Barry Lind [mailto:blind(at)xythos(dot)com]
> Sent: June 25, 2001 12:37 AM
> To: pgsql-patches(at)postgresql(dot)org
> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Since this patch got applied before I got around to commenting on it,
I
> have submitted a new patch to address my issues with the original
patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only

> creates them if necessary in the setDate and setTimestamp methods.
>
> I also removed the close() method. It's only purpose was to free
these
> two SimpleDateFormat objects. I think it is much better to leave
these
> two objects cached on the thread so that other PreparedStatements can
> use them. (This was the intention of a patch I submitted back in
> February where I was trying to remove as many object creations as
> possible to improve performance. That patch as written needed to get
> pulled because of the problem that SimpleDataFormat objects are not
> thread safe. Peter then added the ThreadLocal code to try to solve
the
> performance problem, but introduced the memory leak that originated
this
>
> email thread.) I think the cost of at most two SimpleDateFormat
objects
>
> being cached on each thead is worth the benefits of less object
creation
>
> and subsequent garbage collection.
>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
>
>
>>Patch applied. Thanks.
>>
>>
>>
>>>Here is a patch which inspired by Michael Stephens that should work
>>>
>>>Dave
>>>
>>>
>>>-----Original Message-----
>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>Sent: June 22, 2001 10:14 AM
>>>To: Rainer Mager
>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>
>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>|
>>>
>>>| Interesting. I wasn't aware of this. If question #2 is answered
such
>>>that
>>>| thread safe isn't necessary, then this problem goes away pretty
>>>easily. If
>>>| thread safety is needed then this would have to be rewritten, I can
>>>look
>>>| into doing this if you like.
>>>
>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>and
>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>
> for
>
>>>JDBC driver writers and explains this issue.
>>>
>>>regards,
>>>
>>> Gunnar
>>>
>>>--
>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>
>>>---------------------------(end of
>>>
> broadcast)---------------------------
>
>>>TIP 1: subscribe and unsubscribe commands go to
>>>
> majordomo(at)postgresql(dot)org
>
>>>
>>[ Attachment, skipping... ]
>>
>>
>>
>
>

> I am willing to take on some of the work for the driver, but I
> think the process s/b a group process. I have learned a lot in
> this particular thread. As I already mentioned I would like to
> see a voting procedure like the apache group.

I am not keen on a hard-wired voting process because it makes winners
and losers. I vote only as a last resort. I usually give patches two
days and if there is no comment and it looks good, I apply it. If
anyone objects before or after it is applied, it gets reversed out and
we discuss it.

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

> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base. Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
> doesn't even have the 7.1 code yet) I think the core group needs to step
> in and provide some direction for the JDBC code base. Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know. What I do know is that the JDBC code
> base is suffering from lack of attention and direction.

See my earlier post. We don't have official maintainers of the main
code and I don't see a need for one in the interfaces. We can discuss
issues just like we are doing now. I most cases groups get better
results and single maintainers.

You guys are the official maintainers now. :-)

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

> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base. Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
> doesn't even have the 7.1 code yet) I think the core group needs to step
> in and provide some direction for the JDBC code base. Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know. What I do know is that the JDBC code
> base is suffering from lack of attention and direction.

Let me add I have been concerned about the jdbc interface for the past
year. With you people involved, I am no longer concerned.

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

> resending since I sent it from the wrong email address last time.
>
>
>
> Since this patch got applied before I got around to commenting on it,
>
> I have submitted a new patch to address my issues with the original
> patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only
> creates them if necessary in the setDate and setTimestamp methods.

Barry, did I miss the actual patch you sent?

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)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, Jun 25, 2001 at 09:32:38PM -0400, Bruce Momjian wrote:
> > As far as procedures and voting, I also believe that something needs to
> > be done for the JDBC code base. Since Peter has apparently disappeared
> > (I haven't seen a post from him in about two months, and the
> > jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
> > doesn't even have the 7.1 code yet) I think the core group needs to step
> > in and provide some direction for the JDBC code base. Whether that is
> > appointing someone new as the official/unofficial JDBC guru or adopting
> > some other process, I don't know. What I do know is that the JDBC code
> > base is suffering from lack of attention and direction.
>
> Let me add I have been concerned about the jdbc interface for the past
> year. With you people involved, I am no longer concerned.

Hi, I've only been reading this list for about a week or so now. Joined
it to find out what was happening with this piece of s/w that I was
relying on. I think there is need to push on a bit since as remarked above
it seems that Peter has disappeared. I think a main step is to find out
where we are on the driver, for example its very hard to find out what
isn't implemented yet without reading the code or trying it out. I
haven't found a todo list. A great place to keep all this is on the web
too which needs an update badly. Who has access to update these pages? I
don't care about tutorials or stuff like that yet, but just info on
known bugs, what works and what doesn't, the recommended versions to
run for each database etc and somesort of changelog. I'm willing to help :)

Cheers,

Tom.
--
Thomas O'Dowd. - Nooping - http://nooper.com
tom(at)nooper(dot)com - Testing - http://nooper.co.jp/labs

> > Let me add I have been concerned about the jdbc interface for the past
> > year. With you people involved, I am no longer concerned.
>
> Hi, I've only been reading this list for about a week or so now. Joined
> it to find out what was happening with this piece of s/w that I was
> relying on. I think there is need to push on a bit since as remarked above
> it seems that Peter has disappeared. I think a main step is to find out
> where we are on the driver, for example its very hard to find out what
> isn't implemented yet without reading the code or trying it out. I
> haven't found a todo list. A great place to keep all this is on the web
> too which needs an update badly. Who has access to update these pages? I
> don't care about tutorials or stuff like that yet, but just info on
> known bugs, what works and what doesn't, the recommended versions to
> run for each database etc and somesort of changelog. I'm willing to help :)

Good point. I don't understand JDBC well enough to maintain a TODO
list. If people want I can create one and keep it current but people
are going to have to give me items to add.

I can keep it in the CVS tree and have a web page that links to it with
a nice pretty HTML display. That is how I do the main TODO list, or we
can just add a JDBC section to the main TODO list. That would work too.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)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, Jun 25, 2001 at 10:56:18PM -0400, Dave Cramer wrote:
> I have to agree, we need to compile a todo list.
>
> Mine would include:
>
> 1) Comprehensive test suite. This may be available already.
> 2) Updateable resultSet
> 3) Improved DatabaseMetaData
> 4) Compatible blob support

That looks pretty good from where I stand. I would perhaps separate
out also, the fact that the ResultSet.insertXXX() methods also need to be
implemented as well as the updateXXX methods. I guess this comes under
the updateable resultSet but it's not clear.

What about current known bugs or compliance issues?

How about the web site? Stuff I'd like added would be, where to find
this list, where to find the cvs version, updated binaries, I know
someone on this list has made them available.

Tom.
--
Thomas O'Dowd. - Nooping - http://nooper.com
tom(at)nooper(dot)com - Testing - http://nooper.co.jp/labs

I was the one who made the binaries available at
http://jdbc.fastcrypt.com

I can continue to offer an environment with 1.18, 1.2, 1.3, and j2ee so
we can do builds. Someone else is going to have to do the build using
cygwin; or we can create a microsoft make file for windows.

The compliance issues are in that list, and are essentially 2, 3, 4 on
the list.

Dave

-----Original Message-----
From: Thomas O'Dowd [mailto:tom(at)uwillsee(dot)com] On Behalf Of Thomas O'Dowd
Sent: June 25, 2001 11:30 PM
To: Dave Cramer
Cc: pgsql-jdbc(at)postgresql(dot)org
Subject: Re: [JDBC] Todo/missing? (was Re: [ADMIN] High memory usage
[PATCH])

On Mon, Jun 25, 2001 at 10:56:18PM -0400, Dave Cramer wrote:
> I have to agree, we need to compile a todo list.
>
> Mine would include:
>
> 1) Comprehensive test suite. This may be available already.
> 2) Updateable resultSet
> 3) Improved DatabaseMetaData
> 4) Compatible blob support

That looks pretty good from where I stand. I would perhaps separate
out also, the fact that the ResultSet.insertXXX() methods also need to
be
implemented as well as the updateXXX methods. I guess this comes under
the updateable resultSet but it's not clear.

What about current known bugs or compliance issues?

How about the web site? Stuff I'd like added would be, where to find
this list, where to find the cvs version, updated binaries, I know
someone on this list has made them available.

Tom.
--
Thomas O'Dowd. - Nooping - http://nooper.com
tom(at)nooper(dot)com - Testing - http://nooper.co.jp/labs

> On Mon, Jun 25, 2001 at 10:56:18PM -0400, Dave Cramer wrote:
> > I have to agree, we need to compile a todo list.
> >
> > Mine would include:
> >
> > 1) Comprehensive test suite. This may be available already.
> > 2) Updateable resultSet
> > 3) Improved DatabaseMetaData
> > 4) Compatible blob support

Added to official PostgreSQL TODO:

* JDBC
* Comprehensive test suite. This may be available already.
* Updateable resultSet
* Improved DatabaseMetaData
* Compatible blob support

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

On Tue, Jun 26, 2001 at 12:04:42AM -0400, Bruce Momjian wrote:
>
> Added to official PostgreSQL TODO:
>
> * JDBC
> * Comprehensive test suite. This may be available already.
> * Updateable resultSet
> * Improved DatabaseMetaData
> * Compatible blob support

Bruce,

This is great. I'd like to see Error codes on the list too
pending of course work on the backend first. Good to put it on
the list though, right?

* Error Codes (pending backend implementation)

Cheers,

Tom.
--
Thomas O'Dowd. - Nooping - http://nooper.com
tom(at)nooper(dot)com - Testing - http://nooper.co.jp/labs

Barry,

Your patch is correct, and thread-safe in the light of the next day and
a good night's sleep.

Cheers,

Dave

Barry,

My understanding of the problem is as follows:

The if (anyvar == null) check is flawed in an SMP environment; according
to the spec anyvar can be in the process of being created the jvm could
set anyvar to point to the memory it is going to assign it to but not
complete the creation of the object. You end up with a non-null, but
invalid anyvar. This is the crux of the double locking flaw. While I
hope most compiler writers would be smarter than that, apparently it is
possible according to the spec.

Well if we make the driver completely thread-safe we are going to take a
real performance hit. Personally I would prefer to code assuming it is
not completely thread-safe and have a lightweight driver.

I am willing to take on some of the work for the driver, but I think the
process s/b a group process. I have learned a lot in this particular
thread.
As I already mentioned I would like to see a voting procedure like the
apache group.

Regarding the catch-22 with Blob, etc. I think we need to make a harsh
decision here. Either break the existing code, or create another driver
codebase. If we don't do something we will be doomed to non-compliance.
This will hurt the driver in the not too distant future. There are a lot
of tools out there which the driver needs to be compatible with.

I had a look at the updateable cursors and it looks possible. Do you
know who is working on it?

Dave

-----Original Message-----
From: Barry Lind [mailto:barry(at)xythos(dot)com]
Sent: June 25, 2001 11:39 AM
To: Dave(at)micro-automation(dot)net
Cc: 'PostgreSQL jdbc list'
Subject: Re: [ADMIN] High memory usage [PATCH]

Dave,

The patch I submitted is thread safe. The "if (df==null)" check is
dealing with a ThreadLocal variable. By definition a ThreadLocal
variable can't be used by any other thread since each thread has its own

copy. (Unless the code goes and hands the object off to some other
thread thus causing threading issues, which it doesn't in this case).
Thus I believe the patch I submitted is perfectly thread safe.

To your more general questions:

I think thread safety is very important for all of the JDBC objects.
There are no restrictions placed on what a user could do with these
objects. It is perfectly legal to create a Statement in one thread,
hand that statement off to two or more other threads to access. Now I
wouldn't recommend doing this, but the spec permits it.

As far as procedures and voting, I also believe that something needs to
be done for the JDBC code base. Since Peter has apparently disappeared
(I haven't seen a post from him in about two months, and the
jdbc.postgresql.org website hasn't been updated for about 4 mounths - it

doesn't even have the 7.1 code yet) I think the core group needs to step

in and provide some direction for the JDBC code base. Whether that is
appointing someone new as the official/unofficial JDBC guru or adopting
some other process, I don't know. What I do know is that the JDBC code
base is suffering from lack of attention and direction.

Issues that I am concerned about in the JDBC code base are:
- get/setXXXStream methods are not really spec compliant
- the 'bytea' datatype (i.e. binary) isn't supported, and can't be
without backward compatibility problems because the get/setBytes methods

currently assume that binary means BLOB.
- performance/performance/performance - lots of work could/should be
done to improve performance. Some good work was started last year but
nothing came of it.
- updateable cursors - I beleive some work is being done here by
various parties on the list, but I have some serious concerns about
if/how such functionality can/should be supported.

thanks,
--Barry

Dave Cramer wrote:

> Barry,
>
> My patch was attempting to maintain some sort of thread safety. I do
not
> think the if (df==null) test is thread-safe. It would need to be
> synchronized.
>
> However, as I have mentioned in a couple of previous posts; I'm not
sure
> thread-safety is worthwhile. The driver appears to be thread safe in
> that multiple threads can each use different instances of connections,
> and statements, and resultset's however I don't think it is thread
safe
> in the sense that multiple threads could use the same instance of the
> above objects. The JDBC guide suggests that the driver s/b threadsafe
in
> this sense (multiple threads....same object). The guide suggests that
> one thread may instigate the retrieval of a result set, and another
> would display it.
>
>
> Where this is leading is: What kind of thread safety are we trying to
> achieve?
>
> If it's only one instance per thread then, I would have to agree that
> Barry's patch s/b applied.
>
> P.S.
>
> Is there a formal process within the postgres group for making these
> kind of decisions.
>
> If not I would like to suggest adopting the apache groups +1,-1 voting
> approach.
>
> -----Original Message-----
> From: Barry Lind [mailto:blind(at)xythos(dot)com]
> Sent: June 25, 2001 12:37 AM
> To: pgsql-patches(at)postgresql(dot)org
> Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Since this patch got applied before I got around to commenting on it,
I
> have submitted a new patch to address my issues with the original
patch.
>
> The problem with the patch as applied is that is always creates the
> SimpleDateFormat objects in the constructor of the PreparedStatement
> regardless of whether or not they will ever be used in the
> PreparedStatement. I have reverted back to the old behavior that only

> creates them if necessary in the setDate and setTimestamp methods.
>
> I also removed the close() method. It's only purpose was to free
these
> two SimpleDateFormat objects. I think it is much better to leave
these
> two objects cached on the thread so that other PreparedStatements can
> use them. (This was the intention of a patch I submitted back in
> February where I was trying to remove as many object creations as
> possible to improve performance. That patch as written needed to get
> pulled because of the problem that SimpleDataFormat objects are not
> thread safe. Peter then added the ThreadLocal code to try to solve
the
> performance problem, but introduced the memory leak that originated
this
>
> email thread.) I think the cost of at most two SimpleDateFormat
objects
>
> being cached on each thead is worth the benefits of less object
creation
>
> and subsequent garbage collection.
>
> thanks,
> --Barry
>
>
> Bruce Momjian wrote:
>
>
>>Patch applied. Thanks.
>>
>>
>>
>>>Here is a patch which inspired by Michael Stephens that should work
>>>
>>>Dave
>>>
>>>
>>>-----Original Message-----
>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>Sent: June 22, 2001 10:14 AM
>>>To: Rainer Mager
>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>
>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>|
>>>
>>>| Interesting. I wasn't aware of this. If question #2 is answered
such
>>>that
>>>| thread safe isn't necessary, then this problem goes away pretty
>>>easily. If
>>>| thread safety is needed then this would have to be rewritten, I can
>>>look
>>>| into doing this if you like.
>>>
>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>and
>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>
> for
>
>>>JDBC driver writers and explains this issue.
>>>
>>>regards,
>>>
>>> Gunnar
>>>
>>>--
>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>
>>>---------------------------(end of
>>>
> broadcast)---------------------------
>
>>>TIP 1: subscribe and unsubscribe commands go to
>>>
> majordomo(at)postgresql(dot)org
>
>>>
>>[ Attachment, skipping... ]
>>
>>
>>
>
>

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

* "Dave Cramer" <Dave(at)micro-automation(dot)net> wrote:
|
| Bruce, Barry,
|
| How about checking for the existence of the date object in the
| constructor? This would mean that it would only be created once per
| thread.
|

I only briefly reviewd the code in PreparedStatement yeaterday with respect
to the usage of the SimpleDateFormat, my comments :

- Why the ThreadLocal usage, and not just syncronize on the same
SimpleDateFormat ?

- It seems to that the SimpleDateFormat is never modified after creation.
Wouldn't this mean that we actually could assign the SimpleDateFormat
object to a static member, so we only had to create the object once.
Or is concurrent usage of SimpleDateFormat.format() unsafe ?

I would prefer the latter if safe, as we would get less object creation and
synchronization.

regards,

Gunnar

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

> I only briefly reviewd the code in PreparedStatement yeaterday with respect
> to the usage of the SimpleDateFormat, my comments :
>
> - Why the ThreadLocal usage, and not just syncronize on the same
> SimpleDateFormat ?

This won't scale. :o)

> - It seems to that the SimpleDateFormat is never modified after creation.
> Wouldn't this mean that we actually could assign the SimpleDateFormat
> object to a static member, so we only had to create the object once.
> Or is concurrent usage of SimpleDateFormat.format() unsafe ?

SimpleDateFormat.format() and SimpleDateFormat.parse() are not threadsafe,
I believe the relevant 'bug' is 4228335.

> I would prefer the latter if safe, as we would get less object creation and
> synchronization.

The latter is what we used to have until it became clear that it wasn't
threadsafe.

I think the ThreadLocal solution is the best one available in the
circumstances.

Michael xxx

* Michael Stephenson <mstephenson(at)tirin(dot)openworld(dot)co(dot)uk> wrote:
|
| > - Why the ThreadLocal usage, and not just syncronize on the same
| > SimpleDateFormat ?
|
| This won't scale. :o)
|

Why ? Performance measurements I have seen and also done before has indicated
that synchronization is faster than the usage of ThreadLocal.

cheers,

Gunnar
--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

> On Tue, Jun 26, 2001 at 12:04:42AM -0400, Bruce Momjian wrote:
> >
> > Added to official PostgreSQL TODO:
> >
> > * JDBC
> > * Comprehensive test suite. This may be available already.
> > * Updateable resultSet
> > * Improved DatabaseMetaData
> > * Compatible blob support
>
> Bruce,
>
> This is great. I'd like to see Error codes on the list too
> pending of course work on the backend first. Good to put it on
> the list though, right?
>
> * Error Codes (pending backend implementation)

Added.

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

Can someone send me the patch? I haven't seen any patch from Barry.
How am I missing it?

> Barry,
>
> Your patch is correct, and thread-safe in the light of the next day and
> a good night's sleep.
>
> Cheers,
>
> Dave
>
> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment; according
> to the spec anyvar can be in the process of being created the jvm could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another driver
> codebase. If we don't do something we will be doomed to non-compliance.
> This will hurt the driver in the not too distant future. There are a lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe. The "if (df==null)" check is
> dealing with a ThreadLocal variable. By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects. It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access. Now I
> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base. Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
>
> doesn't even have the 7.1 code yet) I think the core group needs to step
>
> in and provide some direction for the JDBC code base. Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know. What I do know is that the JDBC code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
> - get/setXXXStream methods are not really spec compliant
> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes methods
>
> currently assume that binary means BLOB.
> - performance/performance/performance - lots of work could/should be
> done to improve performance. Some good work was started last year but
> nothing came of it.
> - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
> > Barry,
> >
> > My patch was attempting to maintain some sort of thread safety. I do
> not
> > think the if (df==null) test is thread-safe. It would need to be
> > synchronized.
> >
> > However, as I have mentioned in a couple of previous posts; I'm not
> sure
> > thread-safety is worthwhile. The driver appears to be thread safe in
> > that multiple threads can each use different instances of connections,
> > and statements, and resultset's however I don't think it is thread
> safe
> > in the sense that multiple threads could use the same instance of the
> > above objects. The JDBC guide suggests that the driver s/b threadsafe
> in
> > this sense (multiple threads....same object). The guide suggests that
> > one thread may instigate the retrieval of a result set, and another
> > would display it.
> >
> >
> > Where this is leading is: What kind of thread safety are we trying to
> > achieve?
> >
> > If it's only one instance per thread then, I would have to agree that
> > Barry's patch s/b applied.
> >
> > P.S.
> >
> > Is there a formal process within the postgres group for making these
> > kind of decisions.
> >
> > If not I would like to suggest adopting the apache groups +1,-1 voting
> > approach.
> >
> > -----Original Message-----
> > From: Barry Lind [mailto:blind(at)xythos(dot)com]
> > Sent: June 25, 2001 12:37 AM
> > To: pgsql-patches(at)postgresql(dot)org
> > Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> > Subject: Re: [ADMIN] High memory usage [PATCH]
> >
> > Since this patch got applied before I got around to commenting on it,
> I
> > have submitted a new patch to address my issues with the original
> patch.
> >
> > The problem with the patch as applied is that is always creates the
> > SimpleDateFormat objects in the constructor of the PreparedStatement
> > regardless of whether or not they will ever be used in the
> > PreparedStatement. I have reverted back to the old behavior that only
>
> > creates them if necessary in the setDate and setTimestamp methods.
> >
> > I also removed the close() method. It's only purpose was to free
> these
> > two SimpleDateFormat objects. I think it is much better to leave
> these
> > two objects cached on the thread so that other PreparedStatements can
> > use them. (This was the intention of a patch I submitted back in
> > February where I was trying to remove as many object creations as
> > possible to improve performance. That patch as written needed to get
> > pulled because of the problem that SimpleDataFormat objects are not
> > thread safe. Peter then added the ThreadLocal code to try to solve
> the
> > performance problem, but introduced the memory leak that originated
> this
> >
> > email thread.) I think the cost of at most two SimpleDateFormat
> objects
> >
> > being cached on each thead is worth the benefits of less object
> creation
> >
> > and subsequent garbage collection.
> >
> > thanks,
> > --Barry
> >
> >
> > Bruce Momjian wrote:
> >
> >
> >>Patch applied. Thanks.
> >>
> >>
> >>
> >>>Here is a patch which inspired by Michael Stephens that should work
> >>>
> >>>Dave
> >>>
> >>>
> >>>-----Original Message-----
> >>>From: pgsql-jdbc-owner(at)postgresql(dot)org
> >>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
> >>>Sent: June 22, 2001 10:14 AM
> >>>To: Rainer Mager
> >>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> >>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
> >>>
> >>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> >>>|
> >>>
> >>>| Interesting. I wasn't aware of this. If question #2 is answered
> such
> >>>that
> >>>| thread safe isn't necessary, then this problem goes away pretty
> >>>easily. If
> >>>| thread safety is needed then this would have to be rewritten, I can
> >>>look
> >>>| into doing this if you like.
> >>>
> >>>Thread safety is required by the spec. Do you have "JDBC API tutorial
> >>>and
> >>>reference, 2 ed." from Addison Wesley ? This book contains a section
> >>>
> > for
> >
> >>>JDBC driver writers and explains this issue.
> >>>
> >>>regards,
> >>>
> >>> Gunnar
> >>>
> >>>--
> >>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
> >>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
> >>>
> >>>---------------------------(end of
> >>>
> > broadcast)---------------------------
> >
> >>>TIP 1: subscribe and unsubscribe commands go to
> >>>
> > majordomo(at)postgresql(dot)org
> >
> >>>
> >>[ Attachment, skipping... ]
> >>
> >>
> >>
> >
> >
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

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

> | This won't scale. :o)
>
> Why ? Performance measurements I have seen and also done before has indicated
> that synchronization is faster than the usage of ThreadLocal.

I've just ran a couple of hugely simplified test (basically starting a
couple of thousand threads doing sdf.parse() with either synchronization
or ThreadLocals a couple of times).

With hot spot VM's (I used Sun JDK 1.3.0_02) the difference appears to be
pretty much non existant either way.

With non hot spot VM's (I used Sun JDK 1.2.2) it seems to have a clearer
pattern, the more threads you have the better the ThreadLocal method
performs, and which performs best depends on how many times each Thread
does the format operation (ie how often setDate is called).

For instance with 1000 threads, if they call setDate less than 5 times
the synchronization method seems to be quickest, 5 or more then the
ThreadLocal method performs better.

By the time you have 3000 threads, it only needs to be called 3 times
before the ThreadLocal method becomes quicker. By 5000 threads, if it's
called even twice per thread the syncronization method is 25% slower.

This is fairly inconclusive either way, because clearly different people
use this code in different ways, but I think backs up waht I said about
ThreadLocal's being more scalable, even if they are less efficient in
smaller cases.

I personally would advocate applying a patch similar to below to the
current cvs source.

Michael xxx

--- PreparedStatement.java.orig Tue Jun 26 16:11:16 2001
+++ PreparedStatement.java Tue Jun 26 16:16:26 2001
@@ -44,6 +44,9 @@
private static ThreadLocal tl_df = new ThreadLocal(); // setDate() SimpleDateFormat
private static ThreadLocal tl_tsdf = new ThreadLocal(); // setTimestamp() SimpleDateFormat

+ private SimpleDateFormat sdSdf; // setDate SimpleDateFormat
+ private SimpleDateFormat stSdf; // setTimeStamp SimpleDateFormat
+
/**
* Constructor for the PreparedStatement class.
* Split the SQL statement into segments - separated by the arguments.
@@ -65,13 +68,18 @@
this.sql = sql;
this.connection = connection;

- // might just as well create it here, so we don't take the hit later

- SimpleDateFormat df = new SimpleDateFormat("''yyyy-MM-dd''");
- tl_df.set(df);
-
- df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
- tl_tsdf.set(df);
+ // set up our SimpleDateFormats
+ sdSdf = (SimpleDateFormat)tl_df.get();
+ if (sdSdf == null) {
+ sdSdf = new SimpleDateFormat("''yyyy-MM-dd''");
+ tl_df.set(sdSdf);
+ }
+ stSdf = (SimpleDateFormat)tl_tsdf.get();
+ if (stSdf == null) {
+ stSdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+ tl_tsdf.set(df);
+ }

for (i = 0; i < sql.length(); ++i)
{
@@ -95,17 +103,6 @@
templateStrings[i] = (String)v.elementAt(i);
}

- /**
- * New in 7.1 - overides Statement.close() to dispose of a few local objects
- */
- public void close() throws SQLException
- {
- // free the ThreadLocal caches
- tl_df.set(null);
- tl_tsdf.set(null);
- super.close();
- }
-
/**
* A Prepared SQL query is executed and its ResultSet is returned
*
@@ -342,9 +339,7 @@
*/
public void setDate(int parameterIndex, java.sql.Date x) throws SQLException
{
- SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
-
- set(parameterIndex, df.format(x));
+ set(parameterIndex, sdSdf.format(x));

// The above is how the date should be handled.
//
@@ -381,13 +376,13 @@
*/
public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException
{
- SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
- df.setTimeZone(TimeZone.getTimeZone("GMT"));
+ stSdf.setTimeZone(TimeZone.getTimeZone("GMT"));

// Use the shared StringBuffer
synchronized(sbuf) {
sbuf.setLength(0);
- sbuf.append("'").append(df.format(x)).append('.').append(x.getNanos()/10000000).append("+00'");
+ sbuf.append("'").append(stSdf.format(x)).append('.')
+ .append(x.getNanos()/10000000).append("+00'");
set(parameterIndex, sbuf.toString());
}

I think this is what he is suggesting.

--dc--

-----Original Message-----
From: Bruce Momjian [mailto:pgman(at)candle(dot)pha(dot)pa(dot)us]
Sent: June 26, 2001 11:14 AM
To: Dave(at)micro-automation(dot)net
Cc: 'Barry Lind'; 'PostgreSQL jdbc list'
Subject: Re: [JDBC] High Memory Usage Patch -- Disregard my last message

Can someone send me the patch? I haven't seen any patch from Barry.
How am I missing it?

> Barry,
>
> Your patch is correct, and thread-safe in the light of the next day
and
> a good night's sleep.
>
> Cheers,
>
> Dave
>
> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment;
according
> to the spec anyvar can be in the process of being created the jvm
could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it
is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take
a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think
the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another
driver
> codebase. If we don't do something we will be doomed to
non-compliance.
> This will hurt the driver in the not too distant future. There are a
lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe. The "if (df==null)" check is
> dealing with a ThreadLocal variable. By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its
own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects. It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access. Now I

> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs
to
> be done for the JDBC code base. Since Peter has apparently
disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths -
it
>
> doesn't even have the 7.1 code yet) I think the core group needs to
step
>
> in and provide some direction for the JDBC code base. Whether that is

> appointing someone new as the official/unofficial JDBC guru or
adopting
> some other process, I don't know. What I do know is that the JDBC
code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
> - get/setXXXStream methods are not really spec compliant
> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes
methods
>
> currently assume that binary means BLOB.
> - performance/performance/performance - lots of work could/should
be
> done to improve performance. Some good work was started last year but

> nothing came of it.
> - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
> > Barry,
> >
> > My patch was attempting to maintain some sort of thread safety. I do
> not
> > think the if (df==null) test is thread-safe. It would need to be
> > synchronized.
> >
> > However, as I have mentioned in a couple of previous posts; I'm not
> sure
> > thread-safety is worthwhile. The driver appears to be thread safe in
> > that multiple threads can each use different instances of
connections,
> > and statements, and resultset's however I don't think it is thread
> safe
> > in the sense that multiple threads could use the same instance of
the
> > above objects. The JDBC guide suggests that the driver s/b
threadsafe
> in
> > this sense (multiple threads....same object). The guide suggests
that
> > one thread may instigate the retrieval of a result set, and another
> > would display it.
> >
> >
> > Where this is leading is: What kind of thread safety are we trying
to
> > achieve?
> >
> > If it's only one instance per thread then, I would have to agree
that
> > Barry's patch s/b applied.
> >
> > P.S.
> >
> > Is there a formal process within the postgres group for making these
> > kind of decisions.
> >
> > If not I would like to suggest adopting the apache groups +1,-1
voting
> > approach.
> >
> > -----Original Message-----
> > From: Barry Lind [mailto:blind(at)xythos(dot)com]
> > Sent: June 25, 2001 12:37 AM
> > To: pgsql-patches(at)postgresql(dot)org
> > Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> > Subject: Re: [ADMIN] High memory usage [PATCH]
> >
> > Since this patch got applied before I got around to commenting on
it,
> I
> > have submitted a new patch to address my issues with the original
> patch.
> >
> > The problem with the patch as applied is that is always creates the
> > SimpleDateFormat objects in the constructor of the PreparedStatement

> > regardless of whether or not they will ever be used in the
> > PreparedStatement. I have reverted back to the old behavior that
only
>
> > creates them if necessary in the setDate and setTimestamp methods.
> >
> > I also removed the close() method. It's only purpose was to free
> these
> > two SimpleDateFormat objects. I think it is much better to leave
> these
> > two objects cached on the thread so that other PreparedStatements
can
> > use them. (This was the intention of a patch I submitted back in
> > February where I was trying to remove as many object creations as
> > possible to improve performance. That patch as written needed to
get
> > pulled because of the problem that SimpleDataFormat objects are not
> > thread safe. Peter then added the ThreadLocal code to try to solve
> the
> > performance problem, but introduced the memory leak that originated
> this
> >
> > email thread.) I think the cost of at most two SimpleDateFormat
> objects
> >
> > being cached on each thead is worth the benefits of less object
> creation
> >
> > and subsequent garbage collection.
> >
> > thanks,
> > --Barry
> >
> >
> > Bruce Momjian wrote:
> >
> >
> >>Patch applied. Thanks.
> >>
> >>
> >>
> >>>Here is a patch which inspired by Michael Stephens that should work
> >>>
> >>>Dave
> >>>
> >>>
> >>>-----Original Message-----
> >>>From: pgsql-jdbc-owner(at)postgresql(dot)org
> >>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar
R?nning
> >>>Sent: June 22, 2001 10:14 AM
> >>>To: Rainer Mager
> >>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> >>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
> >>>
> >>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> >>>|
> >>>
> >>>| Interesting. I wasn't aware of this. If question #2 is answered
> such
> >>>that
> >>>| thread safe isn't necessary, then this problem goes away pretty
> >>>easily. If
> >>>| thread safety is needed then this would have to be rewritten, I
can
> >>>look
> >>>| into doing this if you like.
> >>>
> >>>Thread safety is required by the spec. Do you have "JDBC API
tutorial
> >>>and
> >>>reference, 2 ed." from Addison Wesley ? This book contains a
section
> >>>
> > for
> >
> >>>JDBC driver writers and explains this issue.
> >>>
> >>>regards,
> >>>
> >>> Gunnar
> >>>
> >>>--
> >>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
> >>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
> >>>
> >>>---------------------------(end of
> >>>
> > broadcast)---------------------------
> >
> >>>TIP 1: subscribe and unsubscribe commands go to
> >>>
> > majordomo(at)postgresql(dot)org
> >
> >>>
> >>[ Attachment, skipping... ]
> >>
> >>
> >>
> >
> >
>
>
>
>
> ---------------------------(end of
broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to
majordomo(at)postgresql(dot)org)
>
>
>
> ---------------------------(end of
broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

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

Bruce,

I think this is what he is suggesting

Dave

-----Original Message-----
From: Bruce Momjian [mailto:pgman(at)candle(dot)pha(dot)pa(dot)us]
Sent: June 26, 2001 11:14 AM
To: Dave(at)micro-automation(dot)net
Cc: 'Barry Lind'; 'PostgreSQL jdbc list'
Subject: Re: [JDBC] High Memory Usage Patch -- Disregard my last message

Can someone send me the patch? I haven't seen any patch from Barry.
How am I missing it?

> Barry,
>
> Your patch is correct, and thread-safe in the light of the next day
and
> a good night's sleep.
>
> Cheers,
>
> Dave
>
> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment;
according
> to the spec anyvar can be in the process of being created the jvm
could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it
is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take
a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think
the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another
driver
> codebase. If we don't do something we will be doomed to
non-compliance.
> This will hurt the driver in the not too distant future. There are a
lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe. The "if (df==null)" check is
> dealing with a ThreadLocal variable. By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its
own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects. It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access. Now I

> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs
to
> be done for the JDBC code base. Since Peter has apparently
disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths -
it
>
> doesn't even have the 7.1 code yet) I think the core group needs to
step
>
> in and provide some direction for the JDBC code base. Whether that is

> appointing someone new as the official/unofficial JDBC guru or
adopting
> some other process, I don't know. What I do know is that the JDBC
code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
> - get/setXXXStream methods are not really spec compliant
> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes
methods
>
> currently assume that binary means BLOB.
> - performance/performance/performance - lots of work could/should
be
> done to improve performance. Some good work was started last year but

> nothing came of it.
> - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
> > Barry,
> >
> > My patch was attempting to maintain some sort of thread safety. I do
> not
> > think the if (df==null) test is thread-safe. It would need to be
> > synchronized.
> >
> > However, as I have mentioned in a couple of previous posts; I'm not
> sure
> > thread-safety is worthwhile. The driver appears to be thread safe in
> > that multiple threads can each use different instances of
connections,
> > and statements, and resultset's however I don't think it is thread
> safe
> > in the sense that multiple threads could use the same instance of
the
> > above objects. The JDBC guide suggests that the driver s/b
threadsafe
> in
> > this sense (multiple threads....same object). The guide suggests
that
> > one thread may instigate the retrieval of a result set, and another
> > would display it.
> >
> >
> > Where this is leading is: What kind of thread safety are we trying
to
> > achieve?
> >
> > If it's only one instance per thread then, I would have to agree
that
> > Barry's patch s/b applied.
> >
> > P.S.
> >
> > Is there a formal process within the postgres group for making these
> > kind of decisions.
> >
> > If not I would like to suggest adopting the apache groups +1,-1
voting
> > approach.
> >
> > -----Original Message-----
> > From: Barry Lind [mailto:blind(at)xythos(dot)com]
> > Sent: June 25, 2001 12:37 AM
> > To: pgsql-patches(at)postgresql(dot)org
> > Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
> > Subject: Re: [ADMIN] High memory usage [PATCH]
> >
> > Since this patch got applied before I got around to commenting on
it,
> I
> > have submitted a new patch to address my issues with the original
> patch.
> >
> > The problem with the patch as applied is that is always creates the
> > SimpleDateFormat objects in the constructor of the PreparedStatement

> > regardless of whether or not they will ever be used in the
> > PreparedStatement. I have reverted back to the old behavior that
only
>
> > creates them if necessary in the setDate and setTimestamp methods.
> >
> > I also removed the close() method. It's only purpose was to free
> these
> > two SimpleDateFormat objects. I think it is much better to leave
> these
> > two objects cached on the thread so that other PreparedStatements
can
> > use them. (This was the intention of a patch I submitted back in
> > February where I was trying to remove as many object creations as
> > possible to improve performance. That patch as written needed to
get
> > pulled because of the problem that SimpleDataFormat objects are not
> > thread safe. Peter then added the ThreadLocal code to try to solve
> the
> > performance problem, but introduced the memory leak that originated
> this
> >
> > email thread.) I think the cost of at most two SimpleDateFormat
> objects
> >
> > being cached on each thead is worth the benefits of less object
> creation
> >
> > and subsequent garbage collection.
> >
> > thanks,
> > --Barry
> >
> >
> > Bruce Momjian wrote:
> >
> >
> >>Patch applied. Thanks.
> >>
> >>
> >>
> >>>Here is a patch which inspired by Michael Stephens that should work
> >>>
> >>>Dave
> >>>
> >>>
> >>>-----Original Message-----
> >>>From: pgsql-jdbc-owner(at)postgresql(dot)org
> >>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar
R?nning
> >>>Sent: June 22, 2001 10:14 AM
> >>>To: Rainer Mager
> >>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
> >>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
> >>>
> >>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
> >>>|
> >>>
> >>>| Interesting. I wasn't aware of this. If question #2 is answered
> such
> >>>that
> >>>| thread safe isn't necessary, then this problem goes away pretty
> >>>easily. If
> >>>| thread safety is needed then this would have to be rewritten, I
can
> >>>look
> >>>| into doing this if you like.
> >>>
> >>>Thread safety is required by the spec. Do you have "JDBC API
tutorial
> >>>and
> >>>reference, 2 ed." from Addison Wesley ? This book contains a
section
> >>>
> > for
> >
> >>>JDBC driver writers and explains this issue.
> >>>
> >>>regards,
> >>>
> >>> Gunnar
> >>>
> >>>--
> >>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
> >>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
> >>>
> >>>---------------------------(end of
> >>>
> > broadcast)---------------------------
> >
> >>>TIP 1: subscribe and unsubscribe commands go to
> >>>
> > majordomo(at)postgresql(dot)org
> >
> >>>
> >>[ Attachment, skipping... ]
> >>
> >>
> >>
> >
> >
>
>
>
>
> ---------------------------(end of
broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to
majordomo(at)postgresql(dot)org)
>
>
>
> ---------------------------(end of
broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

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

Michael,

Barry pointed out that it isn't necessary for the constructor to be
called from the same thread that is accessing the SimpleDateFormat
objects. I think this requires the test to be done when the objects are
being accessed?

Dave

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Michael Stephenson
Sent: June 26, 2001 11:21 AM
To: 'PostgreSQL jdbc list'; Gunnar Rønning
Subject: Re: [JDBC] Re: RE: [ADMIN] High memory usage [PATCH]

> | This won't scale. :o)
>
> Why ? Performance measurements I have seen and also done before has
indicated
> that synchronization is faster than the usage of ThreadLocal.

I've just ran a couple of hugely simplified test (basically starting a
couple of thousand threads doing sdf.parse() with either synchronization
or ThreadLocals a couple of times).

With hot spot VM's (I used Sun JDK 1.3.0_02) the difference appears to
be
pretty much non existant either way.

With non hot spot VM's (I used Sun JDK 1.2.2) it seems to have a clearer
pattern, the more threads you have the better the ThreadLocal method
performs, and which performs best depends on how many times each Thread
does the format operation (ie how often setDate is called).

For instance with 1000 threads, if they call setDate less than 5 times
the synchronization method seems to be quickest, 5 or more then the
ThreadLocal method performs better.

By the time you have 3000 threads, it only needs to be called 3 times
before the ThreadLocal method becomes quicker. By 5000 threads, if it's
called even twice per thread the syncronization method is 25% slower.

This is fairly inconclusive either way, because clearly different people
use this code in different ways, but I think backs up waht I said about
ThreadLocal's being more scalable, even if they are less efficient in
smaller cases.

I personally would advocate applying a patch similar to below to the
current cvs source.

Michael xxx

--- PreparedStatement.java.orig Tue Jun 26 16:11:16 2001
+++ PreparedStatement.java Tue Jun 26 16:16:26 2001
@@ -44,6 +44,9 @@
private static ThreadLocal tl_df = new ThreadLocal(); //
setDate() SimpleDateFormat
private static ThreadLocal tl_tsdf = new ThreadLocal(); //
setTimestamp() SimpleDateFormat

+ private SimpleDateFormat sdSdf; // setDate SimpleDateFormat
+ private SimpleDateFormat stSdf; // setTimeStamp
SimpleDateFormat
+
/**
* Constructor for the PreparedStatement class.
* Split the SQL statement into segments - separated by the
arguments.
@@ -65,13 +68,18 @@
this.sql = sql;
this.connection = connection;

- // might just as well create it here, so we don't take
the hit later

- SimpleDateFormat df = new
SimpleDateFormat("''yyyy-MM-dd''");
- tl_df.set(df);
-
- df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
- tl_tsdf.set(df);
+ // set up our SimpleDateFormats
+ sdSdf = (SimpleDateFormat)tl_df.get();
+ if (sdSdf == null) {
+ sdSdf = new SimpleDateFormat("''yyyy-MM-dd''");
+ tl_df.set(sdSdf);
+ }
+ stSdf = (SimpleDateFormat)tl_tsdf.get();
+ if (stSdf == null) {
+ stSdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
+ tl_tsdf.set(df);
+ }

for (i = 0; i < sql.length(); ++i)
{
@@ -95,17 +103,6 @@
templateStrings[i] = (String)v.elementAt(i);
}

- /**
- * New in 7.1 - overides Statement.close() to dispose of a few
local objects
- */
- public void close() throws SQLException
- {
- // free the ThreadLocal caches
- tl_df.set(null);
- tl_tsdf.set(null);
- super.close();
- }
-
/**
* A Prepared SQL query is executed and its ResultSet is
returned
*
@@ -342,9 +339,7 @@
*/
public void setDate(int parameterIndex, java.sql.Date x) throws
SQLException
{
- SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
-
- set(parameterIndex, df.format(x));
+ set(parameterIndex, sdSdf.format(x));

// The above is how the date should be handled.
//
@@ -381,13 +376,13 @@
*/
public void setTimestamp(int parameterIndex, Timestamp x) throws
SQLException
{
- SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
- df.setTimeZone(TimeZone.getTimeZone("GMT"));
+ stSdf.setTimeZone(TimeZone.getTimeZone("GMT"));

// Use the shared StringBuffer
synchronized(sbuf) {
sbuf.setLength(0);
-
sbuf.append("'").append(df.format(x)).append('.').append(x.getNanos()/10
000000).append("+00'");
+ sbuf.append("'").append(stSdf.format(x)).append('.')
+ .append(x.getNanos()/10000000).append("+00'");
set(parameterIndex, sbuf.toString());
}

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

* Michael Stephenson <mstephenson(at)tirin(dot)openworld(dot)co(dot)uk> wrote:
|
| I've just ran a couple of hugely simplified test (basically starting a
| couple of thousand threads doing sdf.parse() with either synchronization
| or ThreadLocals a couple of times).
|

I agree that the tests are pretty inconclusive. We need something that
simulates real usage better. What platform are you testing on Linux ?
If so with native threads or green threads ?

|
| I personally would advocate applying a patch similar to below to the
| current cvs source.
|

This I don't understand, was your intention to remove thread safety with this
patch ? Looking at the way you introduce SimpleDateFormat as an instance
variable it seems so.

| Michael xxx
|
| --- PreparedStatement.java.orig Tue Jun 26 16:11:16 2001
| +++ PreparedStatement.java Tue Jun 26 16:16:26 2001
| @@ -44,6 +44,9 @@
| private static ThreadLocal tl_df = new ThreadLocal(); // setDate() SimpleDateFormat
| private static ThreadLocal tl_tsdf = new ThreadLocal(); // setTimestamp() SimpleDateFormat
|
| + private SimpleDateFormat sdSdf; // setDate SimpleDateFormat
| + private SimpleDateFormat stSdf; // setTimeStamp SimpleDateFormat
| +
| /**
| * Constructor for the PreparedStatement class.
| * Split the SQL statement into segments - separated by the arguments.
| @@ -65,13 +68,18 @@
| this.sql = sql;
| this.connection = connection;
|
| - // might just as well create it here, so we don't take the hit later
|
| - SimpleDateFormat df = new SimpleDateFormat("''yyyy-MM-dd''");
| - tl_df.set(df);
| -
| - df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
| - tl_tsdf.set(df);
| + // set up our SimpleDateFormats
| + sdSdf = (SimpleDateFormat)tl_df.get();
| + if (sdSdf == null) {
| + sdSdf = new SimpleDateFormat("''yyyy-MM-dd''");
| + tl_df.set(sdSdf);
| + }
| + stSdf = (SimpleDateFormat)tl_tsdf.get();
| + if (stSdf == null) {
| + stSdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
| + tl_tsdf.set(df);
| + }
|
| for (i = 0; i < sql.length(); ++i)
| {
| @@ -95,17 +103,6 @@
| templateStrings[i] = (String)v.elementAt(i);
| }
|
| - /**
| - * New in 7.1 - overides Statement.close() to dispose of a few local objects
| - */
| - public void close() throws SQLException
| - {
| - // free the ThreadLocal caches
| - tl_df.set(null);
| - tl_tsdf.set(null);
| - super.close();
| - }
| -
| /**
| * A Prepared SQL query is executed and its ResultSet is returned
| *
| @@ -342,9 +339,7 @@
| */
| public void setDate(int parameterIndex, java.sql.Date x) throws SQLException
| {
| - SimpleDateFormat df = (SimpleDateFormat) tl_df.get();
| -
| - set(parameterIndex, df.format(x));
| + set(parameterIndex, sdSdf.format(x));
|
| // The above is how the date should be handled.
| //
| @@ -381,13 +376,13 @@
| */
| public void setTimestamp(int parameterIndex, Timestamp x) throws SQLException
| {
| - SimpleDateFormat df = (SimpleDateFormat) tl_tsdf.get();
| - df.setTimeZone(TimeZone.getTimeZone("GMT"));
| + stSdf.setTimeZone(TimeZone.getTimeZone("GMT"));
|
| // Use the shared StringBuffer
| synchronized(sbuf) {
| sbuf.setLength(0);
| - sbuf.append("'").append(df.format(x)).append('.').append(x.getNanos()/10000000).append("+00'");
| + sbuf.append("'").append(stSdf.format(x)).append('.')
| + .append(x.getNanos()/10000000).append("+00'");
| set(parameterIndex, sbuf.toString());
| }
|
|
|
|
| ---------------------------(end of broadcast)---------------------------
| TIP 2: you can get off all lists at once with the unregister command
| (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
|

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

Dave,

I disagree with your understanding of the double locking issue. You
state "The if (anyvar == null) check is flawed in an SMP environment".
This statement would more correctly read "The if (anyvar == null) check
is flawed in an SMP environment where multiple threads are accessing
anyvar". If only one thread is accessing the variable (as is the case
in this code since the variable is a local variable that comes from a
ThreadLocal object) then there isn't any problem. If the variable in
question here where a member variable, then I would agree with you that
this if check could be problematic.

thanks,
--Barry

Dave Cramer wrote:

> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment; according
> to the spec anyvar can be in the process of being created the jvm could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another driver
> codebase. If we don't do something we will be doomed to non-compliance.
> This will hurt the driver in the not too distant future. There are a lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe. The "if (df==null)" check is
> dealing with a ThreadLocal variable. By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects. It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access. Now I
> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base. Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
>
> doesn't even have the 7.1 code yet) I think the core group needs to step
>
> in and provide some direction for the JDBC code base. Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know. What I do know is that the JDBC code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
> - get/setXXXStream methods are not really spec compliant
> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes methods
>
> currently assume that binary means BLOB.
> - performance/performance/performance - lots of work could/should be
> done to improve performance. Some good work was started last year but
> nothing came of it.
> - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
>
>>Barry,
>>
>>My patch was attempting to maintain some sort of thread safety. I do
>>
> not
>
>>think the if (df==null) test is thread-safe. It would need to be
>>synchronized.
>>
>>However, as I have mentioned in a couple of previous posts; I'm not
>>
> sure
>
>>thread-safety is worthwhile. The driver appears to be thread safe in
>>that multiple threads can each use different instances of connections,
>>and statements, and resultset's however I don't think it is thread
>>
> safe
>
>>in the sense that multiple threads could use the same instance of the
>>above objects. The JDBC guide suggests that the driver s/b threadsafe
>>
> in
>
>>this sense (multiple threads....same object). The guide suggests that
>>one thread may instigate the retrieval of a result set, and another
>>would display it.
>>
>>
>>Where this is leading is: What kind of thread safety are we trying to
>>achieve?
>>
>>If it's only one instance per thread then, I would have to agree that
>>Barry's patch s/b applied.
>>
>>P.S.
>>
>>Is there a formal process within the postgres group for making these
>>kind of decisions.
>>
>>If not I would like to suggest adopting the apache groups +1,-1 voting
>>approach.
>>
>>-----Original Message-----
>>From: Barry Lind [mailto:blind(at)xythos(dot)com]
>>Sent: June 25, 2001 12:37 AM
>>To: pgsql-patches(at)postgresql(dot)org
>>Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
>>Subject: Re: [ADMIN] High memory usage [PATCH]
>>
>>Since this patch got applied before I got around to commenting on it,
>>
> I
>
>>have submitted a new patch to address my issues with the original
>>
> patch.
>
>>The problem with the patch as applied is that is always creates the
>>SimpleDateFormat objects in the constructor of the PreparedStatement
>>regardless of whether or not they will ever be used in the
>>PreparedStatement. I have reverted back to the old behavior that only
>>
>
>>creates them if necessary in the setDate and setTimestamp methods.
>>
>>I also removed the close() method. It's only purpose was to free
>>
> these
>
>>two SimpleDateFormat objects. I think it is much better to leave
>>
> these
>
>>two objects cached on the thread so that other PreparedStatements can
>>use them. (This was the intention of a patch I submitted back in
>>February where I was trying to remove as many object creations as
>>possible to improve performance. That patch as written needed to get
>>pulled because of the problem that SimpleDataFormat objects are not
>>thread safe. Peter then added the ThreadLocal code to try to solve
>>
> the
>
>>performance problem, but introduced the memory leak that originated
>>
> this
>
>>email thread.) I think the cost of at most two SimpleDateFormat
>>
> objects
>
>>being cached on each thead is worth the benefits of less object
>>
> creation
>
>>and subsequent garbage collection.
>>
>>thanks,
>>--Barry
>>
>>
>>Bruce Momjian wrote:
>>
>>
>>
>>>Patch applied. Thanks.
>>>
>>>
>>>
>>>
>>>>Here is a patch which inspired by Michael Stephens that should work
>>>>
>>>>Dave
>>>>
>>>>
>>>>-----Original Message-----
>>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>>Sent: June 22, 2001 10:14 AM
>>>>To: Rainer Mager
>>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>
>>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>>|
>>>>
>>>>| Interesting. I wasn't aware of this. If question #2 is answered
>>>>
> such
>
>>>>that
>>>>| thread safe isn't necessary, then this problem goes away pretty
>>>>easily. If
>>>>| thread safety is needed then this would have to be rewritten, I can
>>>>look
>>>>| into doing this if you like.
>>>>
>>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>>and
>>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>>
>>>>
>>for
>>
>>
>>>>JDBC driver writers and explains this issue.
>>>>
>>>>regards,
>>>>
>>>> Gunnar
>>>>
>>>>--
>>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>>---------------------------(end of
>>>>
>>>>
>>broadcast)---------------------------
>>
>>
>>>>TIP 1: subscribe and unsubscribe commands go to
>>>>
>>>>
>>majordomo(at)postgresql(dot)org
>>
>>
>>>[ Attachment, skipping... ]
>>>
>>>
>>>
>>>
>>
>
>
>

* "Dave Cramer" <Dave(at)micro-automation(dot)net> wrote:
|
| Regarding the catch-22 with Blob, etc. I think we need to make a harsh
| decision here. Either break the existing code, or create another driver
| codebase. If we don't do something we will be doomed to non-compliance.
| This will hurt the driver in the not too distant future. There are a lot
| of tools out there which the driver needs to be compatible with.
|

I would say that it is better to break existing code that depends on
broken behaviour than being out of sync with the standard. Not following
the JDBC standard is just asking for a fork - there are a lot of
object relational tools out there and if want these to work with postgresql
we have no choice.

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

Dave,

I don't personally know who is working on updateable cursors, but if you
search the email archives, there have been a number of emails on this
topic over the last couple of months.

My issue with updateable cursors it that I think this should be
functionality provided by the backend, not patched on to the JDBC
driver. My reasoning here is that updateable cursors are very limited
in what you can do (single table selects, no where clause, etc). In
order to handle this it means that JDBC code is going to need to start
parsing the SQL statement to do this. I don't like the idea of having a
SQL parser on the client when one already exists on the server. You
start running into issues about everytime the server gets a patch to
support some new syntax in the SQL statement that the client parser also
needs to be updated. In my years at Oracle this was always a problem
where the client tools ended up breaking everytime the server added some
new syntax support.

Having said all of that, I think updateable cursors is a good feature,
but I think the majority of the work should be done in the backend and
not in the JDBC driver. The added benefit of this is that then all
front ends have access to this functionality.

thanks,
--Barry

Dave Cramer wrote:

> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment; according
> to the spec anyvar can be in the process of being created the jvm could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another driver
> codebase. If we don't do something we will be doomed to non-compliance.
> This will hurt the driver in the not too distant future. There are a lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe. The "if (df==null)" check is
> dealing with a ThreadLocal variable. By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects. It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access. Now I
> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base. Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
>
> doesn't even have the 7.1 code yet) I think the core group needs to step
>
> in and provide some direction for the JDBC code base. Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know. What I do know is that the JDBC code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
> - get/setXXXStream methods are not really spec compliant
> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes methods
>
> currently assume that binary means BLOB.
> - performance/performance/performance - lots of work could/should be
> done to improve performance. Some good work was started last year but
> nothing came of it.
> - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
>
>>Barry,
>>
>>My patch was attempting to maintain some sort of thread safety. I do
>>
> not
>
>>think the if (df==null) test is thread-safe. It would need to be
>>synchronized.
>>
>>However, as I have mentioned in a couple of previous posts; I'm not
>>
> sure
>
>>thread-safety is worthwhile. The driver appears to be thread safe in
>>that multiple threads can each use different instances of connections,
>>and statements, and resultset's however I don't think it is thread
>>
> safe
>
>>in the sense that multiple threads could use the same instance of the
>>above objects. The JDBC guide suggests that the driver s/b threadsafe
>>
> in
>
>>this sense (multiple threads....same object). The guide suggests that
>>one thread may instigate the retrieval of a result set, and another
>>would display it.
>>
>>
>>Where this is leading is: What kind of thread safety are we trying to
>>achieve?
>>
>>If it's only one instance per thread then, I would have to agree that
>>Barry's patch s/b applied.
>>
>>P.S.
>>
>>Is there a formal process within the postgres group for making these
>>kind of decisions.
>>
>>If not I would like to suggest adopting the apache groups +1,-1 voting
>>approach.
>>
>>-----Original Message-----
>>From: Barry Lind [mailto:blind(at)xythos(dot)com]
>>Sent: June 25, 2001 12:37 AM
>>To: pgsql-patches(at)postgresql(dot)org
>>Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
>>Subject: Re: [ADMIN] High memory usage [PATCH]
>>
>>Since this patch got applied before I got around to commenting on it,
>>
> I
>
>>have submitted a new patch to address my issues with the original
>>
> patch.
>
>>The problem with the patch as applied is that is always creates the
>>SimpleDateFormat objects in the constructor of the PreparedStatement
>>regardless of whether or not they will ever be used in the
>>PreparedStatement. I have reverted back to the old behavior that only
>>
>
>>creates them if necessary in the setDate and setTimestamp methods.
>>
>>I also removed the close() method. It's only purpose was to free
>>
> these
>
>>two SimpleDateFormat objects. I think it is much better to leave
>>
> these
>
>>two objects cached on the thread so that other PreparedStatements can
>>use them. (This was the intention of a patch I submitted back in
>>February where I was trying to remove as many object creations as
>>possible to improve performance. That patch as written needed to get
>>pulled because of the problem that SimpleDataFormat objects are not
>>thread safe. Peter then added the ThreadLocal code to try to solve
>>
> the
>
>>performance problem, but introduced the memory leak that originated
>>
> this
>
>>email thread.) I think the cost of at most two SimpleDateFormat
>>
> objects
>
>>being cached on each thead is worth the benefits of less object
>>
> creation
>
>>and subsequent garbage collection.
>>
>>thanks,
>>--Barry
>>
>>
>>Bruce Momjian wrote:
>>
>>
>>
>>>Patch applied. Thanks.
>>>
>>>
>>>
>>>
>>>>Here is a patch which inspired by Michael Stephens that should work
>>>>
>>>>Dave
>>>>
>>>>
>>>>-----Original Message-----
>>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>>Sent: June 22, 2001 10:14 AM
>>>>To: Rainer Mager
>>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>
>>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>>|
>>>>
>>>>| Interesting. I wasn't aware of this. If question #2 is answered
>>>>
> such
>
>>>>that
>>>>| thread safe isn't necessary, then this problem goes away pretty
>>>>easily. If
>>>>| thread safety is needed then this would have to be rewritten, I can
>>>>look
>>>>| into doing this if you like.
>>>>
>>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>>and
>>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>>
>>>>
>>for
>>
>>
>>>>JDBC driver writers and explains this issue.
>>>>
>>>>regards,
>>>>
>>>> Gunnar
>>>>
>>>>--
>>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>>---------------------------(end of
>>>>
>>>>
>>broadcast)---------------------------
>>
>>
>>>>TIP 1: subscribe and unsubscribe commands go to
>>>>
>>>>
>>majordomo(at)postgresql(dot)org
>>
>>
>>>[ Attachment, skipping... ]
>>>
>>>
>>>
>>>
>>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>
>

> I agree that the tests are pretty inconclusive. We need something that
> simulates real usage better. What platform are you testing on Linux ?
> If so with native threads or green threads ?

Linux, I'm using native threads with the hot spot I'm using native
threads, which as I said was performing pretty much identically in both
cases. The non hot spot machine was using green threads which is where the
difference in performance was measurable.

> | I personally would advocate applying a patch similar to below to the
> | current cvs source.
>
> This I don't understand, was your intention to remove thread safety with this
> patch ? Looking at the way you introduce SimpleDateFormat as an instance
> variable it seems so.

Yeah, the patch is rubbish, I did it that way to remove the
(comparitively expensive) ThreadLocal.get inside the setDate and
SetTimeStamp, which is clearly stupid.

Michael xxx

Bruce,

Here is the patch. When I sent it originally I send from the wrong
email account so it didn't end up getting to the 'patches' list.

thanks,
--Barry

(file patch.txt attached)

Bruce Momjian wrote:

> Can someone send me the patch? I haven't seen any patch from Barry.
> How am I missing it?
>
>
>
>>Barry,
>>
>>Your patch is correct, and thread-safe in the light of the next day and
>>a good night's sleep.
>>
>>Cheers,
>>
>>Dave
>>
>>Barry,
>>
>>My understanding of the problem is as follows:
>>
>>The if (anyvar == null) check is flawed in an SMP environment; according
>>to the spec anyvar can be in the process of being created the jvm could
>>set anyvar to point to the memory it is going to assign it to but not
>>complete the creation of the object. You end up with a non-null, but
>>invalid anyvar. This is the crux of the double locking flaw. While I
>>hope most compiler writers would be smarter than that, apparently it is
>>possible according to the spec.
>>
>>Well if we make the driver completely thread-safe we are going to take a
>>real performance hit. Personally I would prefer to code assuming it is
>>not completely thread-safe and have a lightweight driver.
>>
>>I am willing to take on some of the work for the driver, but I think the
>>process s/b a group process. I have learned a lot in this particular
>>thread.
>>As I already mentioned I would like to see a voting procedure like the
>>apache group.
>>
>>Regarding the catch-22 with Blob, etc. I think we need to make a harsh
>>decision here. Either break the existing code, or create another driver
>>codebase. If we don't do something we will be doomed to non-compliance.
>>This will hurt the driver in the not too distant future. There are a lot
>>of tools out there which the driver needs to be compatible with.
>>
>>I had a look at the updateable cursors and it looks possible. Do you
>>know who is working on it?
>>
>>Dave
>>
>>
>>
>>-----Original Message-----
>>From: Barry Lind [mailto:barry(at)xythos(dot)com]
>>Sent: June 25, 2001 11:39 AM
>>To: Dave(at)micro-automation(dot)net
>>Cc: 'PostgreSQL jdbc list'
>>Subject: Re: [ADMIN] High memory usage [PATCH]
>>
>>Dave,
>>
>>The patch I submitted is thread safe. The "if (df==null)" check is
>>dealing with a ThreadLocal variable. By definition a ThreadLocal
>>variable can't be used by any other thread since each thread has its own
>>
>>copy. (Unless the code goes and hands the object off to some other
>>thread thus causing threading issues, which it doesn't in this case).
>>Thus I believe the patch I submitted is perfectly thread safe.
>>
>>To your more general questions:
>>
>>I think thread safety is very important for all of the JDBC objects.
>>There are no restrictions placed on what a user could do with these
>>objects. It is perfectly legal to create a Statement in one thread,
>>hand that statement off to two or more other threads to access. Now I
>>wouldn't recommend doing this, but the spec permits it.
>>
>>As far as procedures and voting, I also believe that something needs to
>>be done for the JDBC code base. Since Peter has apparently disappeared
>>(I haven't seen a post from him in about two months, and the
>>jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
>>
>>doesn't even have the 7.1 code yet) I think the core group needs to step
>>
>>in and provide some direction for the JDBC code base. Whether that is
>>appointing someone new as the official/unofficial JDBC guru or adopting
>>some other process, I don't know. What I do know is that the JDBC code
>>base is suffering from lack of attention and direction.
>>
>>Issues that I am concerned about in the JDBC code base are:
>> - get/setXXXStream methods are not really spec compliant
>> - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
>>without backward compatibility problems because the get/setBytes methods
>>
>>currently assume that binary means BLOB.
>> - performance/performance/performance - lots of work could/should be
>>done to improve performance. Some good work was started last year but
>>nothing came of it.
>> - updateable cursors - I beleive some work is being done here by
>>various parties on the list, but I have some serious concerns about
>>if/how such functionality can/should be supported.
>>
>>thanks,
>>--Barry
>>
>>
>>Dave Cramer wrote:
>>
>>
>>>Barry,
>>>
>>>My patch was attempting to maintain some sort of thread safety. I do
>>>
>>not
>>
>>>think the if (df==null) test is thread-safe. It would need to be
>>>synchronized.
>>>
>>>However, as I have mentioned in a couple of previous posts; I'm not
>>>
>>sure
>>
>>>thread-safety is worthwhile. The driver appears to be thread safe in
>>>that multiple threads can each use different instances of connections,
>>>and statements, and resultset's however I don't think it is thread
>>>
>>safe
>>
>>>in the sense that multiple threads could use the same instance of the
>>>above objects. The JDBC guide suggests that the driver s/b threadsafe
>>>
>>in
>>
>>>this sense (multiple threads....same object). The guide suggests that
>>>one thread may instigate the retrieval of a result set, and another
>>>would display it.
>>>
>>>
>>>Where this is leading is: What kind of thread safety are we trying to
>>>achieve?
>>>
>>>If it's only one instance per thread then, I would have to agree that
>>>Barry's patch s/b applied.
>>>
>>>P.S.
>>>
>>>Is there a formal process within the postgres group for making these
>>>kind of decisions.
>>>
>>>If not I would like to suggest adopting the apache groups +1,-1 voting
>>>approach.
>>>
>>>-----Original Message-----
>>>From: Barry Lind [mailto:blind(at)xythos(dot)com]
>>>Sent: June 25, 2001 12:37 AM
>>>To: pgsql-patches(at)postgresql(dot)org
>>>Cc: Dave(at)micro-automation(dot)net; 'PostgreSQL jdbc list'
>>>Subject: Re: [ADMIN] High memory usage [PATCH]
>>>
>>>Since this patch got applied before I got around to commenting on it,
>>>
>>I
>>
>>>have submitted a new patch to address my issues with the original
>>>
>>patch.
>>
>>>The problem with the patch as applied is that is always creates the
>>>SimpleDateFormat objects in the constructor of the PreparedStatement
>>>regardless of whether or not they will ever be used in the
>>>PreparedStatement. I have reverted back to the old behavior that only
>>>
>>>creates them if necessary in the setDate and setTimestamp methods.
>>>
>>>I also removed the close() method. It's only purpose was to free
>>>
>>these
>>
>>>two SimpleDateFormat objects. I think it is much better to leave
>>>
>>these
>>
>>>two objects cached on the thread so that other PreparedStatements can
>>>use them. (This was the intention of a patch I submitted back in
>>>February where I was trying to remove as many object creations as
>>>possible to improve performance. That patch as written needed to get
>>>pulled because of the problem that SimpleDataFormat objects are not
>>>thread safe. Peter then added the ThreadLocal code to try to solve
>>>
>>the
>>
>>>performance problem, but introduced the memory leak that originated
>>>
>>this
>>
>>>email thread.) I think the cost of at most two SimpleDateFormat
>>>
>>objects
>>
>>>being cached on each thead is worth the benefits of less object
>>>
>>creation
>>
>>>and subsequent garbage collection.
>>>
>>>thanks,
>>>--Barry
>>>
>>>
>>>Bruce Momjian wrote:
>>>
>>>
>>>
>>>>Patch applied. Thanks.
>>>>
>>>>
>>>>
>>>>
>>>>>Here is a patch which inspired by Michael Stephens that should work
>>>>>
>>>>>Dave
>>>>>
>>>>>
>>>>>-----Original Message-----
>>>>>From: pgsql-jdbc-owner(at)postgresql(dot)org
>>>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar R?nning
>>>>>Sent: June 22, 2001 10:14 AM
>>>>>To: Rainer Mager
>>>>>Cc: Dave Cramer; Bruce Momjian; PostgreSQL jdbc list
>>>>>Subject: Re: [JDBC] Re: [ADMIN] High memory usage [PATCH]
>>>>>
>>>>>* "Rainer Mager" <rmager(at)vgkk(dot)com> wrote:
>>>>>|
>>>>>
>>>>>| Interesting. I wasn't aware of this. If question #2 is answered
>>>>>
>>such
>>
>>>>>that
>>>>>| thread safe isn't necessary, then this problem goes away pretty
>>>>>easily. If
>>>>>| thread safety is needed then this would have to be rewritten, I can
>>>>>look
>>>>>| into doing this if you like.
>>>>>
>>>>>Thread safety is required by the spec. Do you have "JDBC API tutorial
>>>>>and
>>>>>reference, 2 ed." from Addison Wesley ? This book contains a section
>>>>>
>>>>>
>>>for
>>>
>>>
>>>>>JDBC driver writers and explains this issue.
>>>>>
>>>>>regards,
>>>>>
>>>>> Gunnar
>>>>>
>>>>>--
>>>>>Gunnar R?nning - gunnar(at)polygnosis(dot)com
>>>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>>
>>>>>---------------------------(end of
>>>>>
>>>>>
>>>broadcast)---------------------------
>>>
>>>
>>>>>TIP 1: subscribe and unsubscribe commands go to
>>>>>
>>>>>
>>>majordomo(at)postgresql(dot)org
>>>
>>>
>>>>[ Attachment, skipping... ]
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 2: you can get off all lists at once with the unregister command
>> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 4: Don't 'kill -9' the postmaster
>>
>>
>

My only thought was that we could leave the existing driver in place for
people who are depending on it. In retrospect this isn't a good idea. I
would recommend breaking existing code to make the driver compatible
with the standard.

--dc--

-----Original Message-----
From: pgsql-jdbc-owner(at)postgresql(dot)org
[mailto:pgsql-jdbc-owner(at)postgresql(dot)org] On Behalf Of Gunnar Rønning
Sent: June 26, 2001 12:32 PM
To: Dave(at)micro-automation(dot)net
Cc: 'Barry Lind'; 'PostgreSQL jdbc list'
Subject: Re: [JDBC] RE: [ADMIN] High memory usage [PATCH]

* "Dave Cramer" <Dave(at)micro-automation(dot)net> wrote:
|
| Regarding the catch-22 with Blob, etc. I think we need to make a harsh
| decision here. Either break the existing code, or create another
driver
| codebase. If we don't do something we will be doomed to
non-compliance.
| This will hurt the driver in the not too distant future. There are a
lot
| of tools out there which the driver needs to be compatible with.
|

I would say that it is better to break existing code that depends on
broken behaviour than being out of sync with the standard. Not following
the JDBC standard is just asking for a fork - there are a lot of
object relational tools out there and if want these to work with
postgresql
we have no choice.

--
Gunnar Rønning - gunnar(at)polygnosis(dot)com
Senior Consultant, Polygnosis AS, http://www.polygnosis.com/

---------------------------(end of broadcast)---------------------------
TIP 4: Don't 'kill -9' the postmaster