Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)

Поиск
Список
Период
Сортировка
От Andres Freund
Тема Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)
Дата
Msg-id 20130109230507.GA17428@awork2.anarazel.de
обсуждение исходный текст
Ответ на Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)  (Tom Lane <tgl@sss.pgh.pa.us>)
Ответы Re: Re: [PATCH] unified frontend support for pg_malloc et al and palloc/pfree mulation (was xlogreader-v4)  (Andres Freund <andres@2ndquadrant.com>)
Список pgsql-hackers
On 2013-01-09 17:28:35 -0500, Tom Lane wrote:
> Andres Freund <andres@2ndquadrant.com> writes:
> > On 2013-01-09 15:43:19 -0500, Tom Lane wrote:
> >> I had thought of proposing that we code
> >> palloc() like this:
> >>
> >> void *
> >> palloc(Size size)
> >> {
> >>     MemoryContext context = CurrentMemoryContext;
> >>
> >>     AssertArg(MemoryContextIsValid(context));
> >>
> >>     if (!AllocSizeIsValid(size))
> >>         elog(ERROR, "invalid memory alloc request size %lu",
> >>              (unsigned long) size);
> >>
> >>     context->isReset = false;
> >>
> >>     return (*context->methods->alloc) (context, size);
> >> }
> >>
> >> but at least on this specific hardware and compiler that would evidently
> >> be a net loss compared to direct use of CurrentMemoryContext.  I would
> >> not put a lot of faith in that result holding up on other machines
> >> though.
>
> > Thats not optimized to the same? ISTM the compiler should produce
> > exactly the same code for both.
>
> No, that's exactly the point here, you can't just assume that you get
> the same code; this is tense enough that a few instructions matter.
> Remember we're considering non-assert builds, so the AssertArg vanishes.
> So in the form where CurrentMemoryContext is only read after the elog
> call, the compiler can see that it requires but one fetch - it can't
> change within the two lines where it's used.  In the form given above,
> the compiler is required to fetch CurrentMemoryContext into a local
> variable and keep it across the elog call.

I had thought that it could simply store the address in a callee
saved register inside palloc, totally forgetting that palloc would need
to save that register itself in that case.

> (We know this doesn't
> matter, but gcc doesn't; this version of gcc apparently isn't doing much
> with the knowledge that elog won't return.)

Afaics one reason for that is that we don't have any such annotation for
elog(), just for ereport. And I don't immediately see how it could be
added to elog without relying on variadic macros. Bit of a shame, there
probably a good number of callsites that could actually benefit from
that knowledge.
Is it worth making that annotation conditional on variadic macro
support?

Greetings,

Andres Freund

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



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Simon Riggs
Дата:
Сообщение: Re: Index build temp files
Следующее
От: Bruce Momjian
Дата:
Сообщение: 9.2.1 & index-only scans : abnormal heap fetches after VACUUM FULL