Обсуждение: [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch()

Hi,

While playing around with writing a newer allocator last week (using a 
somewhat slab allocator alike architecture) I found 
eval_const_expressions_mutator to be surprisingly high in profiles.

For unknown reasons the function used non chained ifs for every handled
nodeType.

Replacing the if chain with if; else if; ... resulted in a small
speedup. Replacing it with a switch() in a bigger one.

When testing with a statement containing a longer VALUES statement:
pgbench -M prepared -f stmt -T 10:
orig: 10015.28
if: 10075.48
switch: 10246.52

Thats a 2.3% increase over several runs. Other statements yielded smaller but 
all positive results.

As far as I can see there currently is no point for writing the function as it 
is right now. Am I missing something?

Andres

Andres Freund <andres@anarazel.de> writes:
> For unknown reasons the function used non chained ifs for every handled
> nodeType.
> Replacing the if chain with if; else if; ... resulted in a small
> speedup. Replacing it with a switch() in a bigger one.

Cool, but this patch is impossible to validate by eye.  Could you
resubmit a version that doesn't reindent unchanged code?  Leave it
for pgindent to clean that up later.
        regards, tom lane

On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > For unknown reasons the function used non chained ifs for every handled
> > nodeType.
> > Replacing the if chain with if; else if; ... resulted in a small
> > speedup. Replacing it with a switch() in a bigger one.
> 
> Cool, but this patch is impossible to validate by eye.  Could you
> resubmit a version that doesn't reindent unchanged code?  Leave it
> for pgindent to clean that up later.
Sure. It was just to confusing reading the code without reindenting.

Btw, I found git diff/show/blame -w very useful to view reindent code.

Actually git show -w seems to produce an applyable patch which doesn't 
reindent...

Andres

Andres Freund <andres@anarazel.de> writes:
> On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> Replacing the if chain with if; else if; ... resulted in a small
>>> speedup. Replacing it with a switch() in a bigger one.

>> Cool, but this patch is impossible to validate by eye.  Could you
>> resubmit a version that doesn't reindent unchanged code?  Leave it
>> for pgindent to clean that up later.

> Sure. It was just to confusing reading the code without reindenting.
> Btw, I found git diff/show/blame -w very useful to view reindent code.

Applied, thanks.  Bruce, would you mind running pgindent on
just src/backend/optimizer/util/clauses.c ?  That seems safer than
reindenting manually.
        regards, tom lane

Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
> >> Andres Freund <andres@anarazel.de> writes:
> >>> Replacing the if chain with if; else if; ... resulted in a small
> >>> speedup. Replacing it with a switch() in a bigger one.
> 
> >> Cool, but this patch is impossible to validate by eye.  Could you
> >> resubmit a version that doesn't reindent unchanged code?  Leave it
> >> for pgindent to clean that up later.
> 
> > Sure. It was just to confusing reading the code without reindenting.
> > Btw, I found git diff/show/blame -w very useful to view reindent code.
> 
> Applied, thanks.  Bruce, would you mind running pgindent on
> just src/backend/optimizer/util/clauses.c ?  That seems safer than
> reindenting manually.

Done.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +