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: