Re: High Memory Usage Patch -- Disregard my last message - Mailing list pgsql-jdbc

Greetings,

While I haven't participated yet in this discussion, I've been following it
for quite some time (iow, I'm avoiding work I should be doing). I have some
comments on Barry's patch, which I've included at the end of my message to
be sure I'm commenting on the correct patch.

Creating date formatters is fairly expensive compared to doing the actual
formatting, and I think the most frequent use case will be creating several
PreparedStatements to be accessed within a single Thread or occasionally
across multiple Threads. Given that, I would suggest making the
ThreadLocals static members of PreparedStatement, or if other classes could
use them -- ResultSet creates them on the fly for parsing -- putting them
into a DateFormatUtil class.

This would work since no two PreparedStatements could be accessed by the
same Thread concurrently, and two Threads would each get their own copy as
per ThreadLocal's contract. Also, if Thread A creates a PreparedStatement,
and that PS is accessed by several other Threads, under the current scheme
when the PS is closed, only the closing Thread's formatters are removed.
Granted, once the PS is GCed, I suspect the formatters in the other Threads
would also be GCed, but I haven't tested that out.

Finally, the time zone is set in setTimestamp() after each call to
ThreadLocal.get(). Is this necessary? Can't the formatter be setup with the
GMT time zone once at construction time? Or does each call to format() muck
with it?

Anyway, I have just grabbed the 7.1 source and looked over PS.
Unfortunately, I cannot provide an actual "patch" patch. Hopefully Barry or
others can integrate the following into their code if it is deemed useful
and correct.

Peace,
Dave

---------- 8< ------------------------------------------- 8< --------------
...
class PreparedStatement {
   ...
   private static ThreadLocal  tl_df =
       new ThreadLocal() {
         public Object initialValue() {
           return new SimpleDateFormat("''yyyy-MM-dd''");
         }
       };

   private static ThreadLocal  tl_tsdf =
       new ThreadLocal() {
         public Object initialValue() {
           SimpleDateFormat  df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss");
           df.setTimeZone(TimeZone.getTimeZone("GMT"));
           return df;
         }
       };

   ...

   public static SimpleDateFormat getDateFormatter() {
     return (SimpleDateFormat) tl_df.get();
   }

   public static SimpleDateFormat getTimestampFormatter() {
     return (SimpleDateFormat) tl_tsdf.get();
   }

   ...

   public void setDate(int parameterIndex, java.sql.Date x) throws SQLException
   {
     SimpleDateFormat  df = getDateFormatter();

     set(parameterIndex, df.format(x));
     ...
   }

   public void setTimestamp(int parameterIndex, Timestamp x) throws
SQLException
   {
     SimpleDateFormat df = getTimestampFormatter();

     // The following should go either here or in tl_tsdf.initialValue() above
     // if the time zone doesn't need to be reset each time.

     //df.setTimeZone(TimeZone.getTimeZone("GMT"));

     // Use the shared StringBuffer
     ...
   }

   ...
}
---------- 8< ------------------------------------------- 8< --------------

At 10:00 AM 6/26/2001, Barry Lind wrote:
>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
>
>***
>./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