Re: Safe memory allocation functions - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Safe memory allocation functions
Date
Msg-id CA+TgmoYLQmc6aYQyJF1X0vRbedYE1sxaj0KFtcxSzzZe9V_BiA@mail.gmail.com
Whole thread Raw
In response to Re: Safe memory allocation functions  (Michael Paquier <michael.paquier@gmail.com>)
Responses Re: Safe memory allocation functions
List pgsql-hackers
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.

> 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, ...).

> 3) 0003 adds MemoryContextAllocExtended that can be called with the
> following control flags:
> #define ALLOC_HUGE             0x01    /* huge allocation */
> #define ALLOC_ZERO             0x02    /* clear allocated memory */
> #define ALLOC_NO_OOM   0x04    /* no failure if out-of-memory */
> #define ALLOC_ALIGNED  0x08    /* request length suitable for MemSetLoop */
> This groups MemoryContextAlloc, MemoryContextAllocHuge,
> MemoryContextAllocZero and MemoryContextAllocZeroAligned under the
> same central routine.

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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: pg_upgrade and rsync
Next
From: Noah Misch
Date:
Subject: Re: jsonb, unicode escapes and escaped backslashes