Re: [ADMIN] High memory usage [PATCH]

Dave,

I disagree with your understanding of the double locking issue. You
state "The if (anyvar == null) check is flawed in an SMP environment".
This statement would more correctly read "The if (anyvar == null) check
is flawed in an SMP environment where multiple threads are accessing
anyvar". If only one thread is accessing the variable (as is the case
in this code since the variable is a local variable that comes from a
ThreadLocal object) then there isn't any problem. If the variable in
question here where a member variable, then I would agree with you that
this if check could be problematic.

thanks,
--Barry

Dave Cramer wrote:

> 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(at)xythos(dot)com]
> Sent: June 25, 2001 11:39 AM
> To: Dave(at)micro-automation(dot)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(at)xythos(dot)com]
>>Sent: June 25, 2001 12:37 AM
>>To: pgsql-patches(at)postgresql(dot)org
>>Cc: Dave(at)micro-automation(dot)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(at)postgresql(dot)org
>>>>[mailto:pgsql-jdbc-owner(at)postgresql(dot)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(at)vgkk(dot)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(at)polygnosis(dot)com
>>>>Senior Consultant, Polygnosis AS, http://www.polygnosis.com/
>>>>
>>>>---------------------------(end of
>>>>
>>>>
>>broadcast)---------------------------
>>
>>
>>>>TIP 1: subscribe and unsubscribe commands go to
>>>>
>>>>
>>majordomo(at)postgresql(dot)org
>>
>>
>>>[ Attachment, skipping... ]
>>>
>>>
>>>
>>>
>>
>
>
>