Home > mailing lists

Re: Remove redundant strlen call in ReplicationSlotValidateName - Mailing list pgsql-hackers

From	Ranier Vilela
Subject	Re: Remove redundant strlen call in ReplicationSlotValidateName
Date	July 18, 2021 16:08:49
Msg-id	CAEudQAqSnBq02hLdoYYOSk6aW=94eThdYbAyFETSJnT0nBw6vw@mail.gmail.com Whole thread Raw
In response to	Re: Remove redundant strlen call in ReplicationSlotValidateName (Ranier Vilela <ranier.vf@gmail.com>)
Responses	Re: Remove redundant strlen call in ReplicationSlotValidateName (Peter Smith <smithpb2250@gmail.com>)
List	pgsql-hackers

Tree view

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 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.

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%.

	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

strlen.patch

pgsql-hackers by date:

From: Dean Rasheed
Date: 18 July 2021, 13:20:03
Subject: Re: CREATE COLLATION - check for duplicate options and error out if found one

From: Yugo NAGATA
Date: 18 July 2021, 17:27:16
Subject: Re: corruption of WAL page header is never reported

Re: Remove redundant strlen call in ReplicationSlotValidateName - Mailing list pgsql-hackers

Attachment

Previous

Next