Re: [ADMIN] High memory usage [PATCH]

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