Andres Freund <andres@anarazel.de> writes:
> On 2013-01-09 11:57:35 -0500, Tom Lane wrote:
>> My objection is that you're forcing *all* backend code to go through
>> the trampoline. That probably has a negative impact on performance, and
>> if you think it'll get committed without a thorough proof that there's
>> no such impact, you're mistaken. Perhaps you're not aware of exactly
>> how hot a hot-spot palloc is? It's not unlikely that this patch could
>> hurt backend performance by several percent all by itself.
> There is no additional overhead except some minor increase in code size.
> If you look at the patch palloc() now simply directly calls
> CurrentMemoryContext->methods->alloc. So there is no additional function
> call relative to the previous state.
Apparently we're failing to communicate, so let me put this as clearly
as possible: an unsupported assertion that this patch has zero cost
isn't worth the electrons it's written on. We're talking about a place
where single instructions can make a large difference. I see that
you've avoided an extra level of function call by duplicating code,
but there are (at least) two reasons why that could be a loser:
* now there are two hotspots not one, ie both MemoryContextAlloc and
palloc will be competing for L1 cache, likewise for
MemoryContextAllocZero and palloc0;
* depending on machine architecture and compiler optimizations, multiple
fetches of the global variable CurrentMemoryContext are quite likely to
cost more than fetching it once into a parameter register.
As I said, it's possible that this is a wash. But it's very possible
that it isn't.
In any case it's not clear to me why duplicating code like that is a
less ugly approach than having different macro definitions for frontend
and backend.
regards, tom lane
