Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-17 Thread Peter Eisentraut
Am Donnerstag, 6. März 2008 schrieb Robert Lor:
 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).

Patch applied.  I also added a small sed script that generates the dummy 
probes.h file automatically, so it doesn't need to be manually maintained.

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-14 Thread Dave Page
On Wed, Feb 27, 2008 at 8:32 PM, Peter Eisentraut [EMAIL PROTECTED] wrote:
 Robert Lor wrote:
   3) Note on src/backend/Makefile
  The current rule below does not work. After expansion, utils/probes.d
   needs
  to come right after -s, but currently it shows up at the end after
   all the .o files.

  I fixed that part.  Note again that a buildfarm animal testing the dtrace
  support could be helpful. :)

I've now added an OS X Leopard buildfarm member (antelope - not sure
if there is a pun intended there). Once the DTrace support for Mac is
committed I'll enable it on that machine (please nudge me if I miss
it).


-- 
Dave Page
EnterpriseDB UK Ltd: http://www.enterprisedb.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk

-- 
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-13 Thread Robert Lor

Hi Peter,

Robert Lor wrote:


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



I believe you're the reviewer of this patch.  Any idea when it will be 
applied? As far as I'm concerned, all the issues that were raised have 
been addressed in the latest patch I sent.


Regards,
-Robert

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-06 Thread Bruce Momjian

Your patch has been added to the PostgreSQL unapplied patches list at:

http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---


Robert Lor wrote:
 
 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


 
 --
 Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
 To make changes to your subscription:
 http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches

-- 
  Bruce Momjian  [EMAIL PROTECTED]http://momjian.us
  EnterpriseDB http://postgres.enterprisedb.com

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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-05 Thread Jorgen Austvik - Sun Norway

Peter Eisentraut wrote:
I fixed that part.  Note again that a buildfarm animal testing the dtrace 
support could be helpful. :)


Hi!

Our testing on Solaris Nevada (x86 and SPARC) and Solaris 10 (SPARC) in 
the build farm now runs with --enable-dtrace.


Thanks for the tip!

-J
--

Jørgen Austvik, Software Engineering - QA
Sun Microsystems Database Technology Group
begin:vcard
fn;quoted-printable:J=C3=B8rgen Austvik
n;quoted-printable:Austvik;J=C3=B8rgen
org:Sun Microsystems;Database Technology Group
adr:;;Haakon VII gt. 7b;Trondheim;;NO-7485;Norway
email;internet:[EMAIL PROTECTED]
title:Senior Engineer
tel;work:+47 73 84 21 10 
tel;fax:+47 73 84 21 01
tel;cell:+47 901 97 886
x-mozilla-html:FALSE
url:http://www.sun.com/
version:2.1
end:vcard


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://mail.postgresql.org/mj/mj_wwwusr?domain=postgresql.orgextra=pgsql-patches


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-03-05 Thread Robert Lor


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 -	1.42
+++ src/Makefile	6 Mar 2008 04:25:25 -
@@ -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 -	1.127
+++ src/backend/Makefile	6 Mar 2008 04:25:25 -
@@ -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 -	1.257
+++ src/backend/access/transam/xact.c	6 Mar 2008 04:25:25 -
@@ -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 -	1.181
+++ src/backend/storage/lmgr/lock.c	6 Mar 2008 04:25:26 -
@@ -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 

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Tom Lane wrote:

I agree with Peter.  There are a whole lot of include files that are
needed by way more than 3 .c files, and yet are not folded into
postgres.h.  c.h is right out.
  
My concern is that when we start adding more probes (not just the 
backend), we will have to add the following 5 lines in .c files that use 
the Dtrace macros. This seems intrusive and messy to me instead of in a 
centralized place like c.h. What are the disadvantages for keeping the 
way it is now?


#ifdef ENABLE_DTRACE
#include utils/probes.h
#else
#include utils/probes_null.h
#endif

Regards,
-Robert

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote:

 My concern is that when we start adding more probes (not just the  
 backend), we will have to add the following 5 lines in .c files that use  
 the Dtrace macros. This seems intrusive and messy to me instead of in a  
 centralized place like c.h. What are the disadvantages for keeping the  
 way it is now?

 #ifdef ENABLE_DTRACE
 #include utils/probes.h
 #else
 #include utils/probes_null.h
 #endif

