Skip site navigation (1) Skip section navigation (2)

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

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 (view raw or flat)
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

In response to

Responses

pgsql-committers by date

Next:From: Tom LaneDate: 2011-05-23 16:53:31
Subject: pgsql: Install defenses against overflow in BuildTupleHashTable().
Previous:From: Andrew DunstanDate: 2011-05-23 01:51:50
Subject: pgsql: Remove spurious underscore in name of isolation tester on MSVC.

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group