[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
Attachments:
0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patchtext/x-patch; charset=UTF-8; name=0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patchDownload+869-865
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
Attachments:
noreindent-0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patchtext/x-patch; charset=UTF-8; name=noreindent-0001-Replace-a-long-chain-of-if-s-in-eval_const_expressio.patchDownload+26-21
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. +