Why can't this block be centralized in probes.h?

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Alvaro Herrera wrote:

Robert Lor wrote:

  
My concern is that when we start adding more probes (not just the  
backend), we will have to add the following 5 lines in .c files that use  
the Dtrace macros. This seems intrusive and messy to me instead of in a  
centralized place like c.h. What are the disadvantages for keeping the  
way it is now?


#ifdef ENABLE_DTRACE
#include utils/probes.h
#else
#include utils/probes_null.h
#endif



Why can't this block be centralized in probes.h?
  
probes.h is auto generated and it can certainly be massaged to include 
the above logic, but I'd like to avoid doing that if possible.


The thinking initially was to make this tracing feature more like a 
framework and make it as simple as possible to add new probes and as 
un-intrusive as possible, that's why I thought and still think that 
putting the includes in c.h makes sense, unless there are obvious 
disadvantages I'm not aware of.


Regards,
-Robert




---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Peter Eisentraut wrote:
Another thing that is concerning me about this new approach is the way the 
probes are named.  For example, we'd now have a call


POSTGRESQL_LWLOCK_ACQUIRE()

in the code.  This does not say we are *tracing* lock aquisition, but it looks 
like a macro that actually acquires a lock.
  

Definitely a valid concern.
I understand that these probe names follow some global naming scheme.  Is it 
hard to change that?  I'd feel more comfortable with, say, 
(D)TRACE_POSTGRESQL_LWLOCK_ACQUIRE().
  
Because the macro is auto generated and follows certain naming 
conventions, prepending TRACE_ will not work. If you did that, the probe 
name will be called postgresql-lwlock-aquire and the provider will be 
trace which is not what we want.


To avoid the confusion, how about just adding a simple comment like /* 
DTrace probe or  Trace point or something similar */  before all 
occurrences of the macro calls?


Regards,
-Robert

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote:
 Alvaro Herrera wrote:

 Why can't this block be centralized in probes.h?
   
 probes.h is auto generated and it can certainly be massaged to include  
 the above logic, but I'd like to avoid doing that if possible.

Hmm, so let's have a third file that's not autogenerated, which is the
file we will use for #includes, and contains just that block.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes:
 Peter Eisentraut wrote:
 I understand that these probe names follow some global naming scheme.  Is it 
 hard to change that?  I'd feel more comfortable with, say, 
 (D)TRACE_POSTGRESQL_LWLOCK_ACQUIRE().
 
 Because the macro is auto generated and follows certain naming 
 conventions, prepending TRACE_ will not work. If you did that, the probe 
 name will be called postgresql-lwlock-aquire and the provider will be 
 trace which is not what we want.

Ugh.  Is this tool really so badly designed that it thinks it has
ownership of the *entire* namespace within the target program?

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Alvaro Herrera [EMAIL PROTECTED] writes:
 Robert Lor wrote:
 probes.h is auto generated and it can certainly be massaged to include  
 the above logic, but I'd like to avoid doing that if possible.

 Hmm, so let's have a third file that's not autogenerated, which is the
 file we will use for #includes, and contains just that block.

Or just two files.  Call probes_null something else, have it be included
where needed, have it include the autogenerated file when appropriate.

regards, tom lane

---(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] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Tom Lane wrote:

Alvaro Herrera [EMAIL PROTECTED] writes:
  

Hmm, so let's have a third file that's not autogenerated, which is the
file we will use for #includes, and contains just that block.



Or just two files.  Call probes_null something else, have it be included
where needed, have it include the autogenerated file when appropriate.

  


I really like this idea. So, we're now back to having  pg_trace.h which 
includes the autogenerated probes.h when Dtrace is enabled, else null 
macros will be used.


Currently, pg_trace.h is included in c.h, and I feel strongly that it 
should remains there because by design I'd like to
1)  have the tracing feature be available both in the frontend and 
backend without having to do anything extra, which also means that 
probes.h needs to be generated before any compilation
2)  centralize the include of this header just in case the 
implementation needs to be changed for some reason (eg, if this file 
needs to be splitted, etc)
3)  reduce the number of changes to a minimal when adding new probes to 
new .c files


