Re: DTrace probe patch for OS X Leopard

Attached is the updated patch which addressed all the concerns from
Peter and Tom.

* The header file containing the probe macros is now included only in
the .c files that need it.
* All probe macro names now start with TRACE_ (eg:
TRACE_POSTGRESQL_TRANSACTION_START).

Regards,
-Robert
Index: src/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/Makefile,v
retrieving revision 1.42
diff -u -3 -p -r1.42 Makefile
--- src/Makefile    21 Aug 2007 01:11:12 -0000    1.42
+++ src/Makefile    6 Mar 2008 04:25:25 -0000
@@ -14,6 +14,9 @@ include Makefile.global


 all install installdirs uninstall distprep:
+ifeq ($(enable_dtrace), yes)
+    $(MAKE) -C backend ../../src/include/utils/probes.h
+endif
     $(MAKE) -C port $@
     $(MAKE) -C timezone $@
     $(MAKE) -C backend $@
Index: src/backend/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/Makefile,v
retrieving revision 1.127
diff -u -3 -p -r1.127 Makefile
--- src/backend/Makefile    26 Feb 2008 14:42:27 -0000    1.127
+++ src/backend/Makefile    6 Mar 2008 04:25:25 -0000
@@ -20,9 +20,11 @@ SUBDIRS = access bootstrap catalog parse

 include $(srcdir)/common.mk

+ifeq ($(PORTNAME), solaris)
 ifeq ($(enable_dtrace), yes)
 LOCALOBJS += utils/probes.o
 endif
+endif

 OBJS = $(SUBDIROBJS) $(LOCALOBJS) $(top_builddir)/src/port/libpgport_srv.a

@@ -122,6 +124,9 @@ $(srcdir)/parser/parse.h: parser/gram.y
 utils/fmgroids.h: utils/Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
     $(MAKE) -C utils fmgroids.h

+utils/probes.h: utils/probes.d
+    $(MAKE) -C utils probes.h
+
 # Make symlinks for these headers in the include directory. That way
 # we can cut down on the -I options. Also, a symlink is automatically
 # up to date when we update the base file.
@@ -135,9 +140,15 @@ $(top_builddir)/src/include/utils/fmgroi
     cd $(dir $@) && rm -f $(notdir $@) && \
         $(LN_S) ../../../$(subdir)/utils/fmgroids.h .

+$(top_builddir)/src/include/utils/probes.h: utils/probes.h
+    cd $(dir $@) && rm -f $(notdir $@) && \
+        $(LN_S) ../../../$(subdir)/utils/probes.h .

+
+ifeq ($(PORTNAME), solaris)
 utils/probes.o: utils/probes.d $(SUBDIROBJS)
     $(DTRACE) $(DTRACEFLAGS) -G -s $(call expand_subsys,$^) -o $@
+endif


 ##########################################################################
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.257
diff -u -3 -p -r1.257 xact.c
--- src/backend/access/transam/xact.c    15 Jan 2008 18:56:59 -0000    1.257
+++ src/backend/access/transam/xact.c    6 Mar 2008 04:25:25 -0000
@@ -46,6 +46,7 @@
 #include "utils/memutils.h"
 #include "utils/relcache.h"
 #include "utils/xml.h"
+#include "pg_trace.h"


 /*
@@ -1479,7 +1480,7 @@ StartTransaction(void)
     Assert(MyProc->backendId == vxid.backendId);
     MyProc->lxid = vxid.localTransactionId;

-    PG_TRACE1(transaction__start, vxid.localTransactionId);
+    TRACE_POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);

     /*
      * set transaction_timestamp() (a/k/a now()).  We want this to be the same
@@ -1604,7 +1605,7 @@ CommitTransaction(void)
      */
     latestXid = RecordTransactionCommit();

-    PG_TRACE1(transaction__commit, MyProc->lxid);
+    TRACE_POSTGRESQL_TRANSACTION_COMMIT(MyProc->lxid);

     /*
      * Let others know about no transaction in progress by me. Note that this
@@ -1990,7 +1991,7 @@ AbortTransaction(void)
      */
     latestXid = RecordTransactionAbort(false);

-    PG_TRACE1(transaction__abort, MyProc->lxid);
+    TRACE_POSTGRESQL_TRANSACTION_ABORT(MyProc->lxid);

     /*
      * Let others know about no transaction in progress by me. Note that this
Index: src/backend/storage/lmgr/lock.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/lmgr/lock.c,v
retrieving revision 1.181
diff -u -3 -p -r1.181 lock.c
--- src/backend/storage/lmgr/lock.c    2 Feb 2008 22:26:17 -0000    1.181
+++ src/backend/storage/lmgr/lock.c    6 Mar 2008 04:25:26 -0000
@@ -41,6 +41,7 @@
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/resowner.h"
+#include "pg_trace.h"


 /* This configuration variable is used to set the lock table size */
