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@postgresql.org
>>>> [mailto:pgsql-jdbc-owner@postgresql.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@vgkk.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@polygnosis.com
>>>> Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>> ---------------------------(end of
>>>> broadcast)---------------------------
>>>> TIP 1: subscribe and unsubscribe commands go to
>>>> majordomo@postgresql.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
>>
>>
>
>