I haven't heard any major disadvantages about keeping it in c.h, but if 
you are still adamant about keeping it out of c.h, I'll will go along 
with that.


Regards,
-Robert

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes:
 I haven't heard any major disadvantages about keeping it in c.h, but if 
 you are still adamant about keeping it out of c.h, I'll will go along 
 with that.

I was thinking that pg_trace.h involved some backend-only code, but
I'm not sure why I thought that :-(.  Yeah, your plan to do it by
restructuring the contents of pg_trace.h sounds fine.

We still have what I consider a big problem with the names of the
macros.  Perhaps that could be fixed by passing the auto-generated
file through a sed script to put a prefix on the macro names before
we start to use it?

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Peter Eisentraut
Am Freitag, 29. Februar 2008 schrieb Robert Lor:
 My concern is that when we start adding more probes (not just the
 backend), we will have to add the following 5 lines in .c files that use
 the Dtrace macros.

I had already solved this in my intermediate patch I sent you by symlinking 
probes_null.h to probes.h.

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
   subscribe-nomail command to [EMAIL PROTECTED] so that your
   message can get through to the mailing list cleanly


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Peter Eisentraut
Am Freitag, 29. Februar 2008 schrieb Robert Lor:
 Currently, pg_trace.h is included in c.h, and I feel strongly that it
 should remains there because by design I'd like to
 1)  have the tracing feature be available both in the frontend and
 backend without having to do anything extra,

I think each component would have its own probes definition file.

 which also means that 
 probes.h needs to be generated before any compilation

Well, you are going to have to do a lot more work on the makefiles if you want 
to do it that way.  Make works by defining dependencies between files, not by 
hoping that people will execute the commands in the order you write them.  If 
you want every single file in the tree to depend on a rule, you will have to 
do something different.

 2)  centralize the include of this header just in case the
 implementation needs to be changed for some reason (eg, if this file
 needs to be splitted, etc)

We have no evidence that anything like that will ever happen.

 3)  reduce the number of changes to a minimal when adding new probes to
 new .c files

These arguments seem irrelevant in my mind.  When you add new function calls, 
you will usually have to add new header files as well.  It's the normal way 
to do things.

 I haven't heard any major disadvantages about keeping it in c.h, but if
 you are still adamant about keeping it out of c.h, I'll will go along
 with that.

Including only what you need is a principle.  It keeps the namespace clean, it 
speads up compilation time, it makes the build system simpler and more 
efficient.  Otherwise we'd only need one header file for everything.

---(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] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Peter Eisentraut wrote:
I had already solved this in my intermediate patch I sent you by symlinking 
probes_null.h to probes.h.


  


Now I see why you created the symlink. But I thinkt the suggestion by 
Tom/Avaro to include probes.h and the content of probes_null.h in a 
separate header (pg_trace.h) is a good one.


Regards,
-Robert

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

   http://www.postgresql.org/about/donate


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes:
 And don't think adding a simple comment before the macro call is 
 sufficient? This can be documented so everyone knows the convention.

It's stupid.  The need for a comment is proof that the macro is badly
named.  I don't intend to hold still for letting poorly-designed tools
dictate our coding conventions.

regards, tom lane

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Tom Lane wrote:

We still have what I consider a big problem with the names of the
macros.  Perhaps that could be fixed by passing the auto-generated
file through a sed script to put a prefix on the macro names before
we start to use it?
  


Post processing the auto generated header may work, but I think it could 
be unnecessarily complicated and error proned. First, the formats of the 
header between Mac OS X and Solaris are different, and I'm pretty sure 
it'll be different for FreeBSD when it comes out with Dtrace support. 
Second, if there are changes in the  content of the header in the 
future, the sed script may break. I can investigate this approach 
further, but I personally prefer a solution this is less error proned.


And don't think adding a simple comment before the macro call is 
sufficient? This can be documented so everyone knows the convention.


Regards,
-Robert

