[HACKERS] Re: [PATCH 1/2] Provide a common malloc wrappers and palloc et al. emulation for frontend'ish environs

2013-01-09 Thread Andres Freund
On 2013-01-09 11:45:12 -0300, Alvaro Herrera wrote:

 How hard is the backend hit by palloc being now an additional function
 call?  Would it be a good idea to make it (and friends) STATIC_IF_INLINE?

  diff --git a/src/include/port/palloc.h b/src/include/port/palloc.h
  new file mode 100644
  index 000..a7900bf
  --- /dev/null
  +++ b/src/include/port/palloc.h
  @@ -0,0 +1,19 @@
  +/*
  + * common.h
  + * Common support routines for bin/scripts/
  + *
  + * Copyright (c) 2003-2013, PostgreSQL Global Development Group
  + *
  + * src/bin/scripts/common.h
  + */

 You forgot to update the above comment.

*headbang*. Gah. I hate these comments... Will update, but won't send
anything again until more changes have accumulated.

Thanks!

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


[HACKERS] Re: [PATCH 1/2] Provide a common malloc wrappers and palloc et al. emulation for frontend'ish environs

2013-01-09 Thread Andres Freund
On 2013-01-09 11:27:46 -0500, Tom Lane wrote:
 Magnus Hagander mag...@hagander.net writes:
  On Wed, Jan 9, 2013 at 1:47 PM, Andres Freund and...@2ndquadrant.com 
  wrote:
  On 2013-01-09 13:34:12 +0100, Magnus Hagander wrote:
  Am I the only one who finds this way of posting patches really annoying?
 
  Well, I unsurprisingly don't ;)
 
  Yeah, that's not surprising :)
 
 I'm with Magnus.  This is seriously annoying, especially when the
 discussion thread has a title not closely related to the patch
 emails.  (It doesn't help any that the list server doesn't manage to
 deliver the emails in order, at least not to me --- more often than
 not, they're spread out over a few minutes and interleaved with other
 messages.)

Ok, most seem to have a clear preference, so I won't do so anymore.
 
 I also don't find the argument that the commit messages are a substitute
 for patch descriptions to be worth anything.  Commit messages are, or
 should be, for a different audience: they are for whoever writes the
 release notes, or for historical purposes when someone is looking for
 which patches touched a particular area?.  That's not the same as
 explaining/justifying the patch for review purposes.  Reviewers want
 a lot more depth than is appropriate in a commit message.

Aggreed that they have different audiences.

 (TBH, I rarely use any submitter's proposed commit message anyway, but
 that's just me.)

I noticed ;)
 
 I'd prefer posting a single message with the discussion and the
 patch(es).  If you think it's helpful to split a patch into separate
 parts for reviewing, add multiple attachments.  But my experience is
 that such separation isn't nearly as useful as you seem to think.

Well, would it have been better if xlog reading, ilist, binaryheap, this
cleanup, etc. have been in the same patch? They have originated out of
the same work...
Even the splitup in this thread seems to have helped as youve jumped on
the patches where you could give rather quick input (static
relpathbackend(), central Assert definitions), probably without having
read the xlogreader patch itself...

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services


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


Re: [HACKERS] Re: [PATCH 1/2] Provide a common malloc wrappers and palloc et al. emulation for frontend'ish environs

2013-01-09 Thread Tom Lane
Andres Freund and...@2ndquadrant.com writes:
 On 2013-01-09 11:27:46 -0500, Tom Lane wrote:
 I'd prefer posting a single message with the discussion and the
 patch(es).  If you think it's helpful to split a patch into separate
 parts for reviewing, add multiple attachments.  But my experience is
 that such separation isn't nearly as useful as you seem to think.

 Well, would it have been better if xlog reading, ilist, binaryheap, this
 cleanup, etc. have been in the same patch? They have originated out of
 the same work...
 Even the splitup in this thread seems to have helped as youve jumped on
 the patches where you could give rather quick input (static
 relpathbackend(), central Assert definitions), probably without having
 read the xlogreader patch itself...

No, I agree that global-impact things like this palloc rearrangement are
much better proposed and debated separately than as part of something
like xlogreader.  What I was reacting to was the specific patch set
associated with this thread.  I don't see the point of breaking out a
two-line sub-patch such as you did in
http://archives.postgresql.org/message-id/1357730830-25999-3-git-send-email-and...@2ndquadrant.com

regards, tom lane


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


Re: [HACKERS] Re: [PATCH 1/2] Provide a common malloc wrappers and palloc et al. emulation for frontend'ish environs

2013-01-09 Thread anara...@anarazel.de


Tom Lane t...@sss.pgh.pa.us schrieb:

Andres Freund and...@2ndquadrant.com writes:
 On 2013-01-09 11:27:46 -0500, Tom Lane wrote:
 I'd prefer posting a single message with the discussion and the
 patch(es).  If you think it's helpful to split a patch into separate
 parts for reviewing, add multiple attachments.  But my experience is
 that such separation isn't nearly as useful as you seem to think.

 Well, would it have been better if xlog reading, ilist, binaryheap,
this
 cleanup, etc. have been in the same patch? They have originated out
of
 the same work...
 Even the splitup in this thread seems to have helped as youve jumped
on
 the patches where you could give rather quick input (static
 relpathbackend(), central Assert definitions), probably without
having
 read the xlogreader patch itself...

No, I agree that global-impact things like this palloc rearrangement
are
much better proposed and debated separately than as part of something
like xlogreader.  What I was reacting to was the specific patch set
associated with this thread.  I don't see the point of breaking out a
two-line sub-patch such as you did in
http://archives.postgresql.org/message-id/1357730830-25999-3-git-send-email-and...@2ndquadrant.com

Ah, yes. I See your point. The not all that good reasoning I had in mind was 
that that one should be uncontroversial as it seemed to be the only unchecked 
malloc call in src/bin. So it could be committed independent from the more 
controversial stuff... Same with the single whitespace removal patch upthread...

Andres

--- 
Please excuse brevity and formatting - I am writing this on my mobile phone.


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