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

From Bruce Momjian
Subject Re: High Memory Usage Patch -- Disregard my last message
Date
Msg-id 200106261513.f5QFDXj07643@candle.pha.pa.us
Whole thread Raw
In response to High Memory Usage Patch -- Disregard my last message  ("Dave Cramer" <Dave@micro-automation.net>)
Responses RE: High Memory Usage Patch -- Disregard my last message
Barry's patch
List pgsql-jdbc
Can someone send me the patch?  I haven't seen any patch from Barry.
How am I missing it?


> Barry,
>
> Your patch is correct, and thread-safe in the light of the next day and
> a good night's sleep.
>
> Cheers,
>
> Dave
>
> Barry,
>
> My understanding of the problem is as follows:
>
> The if (anyvar == null) check is flawed in an SMP environment; according
> to the spec anyvar can be in the process of being created the jvm could
> set anyvar to point to the memory it is going to assign it to but not
> complete the creation of the object. You end up with a non-null, but
> invalid anyvar. This is the crux of the double locking flaw. While I
> hope most compiler writers would be smarter than that, apparently it is
> possible according to the spec.
>
> Well if we make the driver completely thread-safe we are going to take a
> real performance hit. Personally I would prefer to code assuming it is
> not completely thread-safe and have a lightweight driver.
>
> I am willing to take on some of the work for the driver, but I think the
> process s/b a group process. I have learned a lot in this particular
> thread.
> As I already mentioned I would like to see a voting procedure like the
> apache group.
>
> Regarding the catch-22 with Blob, etc. I think we need to make a harsh
> decision here. Either break the existing code, or create another driver
> codebase. If we don't do something we will be doomed to non-compliance.
> This will hurt the driver in the not too distant future. There are a lot
> of tools out there which the driver needs to be compatible with.
>
> I had a look at the updateable cursors and it looks possible. Do you
> know who is working on it?
>
> Dave
>
>
>
> -----Original Message-----
> From: Barry Lind [mailto:barry@xythos.com]
> Sent: June 25, 2001 11:39 AM
> To: Dave@micro-automation.net
> Cc: 'PostgreSQL jdbc list'
> Subject: Re: [ADMIN] High memory usage [PATCH]
>
> Dave,
>
> The patch I submitted is thread safe.  The "if (df==null)" check is
> dealing with a ThreadLocal variable.  By definition a ThreadLocal
> variable can't be used by any other thread since each thread has its own
>
> copy. (Unless the code goes and hands the object off to some other
> thread thus causing threading issues, which it doesn't in this case).
> Thus I believe the patch I submitted is perfectly thread safe.
>
> To your more general questions:
>
> I think thread safety is very important for all of the JDBC objects.
> There are no restrictions placed on what a user could do with these
> objects.  It is perfectly legal to create a Statement in one thread,
> hand that statement off to two or more other threads to access.  Now I
> wouldn't recommend doing this, but the spec permits it.
>
> As far as procedures and voting, I also believe that something needs to
> be done for the JDBC code base.  Since Peter has apparently disappeared
> (I haven't seen a post from him in about two months, and the
> jdbc.postgresql.org website hasn't been updated for about 4 mounths - it
>
> doesn't even have the 7.1 code yet) I think the core group needs to step
>
> in and provide some direction for the JDBC code base.  Whether that is
> appointing someone new as the official/unofficial JDBC guru or adopting
> some other process, I don't know.  What I do know is that the JDBC code
> base is suffering from lack of attention and direction.
>
> Issues that I am concerned about in the JDBC code base are:
>    - get/setXXXStream methods are not really spec compliant
>    - the 'bytea' datatype (i.e. binary) isn't supported, and can't be
> without backward compatibility problems because the get/setBytes methods
>
> currently assume that binary means BLOB.
>    - performance/performance/performance - lots of work could/should be
> done to improve performance.  Some good work was started last year but
> nothing came of it.
>    - updateable cursors - I beleive some work is being done here by
> various parties on the list, but I have some serious concerns about
> if/how such functionality can/should be supported.
>
> thanks,
> --Barry
>
>
> Dave Cramer wrote:
>
> > Barry,
> >
> > My patch was attempting to maintain some sort of thread safety. I do
> not
> > think the if (df==null) test is thread-safe. It would need to be
> > synchronized.
> >
> > However, as I have mentioned in a couple of previous posts; I'm not
> sure
> > thread-safety is worthwhile. The driver appears to be thread safe in
> > that multiple threads can each use different instances of connections,
> > and statements, and resultset's however I don't think it is thread
> safe
> > in the sense that multiple threads could use the same instance of the
> > above objects. The JDBC guide suggests that the driver s/b threadsafe
> in
> > this sense (multiple threads....same object). The guide suggests that
> > one thread may instigate the retrieval of a result set, and another
> > would display it.
> >
> >
> > Where this is leading is: What kind of thread safety are we trying to
> > achieve?
> >
> > If it's only one instance per thread then, I would have to agree that
> > Barry's patch s/b applied.
> >
> > P.S.
> >
> > Is there a formal process within the postgres group for making these
> > kind of decisions.
> >
> > If not I would like to suggest adopting the apache groups +1,-1 voting
> > approach.
> >
> > -----Original Message-----
> > From: Barry Lind [mailto:blind@xythos.com]
> > Sent: June 25, 2001 12:37 AM
> > To: pgsql-patches@postgresql.org
> > Cc: Dave@micro-automation.net; 'PostgreSQL jdbc list'
> > Subject: Re: [ADMIN] High memory usage [PATCH]
> >
> > 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... ]
> >>
> >>
> >>
> >
> >
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

pgsql-jdbc by date:

Previous
From: Michael Stephenson
Date:
Subject: Re: Re: RE: [ADMIN] High memory usage [PATCH]
Next
From: Bruce Momjian
Date:
Subject: Re: Todo/missing? (was Re: [ADMIN] High memory usage [PATCH])