---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Alvaro Herrera
Robert Lor wrote:

 Post processing the auto generated header may work, but I think it could  
 be unnecessarily complicated and error proned.

Would it work to name the traces trace_transaction__start etc instead?
AFAICS that would cause the macros to be named

POSTGRESQL_TRACE_TRANSACTION_START()

which is not ideal but at least it's a bit more obvious what it's all
about.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Peter Eisentraut wrote:

I think each component would have its own probes definition file.
  


A while back when we met in Toronto, the consensus was to only have one 
provider called postgresql and all probes whether they be from the 
backend or frontend will be grouped together in this one provider.


Well, you are going to have to do a lot more work on the makefiles if you want 
to do it that way.  Make works by defining dependencies between files, not by 
hoping that people will execute the commands in the order you write them.  If 
you want every single file in the tree to depend on a rule, you will have to 
do something different.


  
Actually, I was able to get it to work without doing much based on your 
patch.  Please comment on this updated patch I submitted yesterday

http://archives.postgresql.org/pgsql-patches/2008-02/msg00173.php

Including only what you need is a principle.  It keeps the namespace clean, it 
speads up compilation time, it makes the build system simpler and more 
efficient.  Otherwise we'd only need one header file for everything.
  


Okay, will move the header into individual .c files.

Regards,
-Robert


---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-29 Thread Robert Lor

Alvaro Herrera wrote:

Would it work to name the traces trace_transaction__start etc instead?
AFAICS that would cause the macros to be named

POSTGRESQL_TRACE_TRANSACTION_START()
  
Correct, and that would work. With this approach, all the probe names 
will start with trace-, and this particular one will be called 
trace-transaction-start and can be used this way.


postgresql*:::trace-transaction-start
{
...
}

Actually, it reads better to me than having trace in front. If the above 
macro name is acceptable, I'll take this route.



Regards,
-Robert

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

   http://www.postgresql.org/about/donate


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-28 Thread Robert Lor

Peter,

Robert Lor wrote:

Peter Eisentraut wrote:
I have reworked your build rules so they look more like the idioms 
that we already use for other similar cases.  This should fix the 
troubles you describe and others.
  
There are a couple of problems with your updated patch: 
Based on your patch, I made a few changes and everything works now.  
Patch attached!


Thanks for your help!

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 -	1.42
+++ src/Makefile	28 Feb 2008 21:41:49 -
@@ -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 -	1.127
+++ src/backend/Makefile	28 Feb 2008 21:41:49 -
@@ -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 -	1.257
+++ src/backend/access/transam/xact.c	28 Feb 2008 21:41:49 -
@@ -1479,7 +1479,7 @@ StartTransaction(void)
 	Assert(MyProc-backendId == vxid.backendId);
 	MyProc-lxid = vxid.localTransactionId;
 
-	PG_TRACE1(transaction__start, vxid.localTransactionId);
+	POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);
 
 	/*
 	 * set transaction_timestamp() (a/k/a now()).  We want this to be the same
@@ -1604,7 +1604,7 @@ CommitTransaction(void)
 	 */
 	latestXid = RecordTransactionCommit();
 
-	PG_TRACE1(transaction__commit, MyProc-lxid);
+	POSTGRESQL_TRANSACTION_COMMIT(MyProc-lxid);
 
 	/*
 	 * Let others know about no transaction in progress by me. Note that this
@@ -1990,7 +1990,7 @@ AbortTransaction(void)
 	 */
 	latestXid = RecordTransactionAbort(false);
 
