Lists: | pgsql-hackers |
---|
From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | pgsql-hackers(at)postgresql(dot)org |
Subject: | [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch() |
Date: | 2011-11-18 20:13:25 |
Message-ID: | 201111182113.25826.andres@anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patch | text/x-patch | 59.4 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch() |
Date: | 2011-11-18 21:14:22 |
Message-ID: | 25027.1321650862@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)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
From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch() |
Date: | 2011-11-18 21:45:31 |
Message-ID: | 201111182245.31216.andres@anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)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
Attachment | Content-Type | Size |
---|---|---|
noreindent-0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patch | text/x-patch | 5.7 KB |
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Bruce Momjian <bruce(at)momjian(dot)us>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch() |
Date: | 2011-11-28 19:23:58 |
Message-ID: | 20898.1322508238@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Andres Freund <andres(at)anarazel(dot)de> writes:
> On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
>> Andres Freund <andres(at)anarazel(dot)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
From: | Bruce Momjian <bruce(at)momjian(dot)us> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: [PATCH] Replace a long chain of if's in eval_const_expressions_mutator by a switch() |
Date: | 2011-11-28 21:47:52 |
Message-ID: | 201111282147.pASLlrB21110@momjian.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)de> writes:
> > On Friday, November 18, 2011 10:14:22 PM Tom Lane wrote:
> >> Andres Freund <andres(at)anarazel(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +