Re: Do we want a xmalloc or similar function in the Backend?

Lists: pgsql-hackers
From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Do we want a xmalloc or similar function in the Backend?
Date: 2012-06-19 14:17:29
Message-ID: 201206191617.29711.andres@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

There are 70+ calls of malloc in the backend in the form of

type* foo = malloc(sizeof(...));
if(!foo)
elog(ERROR, "could not allocate memory");

which is a bit annoying to write at times. Would somebody argue against
introducing a function that does the above named xmalloc() or malloc_or_die()?

Greetings,

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Do we want a xmalloc or similar function in the Backend?
Date: 2012-06-19 14:38:56
Message-ID: 12259.1340116736@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> There are 70+ calls of malloc in the backend in the form of

> type* foo = malloc(sizeof(...));
> if(!foo)
> elog(ERROR, "could not allocate memory");

> which is a bit annoying to write at times. Would somebody argue against
> introducing a function that does the above named xmalloc() or malloc_or_die()?

99% of the time, you should be using palloc if that's the behavior you
want. I think most of the malloc calls are in places where we want a
bit more control over the error response.

regards, tom lane


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Do we want a xmalloc or similar function in the Backend?
Date: 2012-06-19 14:51:14
Message-ID: 201206191651.14420.andres@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tuesday, June 19, 2012 04:38:56 PM Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > There are 70+ calls of malloc in the backend in the form of
> >
> > type* foo = malloc(sizeof(...));
> > if(!foo)
> >
> > elog(ERROR, "could not allocate memory");
> >
> > which is a bit annoying to write at times. Would somebody argue against
> > introducing a function that does the above named xmalloc() or
> > malloc_or_die()?
>
> 99% of the time, you should be using palloc if that's the behavior you
> want. I think most of the malloc calls are in places where we want a
> bit more control over the error response.
There are surprisingly many calls that just have the above logic without a
more elaborate error message. Its mostly cases which allocate memory just once
and never release it again to avoid having huge static data around for
processes not using that part of the code.
True enough, most of those could use TopMemoryContext, its a rather
established pattern not to do so in those cases though...

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Do we want a xmalloc or similar function in the Backend?
Date: 2012-06-19 17:35:53
Message-ID: CA+TgmoY1obwUcG6obniKRahB1QZcF1cQQhrN=_nZRjT4dgiPow@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Jun 19, 2012 at 10:17 AM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> There are 70+ calls of malloc in the backend in the form of
>
> type* foo = malloc(sizeof(...));
> if(!foo)
>   elog(ERROR, "could not allocate memory");
>
> which is a bit annoying to write at times. Would somebody argue against
> introducing a function that does the above named xmalloc() or malloc_or_die()?

I can't even find 70 malloc calls in the entire backend, let alone 70
with that pattern. Still, I don't think malloc_or_error (not die)
would be a bad idea.

But the error should definitely be written as:

ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));

...not elog.

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


From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Do we want a xmalloc or similar function in the Backend?
Date: 2012-06-19 17:50:08
Message-ID: 201206191950.09204.andres@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tuesday, June 19, 2012 07:35:53 PM Robert Haas wrote:
> On Tue, Jun 19, 2012 at 10:17 AM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:
> > There are 70+ calls of malloc in the backend in the form of
> >
> > type* foo = malloc(sizeof(...));
> > if(!foo)
> > elog(ERROR, "could not allocate memory");
> >
> > which is a bit annoying to write at times. Would somebody argue against
> > introducing a function that does the above named xmalloc() or
> > malloc_or_die()?
>
> I can't even find 70 malloc calls in the entire backend, let alone 70
> with that pattern. Still, I don't think malloc_or_error (not die)
> would be a bad idea.
$ ack '\bmalloc\s*\(' src/backend/|wc -l
70

10-15 or so of those are comments.

The 70+ came from me running on some development branch with commitfest
patches applied...

> But the error should definitely be written as:
>
> ereport(ERROR,
> (errcode(ERRCODE_OUT_OF_MEMORY),
> errmsg("out of memory")));
>
> ...not elog.
Yes, definitely. Currently some of those locations (e.g. in xlog.c) are only
protected by Asserts...

Andres
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services