Re: pgsql: Pull up isReset flag from AllocSetContext to MemoryContext struc

On 22.05.2011 21:18, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)iki(dot)fi> writes:
>> Pull up isReset flag from AllocSetContext to MemoryContext struct. This
>> avoids the overhead of one function call when calling MemoryContextReset(),
>> and it seems like the isReset optimization would be applicable to any new
>> memory context we might invent in the future anyway.
>
>> This buys back the overhead I just added in previous patch to always call
>> MemoryContextReset() in ExecScan, even when there's no quals or projections.
>
> Do you actually have any measurements that prove that?

Yes, I repeated the test I posted earlier in this thread with this patch:

> CREATE TABLE foo AS SELECT generate_series(1,10000000);
>
> I ran "SELECT COUNT(*) FROM foo" many times with \timing on, and took the smallest time with and without the patch. I got:
>
> 1230 ms with the patch
> 1186 ms without the patch

"with the patch" above means with the patch to just add the
MemoryContextReset call, not the pulling up of isReset flag. After the
pull-up-isReset-flag patch I got the time down to 1174 ms, so it does
buy back the slowdown of adding the MemoryContextReset call.

> This seems like
> a rather ugly destruction of a modularity boundary in return for a
> hypothetical performance gain.

It doesn't seem bad from a modularity point of view to me. The
optimization to not do anything if nothing has been allocated since last
reset seems valid across all conceivable memory context implementations.

> I'm also concerned that you've probably
> added cycles on net to MemoryContextAlloc (where it's no longer possible
> to tail-call AllocSetAlloc), which could very easily cost more cycles on
> most workloads than could ever be saved by making MemoryContextReset a
> shade faster.

Good point. That would be solved by clearing the flag before the
AllocSetAlloc() call. I don't see any harm in clearing the flag before
actually doing the allocation.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com