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

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