Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
Applied.
---
Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
Tom Lane wrote:
> Bruce Momjian <[EMAIL PROTECTED]> 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 <[EMAIL PROTECTED]> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
Index: configure
===
RCS file: /cvsroot/pgsql/configure,v
retrieving revision 1.534
diff -c -c -r1.534 configure
*** configure 7 Feb 2007 00:28:54 - 1.534
--- configure 21 Feb 2007 01:39:46 -
***
*** 314,320
# include
#endif"
! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB STRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON python_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS acx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS LDAP_LIBS_FE LDAP_LIBS_BE HAVE_POSIX_SIGNALS MSGFMT MSGMERGE XGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD TCL_SHLIB_LD_LIBS NSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS'
ac_subst_files=''
# Initialize some variables set by options.
--- 314,320
# include
#endif"
! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug enable_profiling DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB STRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON python_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS acx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS LDAP_LIBS_FE LDAP_LIBS_BE HAVE_POSIX_SIGNALS MSGFMT MSGMERGE XGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD TCL_SHLIB_LD_LIBS NSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS'
ac_subst_files=''
# Initialize some variables set by options.
***
*** 865,870
--- 865,871
--disable-rpath do not embed shared library search path in executables
--disable-spinlocks do not use spinlocks
--enable-debug build with debugging symbols (-g)
+ --enable-profiling build with profiling enabled
--enable-dtrace build with DTrace support
--enable-depend turn on automatic dependency tracking
--enable-cassertenable asserti
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
Bruce Momjian <[EMAIL PROTECTED]> 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
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
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: > > [EMAIL PROTECTED] 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 <[EMAIL PROTECTED]> http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: configure === RCS file: /cvsroot/pgsql/configure,v retrieving revision 1.534 diff -c -c -r1.534 configure *** configure 7 Feb 2007 00:28:54 - 1.534 --- configure 20 Feb 2007 02:18:22 - *** *** 314,320 # include #endif" ! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB STRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON python_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS acx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS LDAP_LIBS_FE LDAP_LIBS_BE HAVE_POSIX_SIGNALS MSGFMT MSGMERGE XGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD TCL_SHLIB_LD_LIBS NSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS' ac_subst_files='' # Initialize some variables set by options. --- 314,320 # include #endif" ! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT exec_prefix prefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir libdir includedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS configure_args build build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls WANTED_LANGUAGES default_port enable_shared enable_rpath enable_debug enable_profiling DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP ELF_SYS LDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
[EMAIL PROTECTED] 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 [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
> >> 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
Re: [PATCHES] [pgsql-patches] Patch to avoid gprofprofilingoverwrites
> > 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
