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

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)