Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-15 Thread Tom Lane
Robert Haas writes: > There are an awful lot of places in our source tree where the error > level is fixed. We could invent a new construct, say ereport_error or > so, that is just like ereport except that it takes no error-level > parameter because it's hard-coded to ERROR. > It would be a bit

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-15 Thread Robert Haas
On Sun, Jan 13, 2013 at 4:16 PM, Tom Lane wrote: > Andres Freund writes: >>> Basically, the aspects of this that I think are likely to be >>> reproducible wins across different platforms are (a) teaching the >>> compiler that elog(ERROR) doesn't return, and (b) reducing code size as >>> much as p

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-13 Thread Tom Lane
Andres Freund writes: >> Basically, the aspects of this that I think are likely to be >> reproducible wins across different platforms are (a) teaching the >> compiler that elog(ERROR) doesn't return, and (b) reducing code size as >> much as possible. The single-function change isn't going to help

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-13 Thread Tom Lane
Andres Freund writes: > On 2013-01-13 14:17:52 -0500, Tom Lane wrote: >> I find these numbers pretty hard to credit. > There are quite some elog(DEBUG*)s in the backend, and those are taken, > so I don't find it unreasonable that doing less work in those cases is > measurable. Meh. If there are

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-13 Thread Tom Lane
Andres Freund writes: > On 2013-01-12 15:39:16 -0500, Tom Lane wrote: >> This is actually a disadvantage of the proposal to replace the abort() >> calls with __builtin_unreachable(), too. The gcc boys interpret the >> semantics of that as "if control reaches here, the behavior is >> undefined" >

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-13 Thread Andres Freund
On 2013-01-12 18:15:17 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2013-01-12 13:16:56 -0500, Tom Lane wrote: > >> However, using a do-block with a local variable is definitely something > >> worth considering. I'm getting less enamored of the __builtin_constant_p > >> idea after findin

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-12 Thread Tom Lane
Andres Freund writes: > On 2013-01-12 13:16:56 -0500, Tom Lane wrote: >> However, using a do-block with a local variable is definitely something >> worth considering. I'm getting less enamored of the __builtin_constant_p >> idea after finding out that the gcc boys seem to have curious ideas >> ab

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-12 Thread Tom Lane
Heikki Linnakangas writes: > On 12.01.2013 20:42, Andres Freund wrote: >>> I don't care for that too much in detail -- if errstart were to return >>> false (it shouldn't, but if it did) this would be utterly broken, > With the current ereport(), we'll call abort() if errstart returns false > and

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-12 Thread Heikki Linnakangas
On 12.01.2013 20:42, Andres Freund wrote: On 2013-01-12 13:16:56 -0500, Tom Lane wrote: Heikki Linnakangas writes: Here's one more construct to consider: #define ereport_domain(elevel, domain, rest) \ do { \ const int elevel_ = elevel; \ if (errst

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-12 Thread Andres Freund
On 2013-01-12 13:16:56 -0500, Tom Lane wrote: > Heikki Linnakangas writes: > > On 11.01.2013 23:49, Tom Lane wrote: > >> Hm ... well, that's a point. I may be putting too much weight on the > >> fact that any such bug for elevel would be a new one that never existed > >> before. What do others t

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-12 Thread Tom Lane
Heikki Linnakangas writes: > On 11.01.2013 23:49, Tom Lane wrote: >> Hm ... well, that's a point. I may be putting too much weight on the >> fact that any such bug for elevel would be a new one that never existed >> before. What do others think? > It sure would be nice to avoid multiple evaluat

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-11 Thread Tom Lane
Andres Freund writes: > On 2013-01-11 16:28:13 -0500, Tom Lane wrote: >> I'm not very satisfied with that answer. Right now, Peter's >> patch has added a class of bugs that never existed before 9.3, and yours >> would add more. It might well be that those classes are empty ... but >> *we can't k

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-11 Thread Tom Lane
Andres Freund writes: > On 2013-01-11 16:16:58 -0500, Tom Lane wrote: >> Uh ... because it's *not* unreachable if elevel < ERROR. Otherwise we'd >> just mark errfinish as __attribute((noreturn)) and be done. Of course, >> that's a gcc-ism too. > Well, I mean with the double evaluation risk. Oh

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-11 Thread Tom Lane
Andres Freund writes: > On 2013-01-11 15:52:19 -0500, Tom Lane wrote: >> I agree the scenario doesn't seem all that probable, but what scares me >> here is that if we use "__builtin_constant_p(elevel) && (elevel) >= ERROR" >> in some builds, and just "(elevel) >= ERROR" in others, then if there is

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-11 Thread Tom Lane
Andres Freund writes: > On 2013-01-11 15:05:54 -0500, Tom Lane wrote: >> And another thing: what if the elevel argument isn't safe for multiple >> evaluation? No such hazard ever existed before these patches, so I'm >> not very comfortable with adding one. (Even if all our own code is >> safe, t

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Tom Lane
Andres Freund writes: > I wonder whether it makes sense to "inline" the contents pstrdup() > additionally? My gut feeling is not, but... I don't recall ever having seen MemoryContextStrdup amount to much, so probably not worth the extra code space. regards, tom lane --

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Tom Lane
Andres Freund writes: > On 2013-01-09 15:43:19 -0500, Tom Lane wrote: >> I had thought of proposing that we code >> palloc() like this: >> >> void * >> palloc(Size size) >> { >> MemoryContext context = CurrentMemoryContext; >> >> AssertArg(MemoryContextIsValid(context)); >> >> if (!

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Tom Lane
Andres Freund writes: > On 2013-01-09 11:57:35 -0500, Tom Lane wrote: >> My objection is that you're forcing *all* backend code to go through >> the trampoline. That probably has a negative impact on performance, and >> if you think it'll get committed without a thorough proof that there's >> no

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Tom Lane
Andres Freund writes: > On 2013-01-09 11:37:46 -0500, Tom Lane wrote: >> I agree that what dirmod is doing is pretty ugly, but it's localized >> enough that I don't care particularly. (Really, the only reason it's >> a hack and not The Solution is that at the moment there's only one >> file doing

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Andres Freund
On 2013-01-09 11:37:46 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2013-01-09 13:46:53 +0200, Heikki Linnakangas wrote: > >> I don't understand the need for this change. Can't you just: > >> #define palloc(s) pg_malloc(s) > >> in the frontend context? > > > Yes, that would be possible, b

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Tom Lane
Andres Freund writes: > On 2013-01-09 13:46:53 +0200, Heikki Linnakangas wrote: >> I don't understand the need for this change. Can't you just: >> #define palloc(s) pg_malloc(s) >> in the frontend context? > Yes, that would be possible, but imo its the inferior solution: I'm with Heikki: in fact

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Andres Freund
On 2013-01-09 13:46:53 +0200, Heikki Linnakangas wrote: > On 09.01.2013 13:27, Andres Freund wrote: > >- makes palloc() into a real function so CurrentMemoryContext doesn't > > need to be provided > > I don't understand the need for this change. Can't you just: > > #define palloc(s) pg_malloc(s)

Re: [HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Heikki Linnakangas
On 09.01.2013 13:27, Andres Freund wrote: - makes palloc() into a real function so CurrentMemoryContext doesn't need to be provided I don't understand the need for this change. Can't you just: #define palloc(s) pg_malloc(s) in the frontend context? - Heikki -- Sent via pgsql-hackers mai

[HACKERS] [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

2013-01-09 Thread Andres Freund
Hi, As promised here's a patch to provide palloc emulation for frontend-ish environments. The patch: - makes palloc() into a real function so CurrentMemoryContext doesn't need to be provided - provides common pg_(malloc,malloc0, realloc, strdup, free) wrappers and removes various versions of