@@ -787,11 +788,11 @@ LockAcquire(const LOCKTAG *locktag,
          * Sleep till someone wakes me up.
          */

-        PG_TRACE2(lock__startwait, locktag->locktag_field2, lockmode);
+        TRACE_POSTGRESQL_LOCK_STARTWAIT(locktag->locktag_field2, lockmode);

         WaitOnLock(locallock, owner);

-        PG_TRACE2(lock__endwait, locktag->locktag_field2, lockmode);
+        TRACE_POSTGRESQL_LOCK_ENDWAIT(locktag->locktag_field2, lockmode);

         /*
          * NOTE: do not do any material change of state between here and
Index: src/backend/storage/lmgr/lwlock.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/storage/lmgr/lwlock.c,v
retrieving revision 1.50
diff -u -3 -p -r1.50 lwlock.c
--- src/backend/storage/lmgr/lwlock.c    1 Jan 2008 19:45:52 -0000    1.50
+++ src/backend/storage/lmgr/lwlock.c    6 Mar 2008 04:25:26 -0000
@@ -28,6 +28,7 @@
 #include "storage/ipc.h"
 #include "storage/proc.h"
 #include "storage/spin.h"
+#include "pg_trace.h"


 /* We use the ShmemLock spinlock to protect LWLockAssign */
@@ -447,7 +448,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
         block_counts[lockid]++;
 #endif

-        PG_TRACE2(lwlock__startwait, lockid, mode);
+        TRACE_POSTGRESQL_LWLOCK_STARTWAIT(lockid, mode);

         for (;;)
         {
@@ -458,7 +459,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
             extraWaits++;
         }

-        PG_TRACE2(lwlock__endwait, lockid, mode);
+        TRACE_POSTGRESQL_LWLOCK_ENDWAIT(lockid, mode);

         LOG_LWDEBUG("LWLockAcquire", lockid, "awakened");

@@ -469,7 +470,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
     /* We are done updating shared state of the lock itself. */
     SpinLockRelease(&lock->mutex);

-    PG_TRACE2(lwlock__acquire, lockid, mode);
+    TRACE_POSTGRESQL_LWLOCK_ACQUIRE(lockid, mode);

     /* Add lock to list of locks held by this backend */
     held_lwlocks[num_held_lwlocks++] = lockid;
@@ -540,13 +541,13 @@ LWLockConditionalAcquire(LWLockId lockid
         /* Failed to get lock, so release interrupt holdoff */
         RESUME_INTERRUPTS();
         LOG_LWDEBUG("LWLockConditionalAcquire", lockid, "failed");
-        PG_TRACE2(lwlock__condacquire__fail, lockid, mode);
+        TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(lockid, mode);
     }
     else
     {
         /* Add lock to list of locks held by this backend */
         held_lwlocks[num_held_lwlocks++] = lockid;
-        PG_TRACE2(lwlock__condacquire, lockid, mode);
+        TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(lockid, mode);
     }

     return !mustwait;
@@ -631,7 +632,7 @@ LWLockRelease(LWLockId lockid)
     /* We are done updating shared state of the lock itself. */
     SpinLockRelease(&lock->mutex);

-    PG_TRACE1(lwlock__release, lockid);
+    TRACE_POSTGRESQL_LWLOCK_RELEASE(lockid);

     /*
      * Awaken any waiters I removed from the queue.
Index: src/backend/utils/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/Makefile,v
retrieving revision 1.26
diff -u -3 -p -r1.26 Makefile
--- src/backend/utils/Makefile    19 Feb 2008 10:30:08 -0000    1.26
+++ src/backend/utils/Makefile    6 Mar 2008 04:25:26 -0000
@@ -13,12 +13,25 @@ SUBDIRS     = adt cache error fmgr hash

 include $(top_srcdir)/src/backend/common.mk

+ifeq ($(enable_dtrace), yes)
+all: fmgroids.h probes.h
+else
 all: fmgroids.h
+endif

 $(SUBDIRS:%=%-recursive): fmgroids.h

 fmgroids.h fmgrtab.c: Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
     AWK='$(AWK)' $(SHELL) $< $(top_srcdir)/src/include/catalog/pg_proc.h

+
+ifeq ($(enable_dtrace), yes)
+probes.h: probes.d
+    $(DTRACE) -h -s $< -o $@
+    sed -e "s/POSTGRESQL_/TRACE_POSTGRESQL_/g" $@ > $@.tmp
+    mv $@.tmp $@
+endif
+
+
 clean:
-    rm -f fmgroids.h fmgrtab.c
+    rm -f fmgroids.h fmgrtab.c probes.h
Index: src/include/Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/Makefile,v
retrieving revision 1.23
diff -u -3 -p -r1.23 Makefile
--- src/include/Makefile    14 Oct 2007 17:07:51 -0000    1.23
+++ src/include/Makefile    6 Mar 2008 04:25:26 -0000
@@ -60,7 +60,7 @@ uninstall:


 clean:
-    rm -f utils/fmgroids.h parser/parse.h
+    rm -f utils/fmgroids.h parser/parse.h utils/probes.h

 distclean maintainer-clean: clean
     rm -f pg_config.h dynloader.h pg_config_os.h stamp-h
Index: src/include/c.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/c.h,v
retrieving revision 1.223
diff -u -3 -p -r1.223 c.h
--- src/include/c.h    23 Feb 2008 19:11:45 -0000    1.223
+++ src/include/c.h    6 Mar 2008 04:25:26 -0000
@@ -57,7 +57,6 @@
 #include "pg_config_os.h"        /* must be before any system header files */
 #endif
 #include "postgres_ext.h"
-#include "pg_trace.h"

 #if _MSC_VER >= 1400
 #define errcode __msvc_errcode
Index: src/include/pg_trace.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/pg_trace.h,v
retrieving revision 1.3
diff -u -3 -p -r1.3 pg_trace.h
--- src/include/pg_trace.h    2 Jan 2008 02:42:06 -0000    1.3
+++ src/include/pg_trace.h    6 Mar 2008 04:25:26 -0000
@@ -14,41 +14,51 @@

 #ifdef ENABLE_DTRACE

-#include <sys/sdt.h>
+#include "utils/probes.h"
+
+#else    /* not ENABLE_DTRACE */

 /*
- * The PG_TRACE macros are mapped to the appropriate macros used by DTrace.
+ * Unless DTrace is explicitly enabled with --enable-dtrace, all the probe
+ * macros will be translated to no-ops.
  *
- * Only one DTrace provider called "postgresql" will be used for PostgreSQL,
- * so the name is hard-coded here to avoid having to specify it in the
- * source code.
+ * When adding new probes, two no-ops macros are needed for each probe.
+ * The macro names can be copied from utils/probes.h.
  */

-#define PG_TRACE(name) \
-    DTRACE_PROBE(postgresql, name)
-#define PG_TRACE1(name, arg1) \
-    DTRACE_PROBE1(postgresql, name, arg1)
-#define PG_TRACE2(name, arg1, arg2) \
-    DTRACE_PROBE2(postgresql, name, arg1, arg2)
-#define PG_TRACE3(name, arg1, arg2, arg3) \
-    DTRACE_PROBE3(postgresql, name, arg1, arg2, arg3)
-#define PG_TRACE4(name, arg1, arg2, arg3, arg4) \
-    DTRACE_PROBE4(postgresql, name, arg1, arg2, arg3, arg4)
-#define PG_TRACE5(name, arg1, arg2, arg3, arg4, arg5) \
-    DTRACE_PROBE5(postgresql, name, arg1, arg2, arg3, arg4, arg5)
-#else                            /* not ENABLE_DTRACE */
+#define    TRACE_POSTGRESQL_TRANSACTION_START(arg0)
+#define    TRACE_POSTGRESQL_TRANSACTION_START_ENABLED() (0)

-/*
- * Unless DTrace is explicitly enabled with --enable-dtrace, the PG_TRACE
- * macros will expand to no-ops.
- */
+#define    TRACE_POSTGRESQL_TRANSACTION_COMMIT(arg0)
+#define    TRACE_POSTGRESQL_TRANSACTION_COMMIT_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_TRANSACTION_ABORT(arg0)
+#define    TRACE_POSTGRESQL_TRANSACTION_ABORT_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_STARTWAIT(arg0, arg1)
+#define    TRACE_POSTGRESQL_LWLOCK_STARTWAIT_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_RELEASE(arg0)
+#define    TRACE_POSTGRESQL_LWLOCK_RELEASE_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_ENDWAIT(arg0, arg1)
+#define    TRACE_POSTGRESQL_LWLOCK_ENDWAIT_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(arg0, arg1)
+#define    TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(arg0, arg1)
+#define    TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LWLOCK_ACQUIRE(arg0, arg1)
+#define    TRACE_POSTGRESQL_LWLOCK_ACQUIRE_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LOCK_STARTWAIT(arg0, arg1)
+#define    TRACE_POSTGRESQL_LOCK_STARTWAIT_ENABLED() (0)
+
+#define    TRACE_POSTGRESQL_LOCK_ENDWAIT(arg0, arg1)
+#define    TRACE_POSTGRESQL_LOCK_ENDWAIT_ENABLED() (0)

-#define PG_TRACE(name)
-#define PG_TRACE1(name, arg1)
-#define PG_TRACE2(name, arg1, arg2)
-#define PG_TRACE3(name, arg1, arg2, arg3)
-#define PG_TRACE4(name, arg1, arg2, arg3, arg4)
-#define PG_TRACE5(name, arg1, arg2, arg3, arg4, arg5)
 #endif   /* not ENABLE_DTRACE */

 #endif   /* PG_TRACE_H */