Re: AllocSetReset improvement

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> >> And I'm worried about adding even a small amount of overhead to
> >> palloc/pfree --- on the vast majority of the profiles I look at, those
> >> are more expensive than AllocSetReset.
>
> > I don't worry about palloc. Because overhead increases only when malloc
> > is executed in AllocSetAlloc. But I'm wooried about pfree, too. However,
> > when palloc/pfree was executed many times, I did not see a bad
influence.
>
> In most of the tests I've looked at, palloc/pfree are executed far more
> often than AllocSetReset, and so adding even one instruction there to
> sometimes save a little work in AllocSetReset is a bad tradeoff. You
> can't optimize to make just one test case look good.

I agree. I give up adding instruction to palloc/pfree.

> I have another idea though: in the case you are looking at, I think
> that the context in question never gets any allocations at all, which
> means its blocks list stays null. We could move the MemSet inside the
> "if (blocks)" test --- if there are no blocks allocated to the context,
> it surely hasn't got any chunks either, so the MemSet is unnecessary.
> That should give us most of the speedup without any extra cost in
> palloc/pfree.

It is a reasonable idea. However, the majority part of MemSet was not
able to be avoided by this idea. Because the per-tuple contexts are used
at the early stage of executor.

function that calls number context set->blocks
MemoryContextReset of calls address is null
---------------------------------------------------------------------
execTuplesMatch(execGrouping.c:65) 499995 0x836dd28 false
agg_fill_hash_table(nodeAgg.c:924) 500000 0x836dd28 false
ExecHashJoin(nodeHashjoin.c:108) 500001 0x836dec0 false
ExecHashJoin(nodeHashjoin.c:217) 500000 0x836dec0 false
ExecHashGetHashValue(nodeHash.c:669) 500005 0x836dec0 false
ExecScanHashBucket(nodeHash.c:785) 500000 0x836dec0 false
ExecScan(execScan.c:86) 500007 0x836df48 true

I am considering another idea: I think that we can change behavior of the
context by switching the method table of context.

An simple version of AllocSetAlloc and AllocSetReset is made. These API
can be accelerated instead of using neither a freelist nor the blocks
(The keeper excludes it). When the context is initialized and reset,
these new API is set to the method table. And, when a freelist or a new
block is needed, the method table is switched to normal API. This can be
executed by doing the pfree/repalloc hook. As a result, overhead of pfree
becomes only once about one context.

I think that this idea is effective in context that executes repeatedly
reset after small allocations such as per-tuple context. And I think that
overhead given to the context that executes a lot of palloc/pfree is a
very few.

An attached patch is a draft of that implementation. Test and comment on
the source code are insufficient yet.

regards,

---
Atsushi Ogawa