Re: [PATCH] Small optimization across postgres (remove strlen duplicate usage)

On 4/19/20 10:29 PM, Ranier Vilela wrote:
> Em dom., 19 de abr. de 2020 às 16:33, Tomas Vondra
> <tomas(dot)vondra(at)2ndquadrant(dot)com <mailto:tomas(dot)vondra(at)2ndquadrant(dot)com>>
> escreveu:
>
> On Sun, Apr 19, 2020 at 11:24:38AM -0300, Ranier Vilela wrote:
> >Hi,
> >strlen it is one of the low fruits that can be harvested.
> >What is your opinion?
> >
>
> That assumes this actually affects/improves performance, without any
> measurements proving that. Considering large number of the places you
> modified are related to DDL (CreateComment, ChooseIndexColumnNames, ...)
> or stuff that runs only once or infrequently (like the changes in
> PostmasterMain or libpqrcv_get_senderinfo). Likewise, it seems entirely
> pointless to worry about strlen() overhead e.g. in fsync_parent_path
> which is probably dominated by I/O.
>
> With code as interconnected as postgres, it is difficult to say that a
> function, which calls strlen, repeatedly, would not have any gain.
> Regarding the functions, I was just being consistent, trying to remove
> all occurrences, even where, there is very little gain.

At least gcc 9.3 optimizes "strlen(s) == 0" to "s[0] == '\0'", even at
low optimization levels. I tried it out with https://godbolt.org/.

Maybe some of the others cases are performance improvements, I have not
checked your patch in details, but strlen() == 0 is easily handled by
the compiler.

C code:

int f1(char *str) {
return strlen(str) == 0;
}

int f2(char *str) {
return str[0] == '\0';
}

Assembly generated with default flags:

f1:
pushq %rbp
movq %rsp, %rbp
movq %rdi, -8(%rbp)
movq -8(%rbp), %rax
movzbl (%rax), %eax
testb %al, %al
sete %al
movzbl %al, %eax
popq %rbp
ret
f2:
pushq %rbp
movq %rsp, %rbp
movq %rdi, -8(%rbp)
movq -8(%rbp), %rax
movzbl (%rax), %eax
testb %al, %al
sete %al
movzbl %al, %eax
popq %rbp
ret

Assembly generated with -O2.

f1:
xorl %eax, %eax
cmpb $0, (%rdi)
sete %al
ret
f2:
xorl %eax, %eax
cmpb $0, (%rdi)
sete %al
ret

Andreas