Re: AllocSetReset improvement - Mailing list pgsql-patches

Tom Lane <tgl@sss.pgh.pa.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