Patch to avoid gprof profiling overwrites

And the patch is where?

You caught me; guess I'd better make something up fast, huh?

Here it is, thanks.

-- Korry

---------------------------------------------------------------------------

korryd@enterprisedb.com wrote:

It's difficult to profile a backend server process (using gprof)
because each process overwrites any earlier profile as it exits.

It is especially tricky to nab a useful profile if you happen to have
autovacuum enabled.

This patch reduces the problem by forcing the backend to 'cd' to a new
directory ($PGDATA/gprof/pid) just before calling exit(), but only if
the backend was compiled with -DLINUX_PROFILE.

I've tested this with Linux, but not with other host architectures.

-- Korry

--
Korry Douglas korryd@enterprisedb.com
EnterpriseDB http://www.enterprisedb.com

--
Korry Douglas korryd@enterprisedb.com
EnterpriseDB http://www.enterprisedb.com

korryd@enterprisedb.com wrote:

And the patch is where?

You caught me; guess I'd better make something up fast, huh?

Here it is, thanks.

-- Korry

---------------------------------------------------------------------------

korryd@enterprisedb.com <mailto:korryd@enterprisedb.com> wrote:

It's difficult to profile a backend server process (using gprof)
because each process overwrites any earlier profile as it exits.

It is especially tricky to nab a useful profile if you happen to have
autovacuum enabled.

This patch reduces the problem by forcing the backend to 'cd' to a new
directory ($PGDATA/gprof/pid) just before calling exit(), but only if
the backend was compiled with -DLINUX_PROFILE.

Nice addition - tested on FreeBSD 6.2, works great!

The name for the define variable could perhaps be better - feels silly
adding -DLINUX_PROFILE on Freebsd! (maybe just PROFILE or GPROF_PROFILE?).

Cheers

Mark

Nice addition - tested on FreeBSD 6.2, works great!

Cool - thanks for the quick feedback.

The name for the define variable could perhaps be better - feels silly
adding -DLINUX_PROFILE on Freebsd! (maybe just PROFILE or GPROF_PROFILE?).

That wasn't my choice, there is other code elsewhere that depends on
that symbol, I just added a little bit more.

-- Korry

korryd@enterprisedb.com wrote:

The name for the define variable could perhaps be better - feels silly
adding -DLINUX_PROFILE on Freebsd! (maybe just PROFILE or GPROF_PROFILE?).

That wasn't my choice, there is other code elsewhere that depends on
that symbol, I just added a little bit more.

Right - but LINUX_PROFILE was added to correct Linux specific oddities
with the time counter accumulation, whereas your patch is not Linux
specific at all. So I think a more representative symbol is required.

Cheers

Mark

Mark Kirkwood wrote:

korryd@enterprisedb.com wrote:

The name for the define variable could perhaps be better - feels
silly adding -DLINUX_PROFILE on Freebsd! (maybe just PROFILE or
GPROF_PROFILE?).

That wasn't my choice, there is other code elsewhere that depends on
that symbol, I just added a little bit more.

Right - but LINUX_PROFILE was added to correct Linux specific oddities
with the time counter accumulation, whereas your patch is not Linux
specific at all. So I think a more representative symbol is required.

In fact - thinking about this a bit more, probably a construction like:

#if defined(LINUX_PROFILE) || defined(PROFILE)

or similar would work - because I think those of us not on Linux do
*not* want to define LINUX_PROFILE, as our timer accumulation for forked
process works fine as it is...but we don't want to make Linux guys have
to define LINUX_PROFILE *and* PROFILE...

Mark Kirkwood <markir@paradise.net.nz> writes:

Right - but LINUX_PROFILE was added to correct Linux specific oddities
with the time counter accumulation, whereas your patch is not Linux
specific at all. So I think a more representative symbol is required.

Yeah, that was my problem with the patch too. The issue that it's
addressing isn't Linux-specific in the least.

Is there a way to detect via #if that profiling is enabled? I wouldn't
expect a really portable answer, but maybe there's something that works
for gcc?

regards, tom lane

