From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-committers(at)postgresql(dot)org |
Subject: | Re: pgsql: Pull up isReset flag from AllocSetContext to MemoryContext struc |
Date: | 2011-05-23 12:01:54 |
Message-ID: | 4DDA4CB2.5030204@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers |
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
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2011-05-23 16:53:31 | pgsql: Install defenses against overflow in BuildTupleHashTable(). |
Previous Message | Andrew Dunstan | 2011-05-23 01:51:50 | pgsql: Remove spurious underscore in name of isolation tester on MSVC. |