Re: Safe memory allocation functions

On Fri, Jan 30, 2015 at 12:29 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jan 28, 2015 at 9:34 AM, Michael Paquier
> <michael.paquier@gmail.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