-	PG_TRACE1(transaction__abort, MyProc-lxid);
+	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 -	1.181
+++ src/backend/storage/lmgr/lock.c	28 Feb 2008 21:41:50 -
@@ -787,11 +787,11 @@ LockAcquire(const LOCKTAG *locktag,
 		 * Sleep till someone wakes me up.
 		 */
 
-		PG_TRACE2(lock__startwait, locktag-locktag_field2, lockmode);
+		POSTGRESQL_LOCK_STARTWAIT(locktag-locktag_field2, lockmode);
 
 		WaitOnLock(locallock, owner);
 
-		PG_TRACE2(lock__endwait, locktag-locktag_field2, lockmode);
+		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 -	1.50
+++ src/backend/storage/lmgr/lwlock.c	28 Feb 2008 21:41:50 -
@@ -447,7 +447,7 @@ LWLockAcquire(LWLockId lockid, 

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-28 Thread Peter Eisentraut
Robert Lor wrote:
 dtrace call in src/Makefile is to generate probes.h before any file is
 compiled so it can be used in c.h to avoid probes.h not found error.
 The dtrace call in src/backend/Makefile is only needed for Solaris.

Is c.h the right place to include this?  The probes are only in the backend 
code, so perhaps postgres.h would be more appropriate.  Or just include it in 
the .c files that need it, of which there are only 3.

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-28 Thread Robert Lor

Peter Eisentraut wrote:
Is c.h the right place to include this?  The probes are only in the backend 
code, so perhaps postgres.h would be more appropriate.  Or just include it in 
the .c files that need it, of which there are only 3.
  
I think putting probes.h in c.h is the right place. It's true that the 
probes are only in the backend now and only in a few files, but in the 
future I can foresee probes added to more files in the backend, the 
procedural language code or any of the commands (initdb, psql, etc).


Regards,
-Robert


---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-28 Thread Tom Lane
Robert Lor [EMAIL PROTECTED] writes:
 Peter Eisentraut wrote:
 Is c.h the right place to include this?  The probes are only in the backend 
 code, so perhaps postgres.h would be more appropriate.  Or just include it 
 in 
 the .c files that need it, of which there are only 3.
 
 I think putting probes.h in c.h is the right place. It's true that the 
 probes are only in the backend now and only in a few files, but in the 
 future I can foresee probes added to more files in the backend, the 
 procedural language code or any of the commands (initdb, psql, etc).

I agree with Peter.  There are a whole lot of include files that are
needed by way more than 3 .c files, and yet are not folded into
postgres.h.  c.h is right out.

regards, tom lane

---(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] DTrace probe patch for OS X Leopard

2008-02-28 Thread Peter Eisentraut
Robert Lor wrote:
 Please find the patch attached per this thread
 http://archives.postgresql.org/pgsql-hackers/2008-02/msg00912.php

Another thing that is concerning me about this new approach is the way the 
probes are named.  For example, we'd now have a call

POSTGRESQL_LWLOCK_ACQUIRE()

in the code.  This does not say we are *tracing* lock aquisition, but it looks 
like a macro that actually acquires a lock.

I understand that these probe names follow some global naming scheme.  Is it 
hard to change that?  I'd feel more comfortable with, say, 
(D)TRACE_POSTGRESQL_LWLOCK_ACQUIRE().

Comments?

---(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] DTrace probe patch for OS X Leopard

2008-02-27 Thread Alvaro Herrera
Robert Lor wrote:

 3) Note on src/backend/Makefile
   The current rule below does not work. After expansion, utils/probes.d  
 needs
   to come right after -s, but currently it shows up at the end after all 
 the .o files.

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

Perhaps you need a $ there:

   $(DTRACE) $(DTRACEFLAGS) -G -s $ $(call expand_subsys,$^) -o $@

However I think you would also need to $(filter-out) the $ from $^.

-- 
Alvaro Herrerahttp://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Robert Lor

Alvaro Herrera wrote:

Perhaps you need a $ there:

   $(DTRACE) $(DTRACEFLAGS) -G -s $ $(call expand_subsys,$^) -o $@

However I think you would also need to $(filter-out) the $ from $^.

  

Your suggestion with $(filter-out ...) works. Thanks!

Attached is the updated patch.

Regards,
-Robert

? src/include/probes_null.h
Index: src/Makefile
===
RCS file: /projects/cvsroot/pgsql/src/Makefile,v
retrieving revision 1.42
diff -c -r1.42 Makefile
*** src/Makefile	21 Aug 2007 01:11:12 -	1.42
--- src/Makefile	27 Feb 2008 17:09:41 -
***
*** 14,19 
--- 14,22 
  
  
  all install installdirs uninstall distprep:
+ ifeq ($(enable_dtrace), yes)
+ 	$(DTRACE) -h -s $(top_builddir)/src/backend/utils/probes.d -o $(top_builddir)/src/include/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 -c -r1.127 Makefile
*** src/backend/Makefile	26 Feb 2008 14:42:27 -	1.127
--- src/backend/Makefile	27 Feb 2008 17:09:41 -
***
*** 20,28 
--- 20,30 
  
  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
  
***
*** 135,144 
  	cd $(dir $@)  rm -f $(notdir $@)  \
  	$(LN_S) ../../../$(subdir)/utils/fmgroids.h .
  
! 
  utils/probes.o: utils/probes.d $(SUBDIROBJS)
! 	$(DTRACE) $(DTRACEFLAGS) -G -s $(call expand_subsys,$^) -o $@
! 
  
  ##
  
--- 137,146 
  	cd $(dir $@)  rm -f $(notdir $@)  \
  	$(LN_S) ../../../$(subdir)/utils/fmgroids.h .
  
! ifeq ($(PORTNAME), solaris)
  utils/probes.o: utils/probes.d $(SUBDIROBJS)
! 	$(DTRACE) $(DTRACEFLAGS) -G -s $ $(filter-out $, $(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 -c -r1.257 xact.c
*** src/backend/access/transam/xact.c	15 Jan 2008 18:56:59 -	1.257
--- src/backend/access/transam/xact.c	27 Feb 2008 17:09:41 -
***
*** 1479,1485 
  	Assert(MyProc-backendId == vxid.backendId);
  	MyProc-lxid = vxid.localTransactionId;
  
! 	PG_TRACE1(transaction__start, vxid.localTransactionId);
  
  	/*
  	 * set transaction_timestamp() (a/k/a now()).  We want this to be the same
--- 1479,1485 
  	Assert(MyProc-backendId == vxid.backendId);
  	MyProc-lxid = vxid.localTransactionId;
  
! 	POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);
  
  	/*
  	 * set transaction_timestamp() (a/k/a now()).  We want this to be the same
***
*** 1604,1610 
  	 */
  	latestXid = RecordTransactionCommit();
  
! 	PG_TRACE1(transaction__commit, MyProc-lxid);
  
  	/*
  	 * Let others know about no transaction in progress by me. Note that this
--- 1604,1610 
  	 */
  	latestXid = RecordTransactionCommit();
  
! 	POSTGRESQL_TRANSACTION_COMMIT(MyProc-lxid);
  
  	/*
  	 * Let others know about no transaction in progress by me. Note that this
***
*** 1990,1996 
  	 */
  	latestXid = RecordTransactionAbort(false);
  
! 	PG_TRACE1(transaction__abort, MyProc-lxid);
  
  	/*
  	 * Let others know about no transaction in progress by me. Note that this
--- 1990,1996 
  	 */
  	latestXid = RecordTransactionAbort(false);
  
! 	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 -c -r1.181 lock.c
*** src/backend/storage/lmgr/lock.c	2 Feb 2008 22:26:17 -	1.181
--- src/backend/storage/lmgr/lock.c	27 Feb 2008 17:09:42 -
***
*** 787,797 
  		 * Sleep till someone wakes me up.
  		 */
  
! 		PG_TRACE2(lock__startwait, locktag-locktag_field2, lockmode);
  
  		WaitOnLock(locallock, owner);
  
! 		PG_TRACE2(lock__endwait, locktag-locktag_field2, lockmode);
  
  		/*
  		 * NOTE: do not do any material change of state between here and
--- 787,797 
  		 * Sleep till someone wakes me up.
  		 */
  
! 		POSTGRESQL_LOCK_STARTWAIT(locktag-locktag_field2, lockmode);
  
  		WaitOnLock(locallock, owner);
  
! 		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

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Peter Eisentraut
Robert Lor wrote:
 3) Note on src/backend/Makefile
    The current rule below does not work. After expansion, utils/probes.d
 needs
    to come right after -s, but currently it shows up at the end after
 all the .o files.

I fixed that part.  Note again that a buildfarm animal testing the dtrace 
support could be helpful. :)

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

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Peter Eisentraut
Robert Lor wrote:
 Please find the patch attached per this thread
 http://archives.postgresql.org/pgsql-hackers/2008-02/msg00912.php

Why do we have two dtrace calls in the makefiles now?  I understand you added 
the new mechanism to support Mac OS X, but doesn't Solaris support that 
mechanism as well, so the old one could be dropped?

Btw., probes_null.h is missing in your patch.

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

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Peter Eisentraut
Robert Lor wrote:
 2) Need help figuring out how to copy src/backend/util/probes.d from src
 tree to
  bld tree at build time. It works fine if compilation is done in the src
 tree.

