On 2021-Jul-16, David Rowley wrote:
> On Fri, 16 Jul 2021 at 20:35, Japin Li <japinli@hotmail.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@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
