Re: [pgsql-patches] Patch to avoid gprofprofilingoverwrites - Mailing list pgsql-patches

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.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@momjian.us>          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 -0000    1.534
--- configure    21 Feb 2007 01:39:46 -0000
***************
*** 314,320 ****
  # include <unistd.h>
  #endif"
  
! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT
exec_prefixprefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir
libdirincludedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS
configure_argsbuild build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls
WANTED_LANGUAGESdefault_port enable_shared enable_rpath enable_debug DTRACE DTRACEFLAGS enable_dtrace CC CFLAGS LDFLAGS
CPPFLAGSac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl with_perl with_python
with_krb5krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP ELF_SYS LDFLAGS_SL AWK
FLEXFLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP STRIP_STATIC_LIB
STRIP_SHARED_LIBYACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags PYTHON
python_versionpython_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6 LIBOBJS
acx_pthread_configPTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS LDAP_LIBS_FE LDAP_LIBS_BE HAVE_POSIX_SIGNALS MSGFMT MSGMERGE
XGETTEXTlocaledir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD
TCL_SHLIB_LD_LIBSNSGMLS JADE have_docbook DOCBOOKSTYLE COLLATEINDEX SGMLSPL vpath_build LTLIBOBJS'
 
  ac_subst_files=''
  
  # Initialize some variables set by options.
--- 314,320 ----
  # include <unistd.h>
  #endif"
  
! ac_subst_vars='SHELL PATH_SEPARATOR PACKAGE_NAME PACKAGE_TARNAME PACKAGE_VERSION PACKAGE_STRING PACKAGE_BUGREPORT
exec_prefixprefix program_transform_name bindir sbindir libexecdir datadir sysconfdir sharedstatedir localstatedir
libdirincludedir oldincludedir infodir mandir build_alias host_alias target_alias DEFS ECHO_C ECHO_N ECHO_T LIBS
configure_argsbuild build_cpu build_vendor build_os host host_cpu host_vendor host_os PORTNAME docdir enable_nls
WANTED_LANGUAGESdefault_port enable_shared enable_rpath enable_debug enable_profiling DTRACE DTRACEFLAGS enable_dtrace
CCCFLAGS LDFLAGS CPPFLAGS ac_ct_CC EXEEXT OBJEXT CPP GCC TAS autodepend INCLUDES enable_thread_safety with_tcl
with_perlwith_python with_krb5 krb_srvtab with_pam with_ldap with_bonjour with_openssl XML2_CONFIG with_zlib EGREP
ELF_SYSLDFLAGS_SL AWK FLEX FLEXFLAGS LN_S LD with_gnu_ld ld_R_works RANLIB ac_ct_RANLIB TAR STRIP ac_ct_STRIP
STRIP_STATIC_LIBSTRIP_SHARED_LIB YACC YFLAGS PERL perl_archlibexp perl_privlibexp perl_useshrplib perl_embed_ldflags
PYTHONpython_version python_configdir python_includespec python_libdir python_libspec python_additional_libs HAVE_IPV6
LIBOBJSacx_pthread_config PTHREAD_CC PTHREAD_LIBS PTHREAD_CFLAGS LDAP_LIBS_FE LDAP_LIBS_BE HAVE_POSIX_SIGNALS MSGFMT
MSGMERGEXGETTEXT localedir TCLSH TCL_CONFIG_SH TCL_INCLUDE_SPEC TCL_LIB_FILE TCL_LIBS TCL_LIB_SPEC TCL_SHARED_BUILD
TCL_SHLIB_LD_LIBSNSGMLS 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-cassert        enable assertion checks (for debugging)
***************
*** 1949,1954 ****
--- 1950,1986 ----
  
  
  #
+ # --enable-profiling enables gcc profiling
+ #
+ 
+ pgac_args="$pgac_args enable_profiling"
+ 
+ # Check whether --enable-profiling or --disable-profiling was given.
+ if test "${enable_profiling+set}" = set; then
+   enableval="$enable_profiling"
+ 
+   case $enableval in
+     yes)
+       :
+       ;;
+     no)
+       :
+       ;;
+     *)
+       { { echo "$as_me:$LINENO: error: no argument expected for --enable-profiling option" >&5
+ echo "$as_me: error: no argument expected for --enable-profiling option" >&2;}
+    { (exit 1); exit 1; }; }
+       ;;
+   esac
+ 
+ else
+   enable_profiling=no
+ 
+ fi;
+ 
+ 
+ 
+ #
  # DTrace
  #
  
***************
*** 3149,3154 ****
--- 3181,3202 ----
    CFLAGS="$CFLAGS -g"
  fi
  
+ # enable profiling if --enable-profiling
+ if test "$enable_profiling" = yes && test "$ac_cv_prog_cc_g" = yes; then
+   if test "$GCC" = yes; then
+ 
+ cat >>confdefs.h <<\_ACEOF
+ #define PROFILE_PID_DIR 1
+ _ACEOF
+ 
+     CFLAGS="$CFLAGS -pg"
+   else
+     { { echo "$as_me:$LINENO: error: --enable-profiling is supported only when using GCC" >&5
+ echo "$as_me: error: --enable-profiling is supported only when using GCC" >&2;}
+    { (exit 1); exit 1; }; }
+   fi
+ fi
+ 
  { echo "$as_me:$LINENO: using CFLAGS=$CFLAGS" >&5
  echo "$as_me: using CFLAGS=$CFLAGS" >&6;}
  