I have reworked your build rules so they look more like the idioms that we 
already use for other similar cases.  This should fix the troubles you 
describe and others.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/
Index: src/backend/Makefile
===
RCS file: /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 -	1.127
+++ src/backend/Makefile	27 Feb 2008 21:05:40 -
@@ -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: /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 -	1.257
+++ src/backend/access/transam/xact.c	27 Feb 2008 21:05:41 -
@@ -1479,7 +1479,7 @@ StartTransaction(void)
 	Assert(MyProc-backendId == vxid.backendId);
 	MyProc-lxid = vxid.localTransactionId;
 
-	PG_TRACE1(transaction__start, vxid.localTransactionId);
+	POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);
 
 	/*
 	 * set transaction_timestamp() (a/k/a now()).  We want this to be the same
@@ -1604,7 +1604,7 @@ CommitTransaction(void)
 	 */
 	latestXid = RecordTransactionCommit();
 
-	PG_TRACE1(transaction__commit, MyProc-lxid);
+	POSTGRESQL_TRANSACTION_COMMIT(MyProc-lxid);
 
 	/*
 	 * Let others know about no transaction in progress by me. Note that this
@@ -1990,7 +1990,7 @@ AbortTransaction(void)
 	 */
 	latestXid = RecordTransactionAbort(false);
 
