Re: [HACKERS] Re: [PATCHES] Try again: S_LOCK reduced contention - Mailing list pgsql-hackers
From | Bruce Momjian |
---|---|
Subject | Re: [HACKERS] Re: [PATCHES] Try again: S_LOCK reduced contention |
Date | |
Msg-id | 199806101850.OAA01517@candle.pha.pa.us Whole thread Raw |
In response to | Re: [HACKERS] Re: [PATCHES] Try again: S_LOCK reduced contention (dg@illustra.com (David Gould)) |
List | pgsql-hackers |
> > Bruce Momjian writes: > > David Gould writes: > > > Most of the original tas() __asm__() implementations are GCC specific. This > > > includes all the Linux platforms except PPC, all the *BSD platforms, even the > > > VAX. GCC is also fairly commonly used even on the commercial OSes. > > > > > > As far as I can tell, the only C coded platforms that are not GCC specific > > > are SCO i386 and SunOS/Solaris on Sun3 and Sparc. The other non-GCC platforms > > > have external tas.s function implementations (HP), or have system specific > > > calls (AIX, OSF, SGI, Nextstep). > > > > That s_lock.h file is a hornets nest of portability problems. I really > > don't want to have multiple functions/macros for different CPU's if I > > can help it. I don't even want to mix functions/macros for the same > > function name if I can help it. I also do not want to start playing > > around with isGNU/isnotGNU in a file that is already complex. > > Actually, my main motivation for this file is to reduce the portability > problems. If you will look at the next patch (when I submit it, probably > tonight) I think you will see that it is fairly clear what to do to port to > a new platform, and how the existing platforms work. > > We already implicitly make a isGCC vs notGCC distinction when we use the > GCC asm() syntax. I am merely intending to make it explict. Ah, I see. I wondered how other compilers were understanding the asm() stuff. I thought it was gcc-specific, but then other platforms were using it. I guess they have gcc. > > > Macros work and we already have tons of them, we don't use inline > > anywhere else, and the actual locks are 80% asm code anyway, so it looks > > the same whether it is in a macro or an inline function. > > > > I have made them macros before for this file, so I can do it again quite > > easily. > > > > As for the benefits, well, when I see lots of calls to a function, and I > > try and eliminate the calls if it is reasonable. In many places, the > > call handling is actually more instructions than the inlining. I look > > at the measured performance change vs. the executable size increase and > > make a decision. With something like s_lock, it just seems normal to > > make it a macro. > > With the old S_LOCK this was a reasonable choice. With the new S_LOCK which > is quite a bit more complex, the macro expansion generates quite a bit of > code. See the generated code for the "MacroSLOCK" case in my large post. Yes, I suspected that may be a problem. I will apply your patch as soon as I see it. > > > > Finally, the difference between a tas() function implementation and the best > > > possible inline implementation appears to be only 0.06 microseconds on a P133. > > > This will add 0.0003 seconds to startup. On SCO only. On Sparc this is a leaf > > > call and possibly even cheaper. No other platforms are affected. > > > > > > Remember also that I am adding two features that previously did not exist, > > > backoff, and stuck lock detection. > > > > Yes, and good improvements. > > Again, please have a look at the (forthcoming) patch. It gives up nothing in > either space or time performance compared to the original, is clearer imho, > and incorporates the the new features. Sounds like a plan. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
pgsql-hackers by date: