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

On 2013-01-09 17:28:35 -0500, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)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