Thread: Re: Remove redundant strlen call in ReplicationSlotValidateName
On Fri, 16 Jul 2021 at 16:26, Japin Li <japinli@hotmail.com> wrote: > Hi, hackers > > 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. > > [1] - https://www.postgresql.org/message-id/MEYP282MB1669CBD98E721C77CA696499B61A9%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
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'; https://godbolt.org/z/q58EGYMfM David
On Fri, 16 Jul 2021 at 18:05, David Rowley <dgrowleyml@gmail.com> 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'; > > https://godbolt.org/z/q58EGYMfM > Thanks for your review, this tool is amazing. The two writes on some compiler might be difference. As Amit Kapila said, it might be helpless for reducing overhead. https://godbolt.org/z/j5on6Khxb -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
Em sex., 16 de jul. de 2021 às 07:05, David Rowley <dgrowleyml@gmail.com> escreveu:
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 think with very simple functions, the compiler can do the job.
But, it's not always like that, I think.
With gcc 11, I can see clear and different ASM.
strlen1(char const*):
sub rsp, 8
cmp BYTE PTR [rdi], 0
je .L8
call strlen
mov r8, rax
xor eax, eax
cmp r8, 64
ja .L9
strlen2(char const*):
sub rsp, 8
call strlen
test eax, eax
je .L15
xor r8d, r8d
cmp eax, 64
jg .L16
For me strlen2's ASM is much more compact.
And as some functions with strlen are always a hotpath, it's worth it.
And as some functions with strlen are always a hotpath, it's worth it.
regards,
Ranier Vilela
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. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Just treat us the way you want to be treated + some extra allowance for ignorance." (Michael Brusser)
Attachment
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
Em sex., 16 de jul. de 2021 às 13:18, Ranier Vilela <ranier.vf@gmail.com> escreveu:
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 cangain.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_newlstrOf 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.
I did the patch, but to my surprise, the results weren't so good.
Despite that claiming a tiny improvement in performance, I didn't expect any slowdown.
I put a counter in pg_regress.c, summing the results of each test and did it three times for HEAD and for the patch.
Some tests were better, but others were bad.
Despite that claiming a tiny improvement in performance, I didn't expect any slowdown.
I put a counter in pg_regress.c, summing the results of each test and did it three times for HEAD and for the patch.
Some tests were better, but others were bad.
Tests comments per example, show 180%, combocid 174%, dbize 165%, xmlmap 136%, lock 134%.
HEAD 1 | HEAD 2 | HEAD 3 | HEAD avg | patch strlen1 | patch strlen2 | patch strlen3 | patch avg | |||
tablespace | 326 | 328 | 339 | 331 | 321 | 320 | 335 | 325,333333333333 | 101,74% | |
boolean | 63 | 48 | 62 | 57,6666666666667 | 63 | 39 | 62 | 54,6666666666667 | 105,49% | |
char | 47 | 25 | 24 | 32 | 33 | 21 | 25 | 26,3333333333333 | 121,52% | |
name | 30 | 33 | 26 | 29,6666666666667 | 34 | 26 | 62 | 40,6666666666667 | 72,95% | |
varchar | 43 | 47 | 26 | 38,6666666666667 | 65 | 31 | 55 | 50,3333333333333 | 76,82% | |
text | 51 | 43 | 53 | 49 | 37 | 57 | 54 | 49,3333333333333 | 99,32% | |
int2 | 29 | 66 | 46 | 47 | 57 | 58 | 38 | 51 | 92,16% | |
int4 | 55 | 35 | 46 | 45,3333333333333 | 51 | 54 | 70 | 58,3333333333333 | 77,71% | |
int8 | 103 | 85 | 111 | 99,6666666666667 | 79 | 96 | 91 | 88,6666666666667 | 112,41% | |
oid | 70 | 85 | 71 | 75,3333333333333 | 45 | 64 | 80 | 63 | 119,58% | |
float4 | 68 | 74 | 86 | 76 | 84 | 68 | 49 | 67 | 113,43% | |
float8 | 98 | 87 | 109 | 98 | 95 | 98 | 88 | 93,6666666666667 | 104,63% | |
bit | 99 | 84 | 91 | 91,3333333333333 | 108 | 96 | 92 | 98,6666666666667 | 92,57% | |
numeric | 414 | 392 | 393 | 399,666666666667 | 379 | 399 | 411 | 396,333333333333 | 100,84% | |
txid | 82 | 84 | 63 | 76,3333333333333 | 71 | 79 | 56 | 68,6666666666667 | 111,17% | |
uuid | 48 | 96 | 76 | 73,3333333333333 | 79 | 76 | 67 | 74 | 99,10% | |
enum | 101 | 125 | 102 | 109,333333333333 | 113 | 117 | 113 | 114,333333333333 | 95,63% | |
money | 81 | 77 | 79 | 79 | 45 | 66 | 77 | 62,6666666666667 | 126,06% | |
rangetypes | 404 | 411 | 415 | 410 | 426 | 404 | 421 | 417 | 98,32% | |
pg_lsn | 60 | 63 | 69 | 64 | 69 | 57 | 51 | 59 | 108,47% | |
regproc | 69 | 52 | 60 | 60,3333333333333 | 79 | 57 | 76 | 70,6666666666667 | 85,38% | |
strings | 145 | 152 | 139 | 145,333333333333 | 170 | 138 | 136 | 148 | 98,20% | |
numerology | 46 | 35 | 31 | 37,3333333333333 | 34 | 35 | 33 | 34 | 109,80% | |
point | 45 | 37 | 45 | 42,3333333333333 | 52 | 38 | 76 | 55,3333333333333 | 76,51% | |
lseg | 25 | 15 | 19 | 19,6666666666667 | 33 | 37 | 19 | 29,6666666666667 | 66,29% | |
line | 31 | 30 | 24 | 28,3333333333333 | 20 | 24 | 32 | 25,3333333333333 | 111,84% | |
box | 259 | 254 | 266 | 259,666666666667 | 255 | 307 | 305 | 289 | 89,85% | |
path | 25 | 20 | 52 | 32,3333333333333 | 35 | 57 | 25 | 39 | 82,91% | |
polygon | 274 | 269 | 271 | 271,333333333333 | 247 | 258 | 315 | 273,333333333333 | 99,27% | |
circle | 35 | 52 | 55 | 47,3333333333333 | 43 | 33 | 51 | 42,3333333333333 | 111,81% | |
date | 108 | 82 | 90 | 93,3333333333333 | 106 | 101 | 104 | 103,666666666667 | 90,03% | |
time | 67 | 62 | 39 | 56 | 69 | 34 | 36 | 46,3333333333333 | 120,86% | |
timetz | 38 | 62 | 41 | 47 | 36 | 75 | 33 | 48 | 97,92% | |
timestamp | 416 | 412 | 425 | 417,666666666667 | 451 | 431 | 357 | 413 | 101,13% | |
timestamptz | 479 | 468 | 474 | 473,666666666667 | 503 | 461 | 411 | 458,333333333333 | 103,35% | |
interval | 81 | 104 | 93 | 92,6666666666667 | 69 | 96 | 79 | 81,3333333333333 | 113,93% | |
inet | 72 | 74 | 108 | 84,6666666666667 | 83 | 80 | 85 | 82,6666666666667 | 102,42% | |
macaddr | 37 | 61 | 43 | 47 | 59 | 74 | 72 | 68,3333333333333 | 68,78% | |
macaddr8 | 54 | 61 | 64 | 59,6666666666667 | 39 | 78 | 78 | 65 | 91,79% | |
multirangetypes | 278 | 271 | 290 | 279,666666666667 | 290 | 333 | 265 | 296 | 94,48% | |
create_function_0 | 66 | 37 | 59 | 54 | 48 | 31 | 39 | 39,3333333333333 | 137,29% | |
geometry | 153 | 136 | 133 | 140,666666666667 | 131 | 137 | 148 | 138,666666666667 | 101,44% | |
horology | 102 | 132 | 98 | 110,666666666667 | 108 | 101 | 97 | 102 | 108,50% | |
tstypes | 81 | 69 | 60 | 70 | 86 | 56 | 54 | 65,3333333333333 | 107,14% | |
regex | 498 | 515 | 517 | 510 | 499 | 486 | 510 | 498,333333333333 | 102,34% | |
type_sanity | 153 | 132 | 134 | 139,666666666667 | 161 | 130 | 144 | 145 | 96,32% | |
opr_sanity | 389 | 377 | 381 | 382,333333333333 | 387 | 387 | 363 | 379 | 100,88% | |
misc_sanity | 33 | 37 | 37 | 35,6666666666667 | 47 | 34 | 38 | 39,6666666666667 | 89,92% | |
comments | 19 | 56 | 35 | 36,6666666666667 | 30 | 17 | 14 | 20,3333333333333 | 180,33% | |
expressions | 62 | 44 | 67 | 57,6666666666667 | 52 | 40 | 69 | 53,6666666666667 | 107,45% | |
unicode | 49 | 52 | 21 | 40,6666666666667 | 53 | 46 | 17 | 38,6666666666667 | 105,17% | |
xid | 46 | 45 | 40 | 43,6666666666667 | 83 | 81 | 38 | 67,3333333333333 | 64,85% | |
mvcc | 98 | 107 | 111 | 105,333333333333 | 95 | 116 | 94 | 101,666666666667 | 103,61% | |
create_function_1 | 9 | 10 | 10 | 9,66666666666667 | 10 | 10 | 9 | 9,66666666666667 | 100,00% | |
create_type | 29 | 28 | 30 | 29 | 28 | 29 | 29 | 28,6666666666667 | 101,16% | |
create_table | 365 | 364 | 364 | 364,333333333333 | 365 | 363 | 360 | 362,666666666667 | 100,46% | |
create_function_2 | 13 | 12 | 12 | 12,3333333333333 | 13 | 12 | 12 | 12,3333333333333 | 100,00% | |
copy | 461 | 450 | 458 | 456,333333333333 | 463 | 454 | 450 | 455,666666666667 | 100,15% | |
copyselect | 29 | 29 | 32 | 30 | 30 | 27 | 29 | 28,6666666666667 | 104,65% | |
copydml | 35 | 35 | 35 | 35 | 37 | 35 | 33 | 35 | 100,00% | |
insert | 303 | 309 | 338 | 316,666666666667 | 305 | 287 | 305 | 299 | 105,91% | |
insert_conflict | 142 | 141 | 176 | 153 | 150 | 137 | 154 | 147 | 104,08% | |
create_misc | 94 | 93 | 96 | 94,3333333333333 | 100 | 94 | 114 | 102,666666666667 | 91,88% | |
create_operator | 24 | 27 | 22 | 24,3333333333333 | 28 | 26 | 24 | 26 | 93,59% | |
create_procedure | 62 | 64 | 65 | 63,6666666666667 | 73 | 63 | 82 | 72,6666666666667 | 87,61% | |
create_index | 880 | 933 | 866 | 893 | 896 | 864 | 904 | 888 | 100,56% | |
create_index_spgist | 582 | 512 | 574 | 556 | 598 | 588 | 515 | 567 | 98,06% | |
create_view | 341 | 250 | 329 | 306,666666666667 | 286 | 334 | 292 | 304 | 100,88% | |
index_including | 183 | 207 | 204 | 198 | 215 | 143 | 163 | 173,666666666667 | 114,01% | |
index_including_gist | 307 | 362 | 364 | 344,333333333333 | 413 | 260 | 278 | 317 | 108,62% | |
create_aggregate | 62 | 69 | 90 | 73,6666666666667 | 60 | 100 | 107 | 89 | 82,77% | |
create_function_3 | 146 | 162 | 173 | 160,333333333333 | 151 | 144 | 181 | 158,666666666667 | 101,05% | |
create_cast | 26 | 29 | 22 | 25,6666666666667 | 24 | 18 | 71 | 37,6666666666667 | 68,14% | |
constraints | 210 | 218 | 216 | 214,666666666667 | 239 | 227 | 195 | 220,333333333333 | 97,43% | |
triggers | 856 | 920 | 882 | 886 | 889 | 871 | 861 | 873,666666666667 | 101,41% | |
select | 103 | 133 | 101 | 112,333333333333 | 92 | 133 | 140 | 121,666666666667 | 92,33% | |
inherit | 615 | 627 | 671 | 637,666666666667 | 642 | 646 | 616 | 634,666666666667 | 100,47% | |
typed_table | 146 | 129 | 131 | 135,333333333333 | 115 | 121 | 116 | 117,333333333333 | 115,34% | |
vacuum | 494 | 417 | 455 | 455,333333333333 | 442 | 451 | 402 | 431,666666666667 | 105,48% | |
drop_if_exists | 115 | 84 | 83 | 94 | 138 | 63 | 57 | 86 | 109,30% | |
updatable_views | 645 | 630 | 678 | 651 | 635 | 644 | 647 | 642 | 101,40% | |
roleattributes | 119 | 95 | 80 | 98 | 105 | 134 | 90 | 109,666666666667 | 89,36% | |
create_am | 156 | 114 | 126 | 132 | 155 | 167 | 191 | 171 | 77,19% | |
hash_func | 83 | 52 | 61 | 65,3333333333333 | 50 | 50 | 91 | 63,6666666666667 | 102,62% | |
errors | 54 | 80 | 63 | 65,6666666666667 | 91 | 33 | 41 | 55 | 119,39% | |
infinite_recurse | 312 | 321 | 361 | 331,333333333333 | 329 | 341 | 401 | 357 | 92,81% | |
sanity_check | 136 | 134 | 134 | 134,666666666667 | 135 | 134 | 134 | 134,333333333333 | 100,25% | |
select_into | 91 | 139 | 66 | 98,6666666666667 | 81 | 118 | 78 | 92,3333333333333 | 106,86% | |
select_distinct | 228 | 202 | 181 | 203,666666666667 | 242 | 238 | 214 | 231,333333333333 | 88,04% | |
select_distinct_on | 27 | 38 | 60 | 41,6666666666667 | 44 | 36 | 24 | 34,6666666666667 | 120,19% | |
select_implicit | 77 | 48 | 47 | 57,3333333333333 | 44 | 51 | 94 | 63 | 91,01% | |
select_having | 38 | 48 | 75 | 53,6666666666667 | 50 | 64 | 39 | 51 | 105,23% | |
subselect | 274 | 297 | 305 | 292 | 316 | 351 | 310 | 325,666666666667 | 89,66% | |
union | 308 | 285 | 289 | 294 | 351 | 289 | 281 | 307 | 95,77% | |
case | 53 | 137 | 47 | 79 | 81 | 56 | 149 | 95,3333333333333 | 82,87% | |
join | 685 | 718 | 736 | 713 | 690 | 678 | 793 | 720,333333333333 | 98,98% | |
aggregates | 706 | 675 | 697 | 692,666666666667 | 802 | 680 | 728 | 736,666666666667 | 94,03% | |
transactions | 285 | 182 | 273 | 246,666666666667 | 159 | 247 | 251 | 219 | 112,63% | |
random | 44 | 67 | 46 | 52,3333333333333 | 43 | 82 | 50 | 58,3333333333333 | 89,71% | |
portals | 264 | 265 | 189 | 239,333333333333 | 263 | 247 | 225 | 245 | 97,69% | |
arrays | 379 | 355 | 358 | 364 | 352 | 398 | 339 | 363 | 100,28% | |
btree_index | 1550 | 1532 | 1476 | 1519,33333333333 | 1708 | 1565 | 1557 | 1610 | 94,37% | |
hash_index | 436 | 480 | 477 | 464,333333333333 | 464 | 447 | 440 | 450,333333333333 | 103,11% | |
update | 468 | 464 | 464 | 465,333333333333 | 522 | 464 | 472 | 486 | 95,75% | |
delete | 112 | 43 | 48 | 67,6666666666667 | 101 | 89 | 41 | 77 | 87,88% | |
namespace | 96 | 50 | 56 | 67,3333333333333 | 136 | 52 | 49 | 79 | 85,23% | |
prepared_xacts | 185 | 109 | 160 | 151,333333333333 | 161 | 127 | 175 | 154,333333333333 | 98,06% | |
brin | 1689 | 1719 | 1740 | 1716 | 1777 | 1671 | 1704 | 1717,33333333333 | 99,92% | |
gin | 1166 | 1213 | 1139 | 1172,66666666667 | 1215 | 1243 | 1095 | 1184,33333333333 | 99,01% | |
gist | 1052 | 1045 | 1073 | 1056,66666666667 | 1029 | 1072 | 1064 | 1055 | 100,16% | |
spgist | 1013 | 1029 | 1094 | 1045,33333333333 | 1009 | 946 | 994 | 983 | 106,34% | |
privileges | 876 | 863 | 904 | 881 | 924 | 942 | 1008 | 958 | 91,96% | |
init_privs | 27 | 20 | 25 | 24 | 44 | 18 | 17 | 26,3333333333333 | 91,14% | |
security_label | 43 | 49 | 63 | 51,6666666666667 | 89 | 46 | 170 | 101,666666666667 | 50,82% | |
collate | 315 | 393 | 346 | 351,333333333333 | 319 | 222 | 341 | 294 | 119,50% | |
matview | 671 | 699 | 663 | 677,666666666667 | 698 | 676 | 723 | 699 | 96,95% | |
lock | 220 | 171 | 177 | 189,333333333333 | 112 | 132 | 179 | 141 | 134,28% | |
replica_identity | 294 | 248 | 235 | 259 | 314 | 229 | 428 | 323,666666666667 | 80,02% | |
rowsecurity | 869 | 888 | 832 | 863 | 853 | 919 | 933 | 901,666666666667 | 95,71% | |
object_address | 244 | 210 | 202 | 218,666666666667 | 257 | 224 | 301 | 260,666666666667 | 83,89% | |
tablesample | 154 | 164 | 194 | 170,666666666667 | 217 | 149 | 169 | 178,333333333333 | 95,70% | |
groupingsets | 644 | 606 | 640 | 630 | 564 | 637 | 665 | 622 | 101,29% | |
drop_operator | 98 | 125 | 55 | 92,6666666666667 | 54 | 72 | 76 | 67,3333333333333 | 137,62% | |
password | 585 | 572 | 535 | 564 | 527 | 538 | 465 | 510 | 110,59% | |
identity | 529 | 435 | 499 | 487,666666666667 | 504 | 571 | 453 | 509,333333333333 | 95,75% | |
generated | 777 | 672 | 759 | 736 | 830 | 744 | 734 | 769,333333333333 | 95,67% | |
join_hash | 1672 | 1706 | 1726 | 1701,33333333333 | 1761 | 1667 | 1691 | 1706,33333333333 | 99,71% | |
brin_bloom | 97 | 97 | 131 | 108,333333333333 | 99 | 109 | 97 | 101,666666666667 | 106,56% | |
brin_multi | 236 | 234 | 269 | 246,333333333333 | 245 | 236 | 226 | 235,666666666667 | 104,53% | |
create_table_like | 281 | 277 | 257 | 271,666666666667 | 283 | 277 | 280 | 280 | 97,02% | |
alter_generic | 124 | 116 | 126 | 122 | 119 | 131 | 137 | 129 | 94,57% | |
alter_operator | 33 | 28 | 31 | 30,6666666666667 | 61 | 36 | 50 | 49 | 62,59% | |
misc | 161 | 172 | 157 | 163,333333333333 | 176 | 160 | 167 | 167,666666666667 | 97,42% | |
async | 19 | 13 | 40 | 24 | 33 | 60 | 22 | 38,3333333333333 | 62,61% | |
dbsize | 73 | 31 | 22 | 42 | 24 | 19 | 33 | 25,3333333333333 | 165,79% | |
misc_functions | 84 | 79 | 84 | 82,3333333333333 | 94 | 89 | 92 | 91,6666666666667 | 89,82% | |
sysviews | 97 | 108 | 86 | 97 | 91 | 88 | 92 | 90,3333333333333 | 107,38% | |
tsrf | 69 | 72 | 80 | 73,6666666666667 | 60 | 96 | 84 | 80 | 92,08% | |
tid | 84 | 37 | 38 | 53 | 47 | 73 | 57 | 59 | 89,83% | |
tidscan | 75 | 61 | 81 | 72,3333333333333 | 98 | 77 | 96 | 90,3333333333333 | 80,07% | |
tidrangescan | 83 | 49 | 76 | 69,3333333333333 | 47 | 69 | 63 | 59,6666666666667 | 116,20% | |
collate.icu.utf8 | 30 | 37 | 29 | 32 | 16 | 60 | 27 | 34,3333333333333 | 93,20% | |
incremental_sort | 174 | 176 | 163 | 171 | 160 | 164 | 179 | 167,666666666667 | 101,99% | |
rules | 303 | 383 | 315 | 333,666666666667 | 385 | 376 | 341 | 367,333333333333 | 90,83% | |
psql | 253 | 315 | 253 | 273,666666666667 | 329 | 243 | 297 | 289,666666666667 | 94,48% | |
psql_crosstab | 35 | 31 | 30 | 32 | 49 | 33 | 33 | 38,3333333333333 | 83,48% | |
amutils | 18 | 20 | 18 | 18,6666666666667 | 34 | 20 | 18 | 24 | 77,78% | |
stats_ext | 1530 | 1568 | 1572 | 1556,66666666667 | 1576 | 1605 | 1571 | 1584 | 98,27% | |
collate.linux.utf8 | 10 | 11 | 11 | 10,6666666666667 | 17 | 10 | 11 | 12,6666666666667 | 84,21% | |
select_parallel | 786 | 786 | 797 | 789,666666666667 | 797 | 807 | 826 | 810 | 97,49% | |
write_parallel | 78 | 80 | 80 | 79,3333333333333 | 79 | 79 | 84 | 80,6666666666667 | 98,35% | |
publication | 76 | 83 | 81 | 80 | 74 | 75 | 74 | 74,3333333333333 | 107,62% | |
subscription | 41 | 48 | 48 | 45,6666666666667 | 40 | 43 | 43 | 42 | 108,73% | |
select_views | 201 | 222 | 199 | 207,333333333333 | 188 | 205 | 204 | 199 | 104,19% | |
portals_p2 | 57 | 38 | 41 | 45,3333333333333 | 35 | 44 | 34 | 37,6666666666667 | 120,35% | |
foreign_key | 969 | 955 | 965 | 963 | 925 | 971 | 947 | 947,666666666667 | 101,62% | |
cluster | 407 | 400 | 408 | 405 | 417 | 439 | 398 | 418 | 96,89% | |
dependency | 107 | 212 | 112 | 143,666666666667 | 178 | 147 | 164 | 163 | 88,14% | |
guc | 101 | 184 | 168 | 151 | 177 | 181 | 161 | 173 | 87,28% | |
bitmapops | 448 | 443 | 429 | 440 | 444 | 460 | 446 | 450 | 97,78% | |
combocid | 156 | 144 | 150 | 150 | 46 | 162 | 50 | 86 | 174,42% | |
tsearch | 396 | 443 | 424 | 421 | 404 | 434 | 415 | 417,666666666667 | 100,80% | |
tsdicts | 137 | 170 | 166 | 157,666666666667 | 98 | 157 | 130 | 128,333333333333 | 122,86% | |
foreign_data | 633 | 637 | 636 | 635,333333333333 | 647 | 648 | 629 | 641,333333333333 | 99,06% | |
window | 374 | 428 | 391 | 397,666666666667 | 353 | 374 | 355 | 360,666666666667 | 110,26% | |
xmlmap | 108 | 51 | 113 | 90,6666666666667 | 85 | 45 | 69 | 66,3333333333333 | 136,68% | |
functional_deps | 177 | 169 | 125 | 157 | 145 | 162 | 144 | 150,333333333333 | 104,43% | |
advisory_lock | 71 | 68 | 79 | 72,6666666666667 | 72 | 57 | 86 | 71,6666666666667 | 101,40% | |
indirect_toast | 541 | 522 | 554 | 539 | 567 | 627 | 596 | 596,666666666667 | 90,34% | |
equivclass | 123 | 197 | 118 | 146 | 184 | 178 | 165 | 175,666666666667 | 83,11% | |
json | 106 | 105 | 111 | 107,333333333333 | 116 | 108 | 110 | 111,333333333333 | 96,41% | |
jsonb | 246 | 253 | 256 | 251,666666666667 | 262 | 253 | 256 | 257 | 97,92% | |
json_encoding | 17 | 16 | 16 | 16,3333333333333 | 17 | 15 | 19 | 17 | 96,08% | |
jsonpath | 31 | 32 | 31 | 31,3333333333333 | 32 | 32 | 32 | 32 | 97,92% | |
jsonpath_encoding | 15 | 14 | 14 | 14,3333333333333 | 15 | 15 | 14 | 14,6666666666667 | 97,73% | |
jsonb_jsonpath | 81 | 81 | 84 | 82 | 83 | 82 | 84 | 83 | 98,80% | |
plancache | 107 | 103 | 145 | 118,333333333333 | 131 | 101 | 120 | 117,333333333333 | 100,85% | |
limit | 186 | 130 | 127 | 147,666666666667 | 138 | 167 | 138 | 147,666666666667 | 100,00% | |
plpgsql | 725 | 713 | 739 | 725,666666666667 | 726 | 699 | 722 | 715,666666666667 | 101,40% | |
copy2 | 324 | 227 | 187 | 246 | 226 | 137 | 226 | 196,333333333333 | 125,30% | |
temp | 215 | 236 | 251 | 234 | 208 | 178 | 190 | 192 | 121,88% | |
domain | 380 | 368 | 391 | 379,666666666667 | 371 | 351 | 351 | 357,666666666667 | 106,15% | |
rangefuncs | 379 | 336 | 371 | 362 | 354 | 373 | 359 | 362 | 100,00% | |
prepare | 164 | 175 | 131 | 156,666666666667 | 87 | 179 | 149 | 138,333333333333 | 113,25% | |
conversion | 190 | 113 | 198 | 167 | 186 | 259 | 221 | 222 | 75,23% | |
truncate | 349 | 362 | 365 | 358,666666666667 | 285 | 342 | 305 | 310,666666666667 | 115,45% | |
alter_table | 1419 | 1415 | 1363 | 1399 | 1406 | 1409 | 1402 | 1405,66666666667 | 99,53% | |
sequence | 260 | 227 | 312 | 266,333333333333 | 236 | 264 | 212 | 237,333333333333 | 112,22% | |
polymorphism | 298 | 341 | 357 | 332 | 342 | 341 | 323 | 335,333333333333 | 99,01% | |
rowtypes | 291 | 266 | 284 | 280,333333333333 | 281 | 254 | 309 | 281,333333333333 | 99,64% | |
returning | 146 | 92 | 64 | 100,666666666667 | 106 | 243 | 108 | 152,333333333333 | 66,08% | |
largeobject | 365 | 447 | 415 | 409 | 426 | 494 | 426 | 448,666666666667 | 91,16% | |
with | 385 | 329 | 360 | 358 | 345 | 286 | 349 | 326,666666666667 | 109,59% | |
xml | 160 | 222 | 217 | 199,666666666667 | 289 | 253 | 228 | 256,666666666667 | 77,79% | |
partition_join | 865 | 861 | 861 | 862,333333333333 | 816 | 924 | 872 | 870,666666666667 | 99,04% | |
partition_prune | 783 | 719 | 791 | 764,333333333333 | 758 | 758 | 781 | 765,666666666667 | 99,83% | |
reloptions | 96 | 84 | 78 | 86 | 74 | 74 | 87 | 78,3333333333333 | 109,79% | |
hash_part | 51 | 43 | 39 | 44,3333333333333 | 46 | 57 | 58 | 53,6666666666667 | 82,61% | |
indexing | 702 | 707 | 685 | 698 | 737 | 714 | 739 | 730 | 95,62% | |
partition_aggregate | 949 | 939 | 884 | 924 | 960 | 957 | 915 | 944 | 97,88% | |
partition_info | 96 | 111 | 77 | 94,6666666666667 | 111 | 83 | 104 | 99,3333333333333 | 95,30% | |
tuplesort | 1359 | 1339 | 1387 | 1361,66666666667 | 1404 | 1381 | 1359 | 1381,33333333333 | 98,58% | |
explain | 97 | 101 | 82 | 93,3333333333333 | 102 | 102 | 74 | 92,6666666666667 | 100,72% | |
compression | 164 | 157 | 162 | 161 | 154 | 149 | 156 | 153 | 105,23% | |
memoize | 90 | 95 | 87 | 90,6666666666667 | 80 | 75 | 84 | 79,6666666666667 | 113,81% | |
event_trigger | 126 | 126 | 135 | 129 | 127 | 111 | 112 | 116,666666666667 | 110,57% | |
oidjoins | 257 | 240 | 252 | 249,666666666667 | 245 | 243 | 245 | 244,333333333333 | 102,18% | |
fast_default | 146 | 145 | 145 | 145,333333333333 | 148 | 145 | 162 | 151,666666666667 | 95,82% | |
stats | 615 | 611 | 611 | 612,333333333333 | 621 | 613 | 611 | 615 | 99,57% |
So I'm posting the patch here, merely as an illustration of my findings.
Perhaps someone with a better understanding of the process of translating C to asm can have an explanation.
Is it worth it to change only where there has been improvement?
regards,
Ranier Vilela
Attachment
On Sun, Jul 18, 2021 at 11:09 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
...
I did the patch, but to my surprise, the results weren't so good.Despite that claiming a tiny improvement in performance, I didn't expect any slowdown.
I put a counter in pg_regress.c, summing the results of each test and did it three times for HEAD and for the patch.
Some tests were better, but others were bad.Tests comments per example, show 180%, combocid 174%, dbize 165%, xmlmap 136%, lock 134%.
... ...
So I'm posting the patch here, merely as an illustration of my findings.
Perhaps someone with a better understanding of the process of translating C to asm can have an explanation.Is it worth it to change only where there has been improvement?
My guess is that your hypothetical performance improvement has been completely swamped by the natural variations of each run.
For example,
drop_if_exists | 115 | 84 | 83 | 94 | 138 | 63 | 57 | 86 | 109,30% |
Those numbers are all over the place, so I doubt the results are really saying anything at all about what is better/worse, because I think you have zero chance to notice a couple of nanoseconds of improvement within the noise when each run is varying from 57 to 138 ms.
IMO the only conclusion you can draw from your results is that any performance gain is too small to be observable.
------
Kind Regards,
Peter Smith.
Fujitsu Australia
Em dom., 18 de jul. de 2021 às 21:23, Peter Smith <smithpb2250@gmail.com> escreveu:
On Sun, Jul 18, 2021 at 11:09 PM Ranier Vilela <ranier.vf@gmail.com> wrote:...I did the patch, but to my surprise, the results weren't so good.Despite that claiming a tiny improvement in performance, I didn't expect any slowdown.
I put a counter in pg_regress.c, summing the results of each test and did it three times for HEAD and for the patch.
Some tests were better, but others were bad.Tests comments per example, show 180%, combocid 174%, dbize 165%, xmlmap 136%, lock 134%.
... ...
So I'm posting the patch here, merely as an illustration of my findings.
Perhaps someone with a better understanding of the process of translating C to asm can have an explanation.Is it worth it to change only where there has been improvement?My guess is that your hypothetical performance improvement has been completely swamped by the natural variations of each run.For example,
drop_if_exists 115 84 83 94 138 63 57 86 109,30% Those numbers are all over the place, so I doubt the results are really saying anything at all about what is better/worse, because I think you have zero chance to notice a couple of nanoseconds of improvement within the noise when each run is varying from 57 to 138 ms.IMO the only conclusion you can draw from your results is that any performance gain is too small to be observable.
Thanks Peter for your explanations.
I can conclude then that the test results are not a reference for performance/regression.
So the patch serves as a refactoring, without any further indication.
regards,
Ranier Vilela