Re: Remove redundant strlen call in ReplicationSlotValidateName
От | Ranier Vilela |
---|---|
Тема | Re: Remove redundant strlen call in ReplicationSlotValidateName |
Дата | |
Msg-id | CAEudQArYTKuhymb=5EMF6zy-6d9rHamMxzuq+Ne6Hd3gqsTO_g@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Remove redundant strlen call in ReplicationSlotValidateName (Alvaro Herrera <alvherre@alvh.no-ip.org>) |
Ответы |
Re: Remove redundant strlen call in ReplicationSlotValidateName
|
Список | pgsql-hackers |
Em sex., 16 de jul. de 2021 às 12:37, Alvaro Herrera <alvherre@alvh.no-ip.org> escreveu:
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
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.
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.
But without any reason to backpatch, it can only be considered as refactoring.
regards,
Ranier Vilela
В списке pgsql-hackers по дате отправления: