Re: High Memory Usage Patch -- Disregard my last message

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

(file patch.txt attached)

Bruce Momjian wrote:

> 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(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... ]
>>>>
>>>>
>>>>
>>>>
>>>
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 2: you can get off all lists at once with the unregister command
>> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>>
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 4: Don't 'kill -9' the postmaster
>>
>>
>