Re: [ADMIN] High memory usage [PATCH] - Mailing list pgsql-patches

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