Lists: | pgsql-patches |
---|
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Nikhil S <nikhil(dot)sontakke(at)enterprisedb(dot)com> |
Cc: | PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Korry Douglas <korryd(at)enterprisedb(dot)com> |
Subject: | Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites |
Date: | 2007-02-20 02:31:27 |
Message-ID: | 200702200231.l1K2VRp24903@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-patches |
OK, I took Korry's gmon.out patch and Nikhil's configure.in patch and
made a combined version. It seems the gmon.out and -pg flags are
GCC-specific, rather than being platform-specific, so what I did was to
allow --enable-profiling to only work with GCC.
Patch attached.
---------------------------------------------------------------------------
Nikhil S wrote:
> Hi Bruce,
>
> I saw this mail of yours a bit late. Have coded up a patch to accept
> --enable-profiling via configure. It is attached with this mail.
>
> It stores the flag in the src/template/linux file.
>
> If you have not started working on it, maybe you can review this and
> forward it to Korry if this is useful?
>
> Regards,
> Nikhils
>
> Bruce Momjian wrote:
> > korryd(at)enterprisedb(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/rtmp/diff | text/x-diff | 9.4 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Nikhil S <nikhil(dot)sontakke(at)enterprisedb(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Korry Douglas <korryd(at)enterprisedb(dot)com> |
Subject: | Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites |
Date: | 2007-02-20 05:03:28 |
Message-ID: | 19949.1171947808@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-patches |
Bruce Momjian <bruce(at)momjian(dot)us> writes:
> + CFLAGS="$CFLAGS -DPROFILE_PID_DIR -pg ${PROFILE_CFLAGS}"
Kindly use AC_DEFINE instead of random -D in CFLAGS (which is the wrong
place for -D anyway). Also, what exactly is the point here of
PROFILE_CFLAGS? I thought it was supposed to allow substituting
something else for -pg, but you've managed to defeat that.
> + snprintf(gprofDirName, MAXPGPATH, "./gprof/%d", getpid());
getpid is not int everywhere; use a cast. Also, the "./" bits are
silly, and if you ask me so is the MAXPGPATH-sized buffer for a string
that can't exceed 20 or so bytes.
regards, tom lane
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Nikhil S <nikhil(dot)sontakke(at)enterprisedb(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Korry Douglas <korryd(at)enterprisedb(dot)com> |
Subject: | Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites |
Date: | 2007-02-21 01:45:15 |
Message-ID: | 200702210145.l1L1jFX20751@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-patches |
Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>
> > + CFLAGS="$CFLAGS -DPROFILE_PID_DIR -pg ${PROFILE_CFLAGS}"
>
> Kindly use AC_DEFINE instead of random -D in CFLAGS (which is the wrong
> place for -D anyway). Also, what exactly is the point here of
> PROFILE_CFLAGS? I thought it was supposed to allow substituting
> something else for -pg, but you've managed to defeat that.
I can't see the value in having a profile flag that just adds an
environment variable. I am hoping other compilers will supply the flags
they need and we can expand this.
> > + snprintf(gprofDirName, MAXPGPATH, "./gprof/%d", getpid());
>
> getpid is not int everywhere; use a cast. Also, the "./" bits are
> silly, and if you ask me so is the MAXPGPATH-sized buffer for a string
> that can't exceed 20 or so bytes.
Patch updated and attached.
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Attachment | Content-Type | Size |
---|---|---|
/pgpatches/profile | text/x-diff | 10.2 KB |
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Nikhil S <nikhil(dot)sontakke(at)enterprisedb(dot)com>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>, Korry Douglas <korryd(at)enterprisedb(dot)com> |
Subject: | Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites |
Date: | 2007-02-21 15:12:31 |
Message-ID: | 200702211512.l1LFCVW25999@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-patches |
Applied.
---------------------------------------------------------------------------
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >
> > > + CFLAGS="$CFLAGS -DPROFILE_PID_DIR -pg ${PROFILE_CFLAGS}"
> >
> > Kindly use AC_DEFINE instead of random -D in CFLAGS (which is the wrong
> > place for -D anyway). Also, what exactly is the point here of
> > PROFILE_CFLAGS? I thought it was supposed to allow substituting
> > something else for -pg, but you've managed to defeat that.
>
> I can't see the value in having a profile flag that just adds an
> environment variable. I am hoping other compilers will supply the flags
> they need and we can expand this.
>
> > > + snprintf(gprofDirName, MAXPGPATH, "./gprof/%d", getpid());
> >
> > getpid is not int everywhere; use a cast. Also, the "./" bits are
> > silly, and if you ask me so is the MAXPGPATH-sized buffer for a string
> > that can't exceed 20 or so bytes.
>
> Patch updated and attached.
>
> --
> Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
> EnterpriseDB http://www.enterprisedb.com
>
> + If your life is a hard drive, Christ can be your backup. +
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
From: | <korryd(at)enterprisedb(dot)com> |
---|---|
To: | "Bruce Momjian" <bruce(at)momjian(dot)us> |
Cc: | "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Nikhil S" <nikhil(dot)sontakke(at)enterprisedb(dot)com>, "PostgreSQL-patches" <pgsql-patches(at)postgresql(dot)org> |
Subject: | Re: [pgsql-patches] Patch to avoidgprofprofilingoverwrites |
Date: | 2007-02-21 15:38:40 |
Message-ID: | 1172072320.5551.1.camel@sakai.localdomain |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-patches |
> Applied.
Thanks for your help Bruce (and Tom and Nikhil).
-- Korry