Avoid is possible a expensive function call (src/backend/utils/adt/varlena.c)
Hi,
The function var_strcmp is a critical function.
Inside the function, there is a shortcut condition,
which allows for a quick exit.
Unfortunately, the current code calls a very expensive function beforehand,
which if the test was true, all the call time is wasted.
So, IMO, it's better to postpone the function call until when it is
actually necessary.
best regards,
Ranier Vilela
Attachments:
avoid-call-expensive-function-varlena.patchapplication/octet-stream; name=avoid-call-expensive-function-varlena.patchDownload
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 543afb66e5..4809e8a5f3 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -1557,8 +1557,6 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
{
pg_locale_t mylocale;
- mylocale = pg_newlocale_from_collation(collid);
-
/*
* memcmp() can't tell us which of two unequal strings sorts first,
* but it's a cheap way to tell if they're equal. Testing shows that
@@ -1571,6 +1569,7 @@ varstr_cmp(const char *arg1, int len1, const char *arg2, int len2, Oid collid)
if (len1 == len2 && memcmp(arg1, arg2, len1) == 0)
return 0;
+ mylocale = pg_newlocale_from_collation(collid);
result = pg_strncoll(arg1, len1, arg2, len2, mylocale);
/* Break tie if necessary. */
On Mon, 4 Mar 2024 at 18:39, Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi,
The function var_strcmp is a critical function.
Inside the function, there is a shortcut condition,
which allows for a quick exit.Unfortunately, the current code calls a very expensive function beforehand, which if the test was true, all the call time is wasted.
So, IMO, it's better to postpone the function call until when it is actually necessary.
Thank you for your contribution.
I agree it would be better, but your current patch is incorrect,
because we need to check if the user has access to the locale (and
throw an error if not) before we return that the two strings are
equal.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
Em seg., 4 de mar. de 2024 às 14:54, Matthias van de Meent <
boekewurm+postgres@gmail.com> escreveu:
On Mon, 4 Mar 2024 at 18:39, Ranier Vilela <ranier.vf@gmail.com> wrote:
Hi,
The function var_strcmp is a critical function.
Inside the function, there is a shortcut condition,
which allows for a quick exit.Unfortunately, the current code calls a very expensive function
beforehand, which if the test was true, all the call time is wasted.
So, IMO, it's better to postpone the function call until when it is
actually necessary.
Thank you for your contribution.
I agree it would be better, but your current patch is incorrect,
because we need to check if the user has access to the locale (and
throw an error if not) before we return that the two strings are
equal.
I can't see any user validation at the function pg_newlocale_from_collation.
meson test pass all checks.
best regards,
Ranier Vilela
On Mon, Mar 04, 2024 at 03:08:03PM -0300, Ranier Vilela wrote:
I can't see any user validation at the function pg_newlocale_from_collation.
Matthias is right, look closer. I can see more than one check,
especially note the one related to the collation version mismatch that
should not be silently ignored.
meson test pass all checks.
Collations are harder to test because they depend on the environment
where the test happens, especially with ICU.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Mon, Mar 04, 2024 at 03:08:03PM -0300, Ranier Vilela wrote:
I can't see any user validation at the function pg_newlocale_from_collation.
Matthias is right, look closer. I can see more than one check,
especially note the one related to the collation version mismatch that
should not be silently ignored.
The fast path through that code doesn't include any checks, true,
but the point is that finding the entry proves that we made those
checks previously. I can't agree with making those semantics
squishy in order to save a few cycles in the exact-equality case.
regards, tom lane
Em seg., 4 de mar. de 2024 às 20:28, Tom Lane <tgl@sss.pgh.pa.us> escreveu:
Michael Paquier <michael@paquier.xyz> writes:
On Mon, Mar 04, 2024 at 03:08:03PM -0300, Ranier Vilela wrote:
I can't see any user validation at the function
pg_newlocale_from_collation.
Matthias is right, look closer. I can see more than one check,
especially note the one related to the collation version mismatch that
should not be silently ignored.The fast path through that code doesn't include any checks, true,
but the point is that finding the entry proves that we made those
checks previously. I can't agree with making those semantics
squishy in order to save a few cycles in the exact-equality case.
Robustness is a fair point.
best regards,
Ranier Vilela