Re: Safe memory allocation functions

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Safe memory allocation functions
Date: 2015-01-30 04:07:09
Message-ID: CAB7nPqT_JSP6zg7xmBfBjiSMHQjG-TDFZAuy7TyM_q8ioMVoqA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 30, 2015 at 12:29 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Jan 28, 2015 at 9:34 AM, Michael Paquier
> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> As a result of all the comments on this thread, here are 3 patches
>> implementing incrementally the different ideas from everybody:
>> 1) 0001 modifies aset.c to return unconditionally NULL in case of an
>> OOM instead of reporting an error. All the OOM error reports are moved
>> to mcxt.c (MemoryContextAlloc* and palloc*)
>
> This seems like a good idea, but I think it's pretty clear that the
> MemoryContextStats(TopMemoryContext) calls ought to move along with
> the OOM error report. The stats are basically another kind of
> error-case output, and the whole point here is that the caller wants
> to have control of what happens when malloc fails. Committed that
> way.
Thanks for the clarifications!

>> 2) 0002 adds the noerror routines for frontend and backend.
>
> We don't have consensus on this name; as I read it, Andres and I are
> both strongly opposed to it. Instead of continuing to litigate that
> point, I'd like to propose that we just leave this out. We are
> unlikely to have so many callers who need the no-oom-error behavior to
> justify adding a bunch of convenience functions --- and if that does
> happen, we can resume arguing about the naming then. For now, let's
> just say that if you want that behavior, you should use
> MemoryContextAllocExtended(CurrentMemoryContext, ...).
Fine for me, any extension or module can still define their own
equivalent of palloc_noerror or whatever using the Extended interface.

>> 3) 0003 adds MemoryContextAllocExtended
> I recommend we leave the existing MemoryContextAlloc* functions alone
> and add a new MemoryContextAllocExtended() function *in addition*. I
> think the reason we have multiple copies of this code is because they
> are sufficiently hot to make the effect of a few extra CPU
> instructions potentially material. By having separate copies of the
> code, we avoid introducing extra branches.
Yes, this refactoring was good for testing actually... I have changed
the flags as follows, appending MCXT_ seemed cleaner after waking up
this morning:
+#define MCXT_ALLOC_HUGE 0x01 /* huge allocation */
+#define MCXT_ALLOC_NO_OOM 0x02 /* no failure if
out-of-memory */
+#define MCXT_ALLOC_ZERO 0x04 /* clear allocated
memory using
+
* MemSetAligned */
+#define MCXT_ALLOC_ZERO_ALIGNED 0x08 /* clear allocated memory using
+
* MemSetLoop */
--
Michael

Attachment Content-Type Size
0001-Create-MemoryContextAllocExtended-central-routine-fo.patch text/x-diff 3.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matt Kelly 2015-01-30 04:35:38 Re: Exposing the stats snapshot timestamp to SQL
Previous Message Venkata Balaji N 2015-01-30 03:48:39 Re: Redesigning checkpoint_segments