Re: Remove redundant strlen call in ReplicationSlotValidateName

Em sex., 16 de jul. de 2021 às 12:37, Alvaro Herrera <
alvherre(at)alvh(dot)no-ip(dot)org> escreveu:

> On 2021-Jul-16, David Rowley wrote:
>
> > On Fri, 16 Jul 2021 at 20:35, Japin Li <japinli(at)hotmail(dot)com> wrote:
> > > > When I fix a bug about ALTER SUBSCRIPTION ... SET (slot_name) [1],
> Ranier Vilela
> > > > finds that ReplicationSlotValidateName() has redundant strlen()
> call, Since it's
> > > > not related to that problem, so I start a new thread to discuss it.
> >
> > I think this is a waste of time. The first strlen() call is just
> > checking for an empty string. I imagine all compilers would just
> > optimise that to checking if the first char is '\0';
>
> I could find the following idioms
>
> 95 times: var[0] == '\0'
> 146 times: *var == '\0'
> 35 times: strlen(var) == 0
>
> Resp.
> git grep "[a-zA-Z_]*\[0\] == '\\\\0"
> git grep "\*[a-zA-Z_]* == '\\\\0"
> git grep "strlen([^)]*) == 0"
>
> See https://postgr.es/m/13847.1587332283@sss.pgh.pa.us about replacing
> strlen with a check on first byte being zero. So still not Ranier's
> patch, but rather the attached. I doubt this change is worth committing
> on its own, though, since performance-wise it doesn't matter at all; if
> somebody were to make it so that all "strlen(foo) == 0" occurrences were
> changed to use the test on byte 0, that could be said to be establishing
> a consistent style, which might be more pallatable.
>
Yeah, can be considered a refactoring.

IMHO not in C is free.
I do not think that this will improve 1% generally, but some function can
gain.

Another example I can mention is this little Lua code, in which I made
comparisons between the generated asms, made some time ago.

p[0] = luaS_newlstr(L, str, strlen(str));

with strlen (msvc 64 bits):
inc r8
cmp BYTE PTR [r11+r8], 0
jne SHORT $LL19(at)luaS_new
mov rdx, r11
mov rcx, rdi
call luaS_newlstr

without strlen (msvc 64 bits):
mov r8, rsi
mov rdx, r11
mov QWORD PTR [rbx+8], rax
mov rcx, rdi
call luaS_newlstr

Of course, that doesn't mean anything about Postgres, but that I'm talking
about using strlen.
Clearly I can see that usage is not always free.

If have some interest in actually changing that in Postgres, I can prepare
a patch,
where static analyzers claim it's performance loss.
But without any reason to backpatch, it can only be considered as
refactoring.

regards,
Ranier Vilela