***************
*** 23975,23980 ****
--- 24023,24029 ----
  s,@enable_shared@,$enable_shared,;t t
  s,@enable_rpath@,$enable_rpath,;t t
  s,@enable_debug@,$enable_debug,;t t
+ s,@enable_profiling@,$enable_profiling,;t t
  s,@DTRACE@,$DTRACE,;t t
  s,@DTRACEFLAGS@,$DTRACEFLAGS,;t t
  s,@enable_dtrace@,$enable_dtrace,;t t
Index: configure.in
===================================================================
RCS file: /cvsroot/pgsql/configure.in,v
retrieving revision 1.501
diff -c -c -r1.501 configure.in
*** configure.in    7 Feb 2007 00:28:54 -0000    1.501
--- configure.in    21 Feb 2007 01:39:47 -0000
***************
*** 206,211 ****
--- 206,218 ----
  AC_SUBST(enable_debug)
  
  #
+ # --enable-profiling enables gcc profiling
+ #
+ PGAC_ARG_BOOL(enable, profiling, no,
+               [  --enable-profiling      build with profiling enabled ])
+ AC_SUBST(enable_profiling)
+ 
+ #
  # DTrace
  #
  PGAC_ARG_BOOL(enable, dtrace, no,
***************
*** 297,302 ****
--- 304,320 ----
    CFLAGS="$CFLAGS -g"
  fi
  
+ # enable profiling if --enable-profiling
+ if test "$enable_profiling" = yes && test "$ac_cv_prog_cc_g" = yes; then
+   if test "$GCC" = yes; then
+     AC_DEFINE([PROFILE_PID_DIR], 1, 
+            [Define to 1 to enable profiling. (--enable-profiling)])
+     CFLAGS="$CFLAGS -pg"
+   else
+     AC_MSG_ERROR([--enable-profiling is supported only when using GCC])
+   fi
+ fi
+ 
  AC_MSG_NOTICE([using CFLAGS=$CFLAGS])
  
  # We already have this in Makefile.win32, but configure needs it too
Index: doc/src/sgml/installation.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/installation.sgml,v
retrieving revision 1.284
diff -c -c -r1.284 installation.sgml
*** doc/src/sgml/installation.sgml    19 Feb 2007 18:57:18 -0000    1.284
--- doc/src/sgml/installation.sgml    21 Feb 2007 01:39:48 -0000
***************
*** 1041,1046 ****
--- 1041,1058 ----
        </varlistentry>
  
        <varlistentry>
+        <term><option>--enable-profiling</option></term>
+        <listitem>
+         <para>
+          If using GCC, all programs and libraries are compiled so they
+          can be profiled.  On backend exit, a subdirectory will be created
+          that contains the <filename>gmon.out</> file for use in profiling.
+          This option is for use only with GCC and when doing development work.
+         </para>
+        </listitem>
+       </varlistentry>
+ 
+       <varlistentry>
         <term><option>--enable-cassert</option></term>
         <listitem>
          <para>
Index: src/backend/storage/ipc/ipc.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/storage/ipc/ipc.c,v
retrieving revision 1.95
diff -c -c -r1.95 ipc.c
*** src/backend/storage/ipc/ipc.c    5 Jan 2007 22:19:37 -0000    1.95
--- src/backend/storage/ipc/ipc.c    21 Feb 2007 01:39:49 -0000
***************
*** 110,115 ****
--- 110,145 ----
                                    on_proc_exit_list[on_proc_exit_index].arg);
  
      elog(DEBUG3, "exit(%d)", code);
+ 
+ #ifdef PROFILE_PID_DIR
+     {
+         /*
+          * If we are profiling ourself then gprof's mcleanup() is about
+          * to write out a profile to ./gmon.out.  Since mcleanup() always 
+          * uses a fixed file name, each backend will overwrite earlier
+          * profiles. To fix that, we create a separate subdirectory for
+          * each backend (./gprof/pid) and 'cd' to that subdirectory before
+          * we exit() - that forces mcleanup() to write each profile into
+          * its own directory.  We end up with something like:
+          *    $PGDATA/gprof/8829/gmon.out
+          *    $PGDATA/gprof/8845/gmon.out
+          *        ...
+          *
+          * Note that we do this here instead of in an on_proc_exit() 
+          * callback because we want to ensure that this code executes
+          * last - we don't want to interfere with any other on_proc_exit()
+          * callback.
+          */
+         char gprofDirName[32];
+ 
+         snprintf(gprofDirName, 32, "gprof/%d", (int) getpid());
+         
+         mkdir("gprof", 0777);
+         mkdir(gprofDirName, 0777);
+         chdir(gprofDirName);
+     }
+ #endif
+ 
      exit(code);
  }
  
Index: src/include/pg_config.h.in
===================================================================
RCS file: /cvsroot/pgsql/src/include/pg_config.h.in,v
retrieving revision 1.111
diff -c -c -r1.111 pg_config.h.in
*** src/include/pg_config.h.in    7 Feb 2007 00:28:55 -0000    1.111
--- src/include/pg_config.h.in    21 Feb 2007 01:39:50 -0000
***************
*** 602,607 ****
--- 602,610 ----
  /* A string containing the version number, platform, and C compiler */
  #undef PG_VERSION_STR
  
+ /* Define to 1 to enable profiling. (--enable-profiling) */
+ #undef PROFILE_PID_DIR
+ 
  /* Define to the necessary symbol if this constant uses a non-standard name on
     your system. */
  #undef PTHREAD_CREATE_JOINABLE