Re: Safe memory allocation functions

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