On Thu, 2007-02-01 at 00:59 -0500, Tom Lane wrote:

Is there a way to detect via #if that profiling is enabled? I wouldn't
expect a really portable answer, but maybe there's something that works
for gcc?

What about a "--enable-gprof" (or "--enable-profiling"?) configure flag?
This could add the appropriate compiler flags to CFLAGS, enable
LINUX_PROFILE if on Linux, and enable the "gprof/pid" mkdir().

-Neil

Neil Conway wrote:

What about a "--enable-gprof" (or "--enable-profiling"?) configure
flag? This could add the appropriate compiler flags to CFLAGS, enable
LINUX_PROFILE if on Linux, and enable the "gprof/pid" mkdir().

That would really only work for GCC, wouldn't it?

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Right - but LINUX_PROFILE was added to correct Linux specific oddities
with the time counter accumulation, whereas your patch is not Linux
specific at all. So I think a more representative symbol is required.

Yeah, that was my problem with the patch too. The issue that it's
addressing isn't Linux-specific in the least.

Is there a way to detect via #if that profiling is enabled? I wouldn't
expect a really portable answer, but maybe there's something that works
for gcc?

You're right - I hadn't really looked beyond the problem that I was
trying to solve.

The technique should work for gprof on other platforms, but, as you
point out, LINUX_PROFILE is unique to Linux (no, I hadn't noticed that,
but it seems pretty obvious in retrospect).

I like Neil's idea of a using --enable-profiling configure flag. Every
time I need to profile, I have to go search the archives for 'gprof' -
it would be nice to see --enable-profiling when I configure --help.

-- Korry

Peter Eisentraut <peter_e@gmx.net> writes:

Neil Conway wrote:

What about a "--enable-gprof" (or "--enable-profiling"?) configure
flag? This could add the appropriate compiler flags to CFLAGS, enable
LINUX_PROFILE if on Linux, and enable the "gprof/pid" mkdir().

That would really only work for GCC, wouldn't it?

Well, yeah, but that's what many of us use anyway. I would envision it
as adding $(PROFILE) to CFLAGS, and then there would be one place
to adjust "-pg" to something else for another compiler --- perhaps the
template files could be given a chance to change PROFILE to something
else.

regards, tom lane

What about a "--enable-gprof" (or "--enable-profiling"?) configure
flag? This could add the appropriate compiler flags to CFLAGS, enable
LINUX_PROFILE if on Linux, and enable the "gprof/pid" mkdir().

That would really only work for GCC, wouldn't it?

Well, yeah, but that's what many of us use anyway. I would envision it
as adding $(PROFILE) to CFLAGS, and then there would be one place
to adjust "-pg" to something else for another compiler --- perhaps the
template files could be given a chance to change PROFILE to something
else.

I don't feel competent to muck around with configure.in (sorry, I'm not
tying to shirk the work, I've just never had any success in writing
configure/automake/autoconf stuff - I have the "leaping goats" book, but
I need a small meaningful example to start with).

Can someone else volunteer to make this change? And then forward the
patch to me so I can learn something useful about how to change
configure.in without breaking it?

-- Korry

korryd@enterprisedb.com wrote:

What about a "--enable-gprof" (or "--enable-profiling"?) configure
flag? This could add the appropriate compiler flags to CFLAGS, enable
LINUX_PROFILE if on Linux, and enable the "gprof/pid" mkdir().

That would really only work for GCC, wouldn't it?

Well, yeah, but that's what many of us use anyway. I would envision it
as adding $(PROFILE) to CFLAGS, and then there would be one place
to adjust "-pg" to something else for another compiler --- perhaps the
template files could be given a chance to change PROFILE to something
else.

I don't feel competent to muck around with configure.in (sorry, I'm not
tying to shirk the work, I've just never had any success in writing
configure/automake/autoconf stuff - I have the "leaping goats" book, but
I need a small meaningful example to start with).

Can someone else volunteer to make this change? And then forward the
patch to me so I can learn something useful about how to change
configure.in without breaking it?

I can work on this.

--
Bruce Momjian bruce@momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +