Re: check for null value before looking up the hash function

Поиск
Список
Период
Сортировка
От David G. Johnston
Тема Re: check for null value before looking up the hash function
Дата
Msg-id CAKFQuwaNO8+RMze=0c+yX+=ocvAO=1Nt8OO7x0=FHn7TEs_XKQ@mail.gmail.com
обсуждение исходный текст
Ответ на Re: check for null value before looking up the hash function  (Ranier Vilela <ranier.vf@gmail.com>)
Ответы Re: check for null value before looking up the hash function  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Список pgsql-hackers
On Sat, May 21, 2022 at 10:04 AM Ranier Vilela <ranier.vf@gmail.com> wrote:
Em sáb., 21 de mai. de 2022 às 13:13, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Ranier Vilela <ranier.vf@gmail.com> writes:
> Em sáb., 21 de mai. de 2022 às 12:05, Tomas Vondra <
> tomas.vondra@enterprisedb.com> escreveu:
>> That's a quite bold claim, and yet you haven't supported it by any
>> argument whatsoever. Trade-offs between complexity and efficiency are a
>> crucial development task, so complicating the logic clearly does matter.

> What I meant is that complicating the logic in search of efficiency is
> worth it, and that's what everyone is looking for in this thread.
> Likewise, not complicating the logic, losing a little bit of efficiency,
> applied to all the code, leads to a big loss of efficiency.
> In other words, I never miss an opportunity to gain efficiency.

[ shrug... ]  You quietly ignored Tomas' main point, which is that no
evidence has been provided that there's actually any efficiency gain.
IMHO, the point here, is for-non-commiters everything needs benchmarks.
But, I have been see many commits withouts benchmarks or any evidence gains.
And of course, having benchmarks is better, but for micro-optimizations,
It doesn't seem like it's needed that much.

Mostly because committers don't tend to do this kind of drive-by patching that changes long-established code, which are fairly categorized as premature optimizations.



(1) Sure, in the case where only null values are encountered during a
query, we can save a cache lookup, but will that be even micro-measurable
compared to general query overhead?  Seems unlikely, especially if this is
changed in only one place.  That ties into my complaint about how this is
just one instance of a fairly widely used coding pattern.
Of course, changing only in one place, the gain is tiny, but, step by step,
the coding pattern is changing too, becoming new "fairly widely".

Agreed, but that isn't what was done here, there was no investigation of the overall coding practice and suggestions to change them all to the improved form.
 

(2) What are the effects when we *do* eventually encounter a non-null
value?  The existing coding will perform all the necessary lookups
at first call, but with the proposed change those may be spread across
query execution.  It's not implausible that that leads to a net loss
of efficiency, due to locality-of-access effects.
Weel the current code, test branch for nulls first.
Most of the time, this is not true.
So, the current code has poor branch prediction, at least.
What I proposed, improves the branch prediction, at least.

Per my other reply, the v3 proposal did not, IMHO, do a good job of branch prediction either.

I find an improvement on code complexity grounds to be warranted, though the benefit seems unlikely to outweigh the cost of doing it everywhere (fixing only one place actually increases the cost component).

Even without the plausible locality-of-access argument the benefit here is likely to be a micro-optimization that provides only minimal benefit.  Evidence to the contrary is welcomed but, yes, the burden is going to be placed squarely on the patch author(s) to demonstrate the benefit accrued from the code churn is worth the cost.

David J.

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Ranier Vilela
Дата:
Сообщение: Re: check for null value before looking up the hash function
Следующее
От: Tom Lane
Дата:
Сообщение: Re: definition of CalculateMaxmumSafeLSN