Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[chore](clang-tidy): add bugprone linters #29521

Merged
merged 4 commits into from
Feb 5, 2024
Merged

[chore](clang-tidy): add bugprone linters #29521

merged 4 commits into from
Feb 5, 2024

Conversation

py023
Copy link
Contributor

@py023 py023 commented Jan 4, 2024

Proposed changes

This PR introduces 4 bugprone linter rules to .clang-tidy, these linters found some bugs in #28965. This PR also add some comments to mute false positive reports.

Further comments

If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

clang-tidy review says "All clean, LGTM! 👍"

@py023
chore: auto format
986988e 
Signed-off-by: pengyu <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 4, 2024

clang-tidy review says "All clean, LGTM! 👍"

@gavinchou
Copy link
Collaborator

run buildall

gavinchou
gavinchou approved these changes Jan 5, 2024
View reviewed changes
Copy link
Collaborator

@gavinchou gavinchou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 5, 2024

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G', run with scripts in https://github.com/apache/doris/tree/master/tools/tpch-tools

Tpch sf100 test result on commit 986988e5273203b89b1c0260f40cc326c26f002d, data reload: false

------ Round 1 ----------------------------------
q1	18428	5246	5145	5145
q2	2027	159	144	144
q3	10612	1075	1178	1075
q4	10196	781	828	781
q5	7831	2956	2909	2909
q6	215	135	132	132
q7	917	526	542	526
q8	9271	2069	2049	2049
q9	6897	6484	6419	6419
q10	8253	3099	3039	3039
q11	442	229	217	217
q12	402	237	241	237
q13	18023	3602	3691	3602
q14	240	220	217	217
q15	569	521	516	516
q16	461	403	399	399
q17	976	525	501	501
q18	7523	6728	6677	6677
q19	1625	1412	1392	1392
q20	724	358	328	328
q21	2825	2412	2403	2403
q22	396	328	342	328
Total cold run time: 108853 ms
Total hot run time: 39036 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5190	5104	5137	5104
q2	348	229	248	229
q3	3325	3295	3256	3256
q4	2126	2028	2037	2028
q5	5821	5764	5802	5764
q6	217	126	128	126
q7	2330	1917	1945	1917
q8	3411	3449	3496	3449
q9	8884	8865	8809	8809
q10	3800	3878	3833	3833
q11	597	489	490	489
q12	816	650	643	643
q13	7698	3205	3171	3171
q14	298	264	266	264
q15	579	532	520	520
q16	557	487	506	487
q17	1950	1781	1755	1755
q18	8750	8391	8379	8379
q19	1644	1618	1587	1587
q20	2186	1978	1970	1970
q21	5693	5346	5320	5320
q22	568	516	530	516
Total cold run time: 66788 ms
Total hot run time: 59616 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.63% (8617/23522)
Line Coverage: 28.67% (70030/244288)
Region Coverage: 27.64% (36248/131121)
Branch Coverage: 24.34% (18515/76078)
Coverage Report: http://coverage.selectdb-in.cc/coverage/986988e5273203b89b1c0260f40cc326c26f002d_986988e5273203b89b1c0260f40cc326c26f002d/report/index.html

@doris-robot
Copy link

TPC-DS test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G', run with scripts in https://github.com/apache/doris/tree/master/tools/tpcds-tools

TPC-DS sf100 test result on commit 986988e5273203b89b1c0260f40cc326c26f002d, data reload: false

run tpcds-sf100 query with default conf and session variables
query1	932	349	337	337
query2	6415	1950	2006	1950
query3	6647	210	209	209
query4	27092	22744	22423	22423
query5	3892	567	500	500
query6	270	184	179	179
query7	4603	274	266	266
query8	232	202	207	202
query9	8230	2635	2570	2570
query10	395	265	257	257
query11	16351	15451	15580	15451
query12	134	77	77	77
query13	1670	315	314	314
query14	11452	7185	7186	7185
query15	217	192	195	192
query16	6407	278	266	266
query17	1856	497	482	482
query18	1949	266	258	258
query19	185	139	134	134
query20	80	74	75	74
query21	184	101	95	95
query22	5058	4802	4716	4716
query23	32547	31154	31349	31154
query24	12156	2826	2838	2826
query25	595	346	333	333
query26	1731	144	143	143
query27	2912	277	280	277
query28	7057	1879	1871	1871
query29	2067	389	382	382
query30	289	142	149	142
query31	995	773	759	759
query32	89	57	56	56
query33	743	244	270	244
query34	922	460	450	450
query35	880	767	753	753
query36	1229	1252	1188	1188
query37	185	63	70	63
query38	3355	3316	3257	3257
query39	1349	1297	1277	1277
query40	304	85	86	85
query41	38	35	33	33
query42	85	84	84	84
query43	530	502	508	502
query44	1091	701	712	701
query45	202	189	184	184
query46	1077	628	628	628
query47	1645	1545	1534	1534
query48	360	252	262	252
query49	1229	314	332	314
query50	774	358	328	328
query51	5387	5273	5227	5227
query52	90	88	86	86
query53	215	151	150	150
query54	1345	571	591	571
query55	98	85	77	77
query56	189	184	197	184
query57	1054	958	965	958
query58	221	199	201	199
query59	2874	2681	2670	2670
query60	233	217	225	217
query61	84	83	83	83
query62	657	466	483	466
query63	175	148	149	148
query64	5918	1738	1678	1678
query65	3337	3266	3272	3266
query66	1394	332	352	332
query67	15492	15109	15041	15041
query68	13179	526	507	507
query69	542	245	249	245
query70	2080	1429	1525	1429
query71	494	202	197	197
query72	5609	3504	3518	3504
query73	3371	318	316	316
query74	7131	6487	6406	6406
query75	5368	2309	2238	2238
query76	6309	1178	1165	1165
query77	658	247	256	247
query78	9181	8628	8514	8514
query79	1885	502	520	502
query80	573	355	355	355
query81	472	207	206	206
query82	205	91	92	91
query83	184	138	137	137
query84	246	53	53	53
query85	968	273	264	264
query86	392	382	389	382
query87	3552	3346	3371	3346
query88	3061	2279	2253	2253
query89	352	262	266	262
query90	1986	196	197	196
query91	119	88	106	88
query92	61	53	51	51
query93	1415	444	425	425
query94	976	176	178	176
query95	465	422	399	399
query96	639	329	324	324
query97	4283	4190	4194	4190
query98	206	199	193	193
query99	1094	820	800	800
Total cold run time: 296506 ms
Total hot run time: 178771 ms

@doris-robot
Copy link

(From new machine)TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 47.09 seconds
stream load tsv: 569 seconds loaded 74807831229 Bytes, about 125 MB/s
stream load json: 19 seconds loaded 2358488459 Bytes, about 118 MB/s
stream load orc: 66 seconds loaded 1101869774 Bytes, about 15 MB/s
stream load parquet: 32 seconds loaded 861443392 Bytes, about 25 MB/s
insert into select: 27.5 seconds inserted 10000000 Rows, about 363K ops/s
storage size: 17183853002 Bytes

@doris-robot
Copy link

TPC-H test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G', run with scripts in https://github.com/apache/doris/tree/master/tools/tpch-tools

Tpch sf100 test result on commit 986988e5273203b89b1c0260f40cc326c26f002d, data reload: false

run tpch-sf100 query with default conf and session variables
q1	5475	5136	5161	5136
q2	389	166	144	144
q3	1472	1157	1156	1156
q4	1074	779	788	779
q5	3152	3113	3097	3097
q6	220	131	135	131
q7	972	549	536	536
q8	2155	2276	2276	2276
q9	6701	6720	6654	6654
q10	3197	3134	3178	3134
q11	354	233	221	221
q12	370	235	225	225
q13	4432	3622	3650	3622
q14	251	220	212	212
q15	567	550	529	529
q16	462	386	398	386
q17	1045	495	511	495
q18	7106	6763	7642	6763
q19	1644	1564	1439	1439
q20	592	353	342	342
q21	2857	2522	2461	2461
q22	405	324	345	324
Total cold run time: 44892 ms
Total hot run time: 40062 ms

run tpch-sf100 query with default conf and set session variable runtime_filter_mode=off
q1	5099	5089	5038	5038
q2	336	237	235	235
q3	3349	3327	3318	3318
q4	2145	2069	2067	2067
q5	5943	5919	5928	5919
q6	225	124	127	124
q7	2364	1893	1956	1893
q8	3561	3668	3666	3666
q9	9048	8988	8985	8985
q10	3849	3906	3901	3901
q11	569	483	481	481
q12	799	657	618	618
q13	3885	3170	3184	3170
q14	302	289	267	267
q15	597	534	531	531
q16	562	501	513	501
q17	2039	1819	1801	1801
q18	8726	8329	8365	8329
q19	1742	1688	1697	1688
q20	2275	1969	1970	1969
q21	5763	5330	5396	5330
q22	557	505	485	485
Total cold run time: 63735 ms
Total hot run time: 60316 ms

morningman
morningman approved these changes Jan 30, 2024
View reviewed changes
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Jan 30, 2024
@py023
Copy link
Contributor Author

py023 commented Jan 30, 2024

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TPC-H: Total hot run time: 36864 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed, data reload: false

------ Round 1 ----------------------------------
q1	17697	4545	4402	4402
q2	2038	141	135	135
q3	10668	916	918	916
q4	4649	801	771	771
q5	7928	2761	2835	2761
q6	184	120	120	120
q7	1106	721	719	719
q8	9287	2016	2008	2008
q9	7671	6313	6309	6309
q10	8100	2463	2444	2444
q11	420	215	213	213
q12	772	282	273	273
q13	18028	3303	3273	3273
q14	278	260	252	252
q15	524	497	478	478
q16	457	406	427	406
q17	949	517	509	509
q18	6979	5969	5881	5881
q19	1569	1404	1364	1364
q20	612	353	326	326
q21	6864	3015	3094	3015
q22	817	312	289	289
Total cold run time: 107597 ms
Total hot run time: 36864 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4414	4520	4452	4452
q2	343	224	223	223
q3	2999	2833	2788	2788
q4	1854	1655	1620	1620
q5	5118	5224	5267	5224
q6	192	113	116	113
q7	2114	1832	1794	1794
q8	3081	3258	3296	3258
q9	8337	8283	8277	8277
q10	5973	3632	3551	3551
q11	529	484	443	443
q12	773	583	559	559
q13	11367	3059	3085	3059
q14	268	249	251	249
q15	544	480	490	480
q16	530	438	458	438
q17	1847	1691	1687	1687
q18	7953	7931	7608	7608
q19	10739	1499	1507	1499
q20	2129	1925	1900	1900
q21	4731	4490	4525	4490
q22	539	479	444	444
Total cold run time: 76374 ms
Total hot run time: 54156 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 173274 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed, data reload: false

query1	923	330	325	325
query2	6579	1916	2024	1916
query3	6702	209	208	208
query4	27387	22062	22195	22062
query5	4452	413	345	345
query6	243	162	153	153
query7	4605	262	259	259
query8	243	172	174	172
query9	8962	2249	2217	2217
query10	413	208	208	208
query11	16572	15460	15336	15336
query12	118	66	62	62
query13	1679	365	364	364
query14	9240	6510	6406	6406
query15	208	184	177	177
query16	6232	255	249	249
query17	1866	476	464	464
query18	1856	279	250	250
query19	172	131	128	128
query20	70	68	70	68
query21	191	130	131	130
query22	5078	4999	4917	4917
query23	31199	30410	30316	30316
query24	10933	2744	2780	2744
query25	549	308	301	301
query26	865	135	136	135
query27	2777	289	284	284
query28	6235	1846	1835	1835
query29	944	611	602	602
query30	281	135	136	135
query31	891	717	720	717
query32	89	50	49	49
query33	574	208	211	208
query34	878	443	458	443
query35	839	784	762	762
query36	1351	1232	1203	1203
query37	94	57	56	56
query38	3302	3180	3161	3161
query39	1308	1232	1240	1232
query40	193	82	79	79
query41	37	37	35	35
query42	89	82	83	82
query43	512	462	496	462
query44	1034	684	681	681
query45	194	177	172	172
query46	1036	664	631	631
query47	1596	1558	1505	1505
query48	393	301	294	294
query49	1114	282	288	282
query50	691	307	312	307
query51	5273	5131	5178	5131
query52	92	79	83	79
query53	320	255	255	255
query54	233	176	196	176
query55	77	74	73	73
query56	173	158	167	158
query57	979	910	913	910
query58	182	160	163	160
query59	2394	2403	2515	2403
query60	204	176	176	176
query61	86	83	88	83
query62	574	364	352	352
query63	271	260	269	260
query64	4018	1765	1693	1693
query65	3287	3210	3203	3203
query66	1134	314	316	314
query67	14529	14406	14448	14406
query68	5424	503	499	499
query69	450	315	309	309
query70	1543	1514	1535	1514
query71	275	208	208	208
query72	4059	2825	2845	2825
query73	686	319	318	318
query74	6778	6208	6277	6208
query75	2846	2316	2320	2316
query76	3417	979	899	899
query77	374	242	226	226
query78	9373	8845	8495	8495
query79	2098	498	516	498
query80	1232	323	311	311
query81	544	194	204	194
query82	677	82	78	78
query83	220	112	115	112
query84	231	74	73	73
query85	2046	340	358	340
query86	523	414	386	386
query87	3410	3253	3278	3253
query88	3796	2272	2259	2259
query89	433	334	351	334
query90	1910	190	189	189
query91	171	116	115	115
query92	55	45	42	42
query93	2115	423	429	423
query94	1381	158	163	158
query95	513	464	452	452
query96	628	326	332	326
query97	4254	4100	4179	4100
query98	198	197	193	193
query99	961	660	691	660
Total cold run time: 270236 ms
Total hot run time: 173274 ms

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 36.19% (8626/23835)
Line Coverage: 28.25% (70544/249689)
Region Coverage: 27.25% (36406/133580)
Branch Coverage: 24.03% (18642/77562)
Coverage Report: http://coverage.selectdb-in.cc/coverage/642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed_642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed/report/index.html

@doris-robot
Copy link

ClickBench: Total hot run time: 30.33 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed, data reload: false

query1	0.04	0.03	0.02
query2	0.05	0.02	0.03
query3	0.22	0.06	0.06
query4	1.68	0.10	0.10
query5	0.53	0.51	0.50
query6	1.20	0.65	0.64
query7	0.02	0.01	0.02
query8	0.03	0.02	0.02
query9	0.56	0.50	0.52
query10	0.55	0.56	0.55
query11	0.12	0.08	0.08
query12	0.11	0.09	0.10
query13	0.60	0.62	0.59
query14	0.76	0.81	0.82
query15	0.78	0.76	0.78
query16	0.40	0.37	0.38
query17	0.99	1.04	1.04
query18	0.22	0.24	0.26
query19	1.85	1.77	1.77
query20	0.01	0.01	0.01
query21	15.41	0.58	0.60
query22	2.22	2.43	1.92
query23	17.07	0.83	0.76
query24	2.56	0.30	0.41
query25	0.38	0.11	0.16
query26	0.41	0.14	0.14
query27	0.06	0.06	0.05
query28	12.40	0.83	0.83
query29	12.50	3.17	3.09
query30	0.65	0.48	0.55
query31	2.79	0.34	0.36
query32	3.37	0.48	0.49
query33	3.22	3.21	3.21
query34	15.91	4.27	4.30
query35	4.28	4.28	4.25
query36	1.12	1.06	1.06
query37	0.06	0.05	0.05
query38	0.04	0.03	0.03
query39	0.02	0.01	0.01
query40	0.16	0.13	0.13
query41	0.07	0.02	0.02
query42	0.02	0.02	0.02
query43	0.02	0.02	0.02
Total cold run time: 105.46 s
Total hot run time: 30.33 s

@doris-robot
Copy link

Load test result on machine: 'aliyun_ecs.c7a.8xlarge_32C64G'

Load test result on commit 642c87eb8c8cd31c9f5a4b0140ea22645eefa1ed with default session variables
Stream load json:         19 seconds loaded 2358488459 Bytes, about 118 MB/s
Stream load orc:          58 seconds loaded 1101869774 Bytes, about 18 MB/s
Stream load parquet:      31 seconds loaded 861443392 Bytes, about 26 MB/s
Insert into select:       13.6 seconds inserted 10000000 Rows, about 735K ops/s

morningman
morningman approved these changes Feb 5, 2024
View reviewed changes
Copy link
Contributor

@morningman morningman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morningman morningman merged commit 98e44fb into apache:master Feb 5, 2024
25 of 26 checks passed
yiguolei pushed a commit that referenced this pull request Feb 5, 2024
@py023 @Doris-Extras
[chore](clang-tidy): add bugprone linters (#29521)
4b42156 
This PR introduces 4 bugprone linter rules to .clang-tidy, these linters found some bugs in #28965. This PR also add some comments to mute false positive reports.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@morningman morningman morningman approved these changes

@gavinchou gavinchou gavinchou approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@py023 @gavinchou @doris-robot @morningman