Remove repeated calls to PQserverVersion

Started by Peter Smithalmost 5 years ago6 messageshackers
Jump to latest
#1Peter Smith
smithpb2250@gmail.com

Hi,

I found a few functions making unnecessary repeated calls to
PQserverVersion(conn); instead of just calling once and assigning to a
local variable.

PSA a little patch which culls those extra calls.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

Attachments:

v1-0001-Avoid-unnecessary-calls-to-PGserverVersion.patchapplication/octet-stream; name=v1-0001-Avoid-unnecessary-calls-to-PGserverVersion.patchDownload+38-26
#2Michael Paquier
michael@paquier.xyz
In reply to: Peter Smith (#1)
Re: Remove repeated calls to PQserverVersion

On Tue, Jul 13, 2021 at 07:02:27PM +1000, Peter Smith wrote:

I found a few functions making unnecessary repeated calls to
PQserverVersion(conn); instead of just calling once and assigning to a
local variable.

Does it really matter? PQserverVersion() does a simple lookup at the
internals of PGconn.
--
Michael

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#2)
Re: Remove repeated calls to PQserverVersion

Michael Paquier <michael@paquier.xyz> writes:

On Tue, Jul 13, 2021 at 07:02:27PM +1000, Peter Smith wrote:

I found a few functions making unnecessary repeated calls to
PQserverVersion(conn); instead of just calling once and assigning to a
local variable.

Does it really matter? PQserverVersion() does a simple lookup at the
internals of PGconn.

Yeah, it'd have to be mighty hot code to be worth caring about that,
and none of these spots look like it could be worth it.

regards, tom lane

#4Peter Smith
smithpb2250@gmail.com
In reply to: Tom Lane (#3)
Re: Remove repeated calls to PQserverVersion

On Wed, Jul 14, 2021 at 12:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael@paquier.xyz> writes:

On Tue, Jul 13, 2021 at 07:02:27PM +1000, Peter Smith wrote:

I found a few functions making unnecessary repeated calls to
PQserverVersion(conn); instead of just calling once and assigning to a
local variable.

Does it really matter? PQserverVersion() does a simple lookup at the
internals of PGconn.

Yeah, it'd have to be mighty hot code to be worth caring about that,
and none of these spots look like it could be worth it.

I agree there would be no observable performance improvements.

But I never made any claims about performance; my motivation for this
trivial patch was more like just "code tidy" or "refactor", so
applying performance as the only worthiness criteria for a "code tidy"
patch seemed like a misrepresentation here.

Of course you can judge the patch is still not worthwhile for other
reasons. So be it.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Peter Smith (#4)
Re: Remove repeated calls to PQserverVersion

On 2021-Jul-14, Peter Smith wrote:

But I never made any claims about performance; my motivation for this
trivial patch was more like just "code tidy" or "refactor", so
applying performance as the only worthiness criteria for a "code tidy"
patch seemed like a misrepresentation here.

Of course you can judge the patch is still not worthwhile for other
reasons. So be it.

Personally, I like the simplicity of the function call in those places,
because when reading just that line one immediately knows where the
value is coming from. If you assign it to a variable, the line is not
standalone and I have to find out where the assignment is, and verify
that there hasn't been any other assignment in between.

--
Álvaro Herrera 39°49'30"S 73°17'W — https://www.EnterpriseDB.com/
"Pido que me den el Nobel por razones humanitarias" (Nicanor Parra)

#6Daniel Gustafsson
daniel@yesql.se
In reply to: Alvaro Herrera (#5)
Re: Remove repeated calls to PQserverVersion

On 14 Jul 2021, at 02:19, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

Personally, I like the simplicity of the function call in those places,

+1

--
Daniel Gustafsson https://vmware.com/