Re: [PATCHES] Try again: S_LOCK reduced contentionh]

>
> Bruce Momjian:
> > > > OK, here is my argument for inlining tas().
> > David Gould:
> > > I am going out of town till Monday, so don't have time to give this
> > > the thoughtful response it deserves. I will get back to you on this as
> > > I am not quite convinced, but obviously in the face of this I need to
> > > explain my reasoning.
> >
> > Sure. I just know that the reduction from 0.28 to 0.08 was performed
> > one 0.01 at a time. See the MemSet() macro. I am sure you will hate it
> > too, but it did reduce the number of calls to memset() and reduced
> > wallclock execution time as measured from the client.
>
> This is always how micro-optimization goes, 1% and 2% gains here and there.
> I am very familiar with it.

I said 0.01 seconds at a time, not 1% at a time. At this point, a 0.01
seconds savings is 12%, because the total test takes 0.08 seconds.

>
> Anyhow, I am just back, and it occured to me to ask you for an exact
> reproducable test setup, so that I can run the same thing and play around
> with it a bit. I am not convinced by your result for a number of reasons,
> but rather than just make assertions or speculations, I think I should do a
> little data collection.
>
> Also, if you have any other cases that you believe useful to test with, I
> would be very happy to try those too.
>
> Have you done call graph profile for this, or just flat profiling? I think
> you may find the call graph (gprof) output revealing, although perhaps not
> on this particular topic...

I ran gprof. I did not look at the call graph, just the total number of
calls. We have a very modular system, and the call overhead can get
exessive. gprof shows tas() getting called far more than any other
function. It shows it as 0.01 seconds, on a 0.08 second test! Now, I
realize that gprof measurement is not perfect, but it certainly shows
tas as being called a lot.

The test is easy. Execute this from psql:

select * from pg_type where oid = 234234;

Compile with profiling, run this from psql, and run gprof on the
gmon.out file in pgsql/data/base/testdb.

I don't understand your hesitation. The code WAS inlined. It was
inlined because gprof showed is as being called a lot. Most of them are
ASM anyway, so what does it matter if it sits in a a *.c or *.h file, an
asm() call looks the same in a macro or in a function.

If it makes you feel better, put it in something called tas.h, and add
it as an include in all the files that include s_lock.h, or have
s_lock.h include tas.h.

I am not looking around for 1% optimization. I am using gprof output to
improve things that gprof shows need improving.

>
> One last item, appropos start up time. Illustra uses what we call "Server
> Caching". Basically when a connect is terminated, the backend instead of
> exiting goes into a pool of idle servers. When next a connection comes in for
> the same database, instead of forking and initing a new server, we merely
> reuse the old one. This saves lots of startup time. However, there are some
> problems in practice so we might want to do something "like this only
> different". The idea that occurred to me is to have the postmaster
> "pre-spawn" some servers in each (configurable) database. These would run
> all the initialization and then just wait for a socket to be handed to them.
> The postmaster would during idle time replenish the pool of ready servers.
> I think this might have a lot more impact on startup time than turning things
> into macros...

Sure, it will have a lot more impact than making things into macros, and
I am all for it, but inlining does improve things, and it was a macro
that worked on all platforms before it was changed. (Except
linux/alpha, which has to be a function.)

We have tons of macros already from Berkeley. ctags makes the macros
just as easy for me to reference as functions.

--
Bruce Momjian | 830 Blythe Avenue
maillist(at)candle(dot)pha(dot)pa(dot)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)