-	PG_TRACE1(transaction__abort, MyProc-lxid);
+	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: /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 -	1.181
+++ src/backend/storage/lmgr/lock.c	27 Feb 2008 21:05:41 -
@@ -787,11 +787,11 @@ LockAcquire(const LOCKTAG *locktag,
 		 * Sleep till someone wakes me up.
 		 */
 
-		PG_TRACE2(lock__startwait, locktag-locktag_field2, lockmode);
+		POSTGRESQL_LOCK_STARTWAIT(locktag-locktag_field2, lockmode);
 
 		WaitOnLock(locallock, owner);
 
-		PG_TRACE2(lock__endwait, locktag-locktag_field2, lockmode);
+		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: /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 -	1.50
+++ src/backend/storage/lmgr/lwlock.c	27 Feb 2008 21:05:41 -
@@ -447,7 +447,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
 		block_counts[lockid]++;
 #endif
 
-		PG_TRACE2(lwlock__startwait, lockid, mode);
+		POSTGRESQL_LWLOCK_STARTWAIT(lockid, mode);
 
 		for (;;)
 		{
@@ -458,7 +458,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
 			extraWaits++;
 		}
 
-		PG_TRACE2(lwlock__endwait, lockid, mode);
+		POSTGRESQL_LWLOCK_ENDWAIT(lockid, mode);
 
 		LOG_LWDEBUG(LWLockAcquire, lockid, awakened);
 
@@ -469,7 +469,7 @@ LWLockAcquire(LWLockId lockid, LWLockMod
 	/* We are done updating shared state of the lock itself. */
 	SpinLockRelease(lock-mutex);
 
-	PG_TRACE2(lwlock__acquire, 

Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Robert Lor

Peter Eisentraut wrote:

Robert Lor wrote:
  

Please find the patch attached per this thread
http://archives.postgresql.org/pgsql-hackers/2008-02/msg00912.php



Why do we have two dtrace calls in the makefiles now?
The build process for Mac OS X is different than that of Solaris. The 
dtrace call in src/Makefile is to generate probes.h before any file is 
compiled so it can be used in c.h to avoid probes.h not found error. 
The dtrace call in src/backend/Makefile is only needed for Solaris.
  I understand you added 
the new mechanism to support Mac OS X, but doesn't Solaris support that 
mechanism as well, so the old one could be dropped?
  

Both are needed.

Btw., probes_null.h is missing in your patch.

  

I included this file (separate from the patch) in the first email.

Regards,
-Robert


---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] DTrace probe patch for OS X Leopard

2008-02-27 Thread Robert Lor

Peter Eisentraut wrote:
I have reworked your build rules so they look more like the idioms that we 
already use for other similar cases.  This should fix the troubles you 
describe and others.
  

There are a couple of problems with your updated patch:

1) utils/probes.h needs to be generated before any file is compiled 
since it's used in c.h and a lot of files include c.h. That's why in my 
previous patch, I had a rule to call $(DTRACE) -h -s $ -o $@ in 
src/Makefile, and I don't think it will work putting it in 
src/backend/utils/Makefile. If utils/probes.h doesn't exist, you get 
compilation errors right of the bat.


...
gmake -C port all
gmake[2]: Entering directory `/export/home/pgs/src/pgsql/src/port'
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith -Winline 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing 
-I../../src/port -DFRONTEND -I../../src/include   -c -o isinf.o isinf.c 
-MMD -MP -MF .deps/isinf.Po

In file included from isinf.c:15:
../../src/include/c.h:60:26: utils/probes.h: No such file or directory
gmake[2]: *** [isinf.o] Error 1
gmake[2]: Leaving directory `/export/home/pgs/src/pgsql/src/port'
gmake[1]: *** [all] Error 2
gmake[1]: Leaving directory `/export/home/pgs/src/pgsql/src'
gmake: *** [all] Error 2

It there a simple way to link to src/backend/utils/probes.d from a build 
tree and put the generated probes.h in src/include/utils instead of 
generate probes.h in src/backend/utils and link to in from 
src/include/utils?


2) c.h needs to have an ifdef like below. probes_null.h is attached, and 
this file is static and is used in the case Dtrace is not enabled.


#ifdef ENABLE_DTRACE
#include utils/probes.h
#else
#include utils/probes_null.h
#endif


Regards,
-Robert
/* --
 *  probes_null.h
 *
 *  Definitions of probe macros used when DTrace is not enabled.
 *
 *  Copyright (c) 2006-2008, PostgreSQL Global Development Group
 *
 * --
 */

#ifndef _PROBES_NULL_H
#define _PROBES_NULL_H

#define POSTGRESQL_LOCK_ENDWAIT(arg0, arg1)
#define POSTGRESQL_LOCK_ENDWAIT_ENABLED()

#define POSTGRESQL_LOCK_STARTWAIT(arg0, arg1)
#define POSTGRESQL_LOCK_STARTWAIT_ENABLED()

#define POSTGRESQL_LWLOCK_ACQUIRE(arg0, arg1)
#define POSTGRESQL_LWLOCK_ACQUIRE_ENABLED()

#define POSTGRESQL_LWLOCK_CONDACQUIRE(arg0, arg1)
#define POSTGRESQL_LWLOCK_CONDACQUIRE_ENABLED()

#define POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(arg0, arg1)
#define POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL_ENABLED()

#define POSTGRESQL_LWLOCK_ENDWAIT(arg0, arg1)
#define POSTGRESQL_LWLOCK_ENDWAIT_ENABLED()

#define POSTGRESQL_LWLOCK_RELEASE(arg0)
#define POSTGRESQL_LWLOCK_RELEASE_ENABLED()

#define POSTGRESQL_LWLOCK_STARTWAIT(arg0, arg1)
#define POSTGRESQL_LWLOCK_STARTWAIT_ENABLED()

#define POSTGRESQL_TRANSACTION_ABORT(arg0)
#define POSTGRESQL_TRANSACTION_ABORT_ENABLED()

#define POSTGRESQL_TRANSACTION_COMMIT(arg0)
#define POSTGRESQL_TRANSACTION_COMMIT_ENABLED()

#define POSTGRESQL_TRANSACTION_START(arg0)
#define POSTGRESQL_TRANSACTION_START_ENABLED()

#endif  /* _PROBES_NULL_H */

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq