Re: [HACKERS] Re: [PATCHES] Try again: S_LOCK reduced contention - Mailing 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.

> 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.

> > 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.

-dg

David Gould           dg@illustra.com            510.628.3783 or 510.305.9468
Informix Software                      300 Lakeside Drive   Oakland, CA 94612
 - A child of five could understand this!  Fetch me a child of five.