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

[Refactor](Variant) should not call finalize in const functions #37839

Merged
merged 2 commits into from
Jul 16, 2024
Merged

[Refactor](Variant) should not call finalize in const functions #37839

merged 2 commits into from
Jul 16, 2024

Conversation

eldenmoon
Copy link
Member

finalize may change the inner data structure in ColumnObject, and lead to heap use after free in some scenario

Proposed changes

Issue Number: close #xxx

@eldenmoon
[Refactor](Variant) should not call finalize in const functions
9cdc963 
`finalize` may change the inner data structure in ColumnObject, and lead to heap use after free in some scenario
@doris-robot
Copy link

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR

Since 2024-03-18, the Document has been moved to doris-website.
See Doris Document.

@eldenmoon
Copy link
Member Author

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: 40054 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 9cdc963d96f5288677197ebf3e4568c966433c7e, data reload: false

------ Round 1 ----------------------------------
q1	18245	4595	4373	4373
q2	2587	197	203	197
q3	11020	1185	1098	1098
q4	10249	832	751	751
q5	8065	2754	2686	2686
q6	232	145	142	142
q7	995	622	608	608
q8	9579	2108	2118	2108
q9	8816	6603	6582	6582
q10	8880	3755	3799	3755
q11	453	236	242	236
q12	396	232	224	224
q13	17779	2989	2988	2988
q14	275	241	239	239
q15	520	476	486	476
q16	502	397	377	377
q17	978	647	737	647
q18	8169	7603	7360	7360
q19	7736	1441	1348	1348
q20	658	324	326	324
q21	5026	3267	3259	3259
q22	363	296	276	276
Total cold run time: 121523 ms
Total hot run time: 40054 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4386	4262	4257	4257
q2	368	272	273	272
q3	2985	2761	2781	2761
q4	1924	1605	1589	1589
q5	5306	5360	5366	5360
q6	221	133	138	133
q7	2150	1739	1751	1739
q8	3240	3358	3338	3338
q9	8481	8494	8488	8488
q10	3915	3709	3726	3709
q11	615	476	496	476
q12	784	604	606	604
q13	16378	2974	2953	2953
q14	297	280	273	273
q15	519	466	471	466
q16	477	410	446	410
q17	1812	1475	1463	1463
q18	7596	7542	7305	7305
q19	1691	1626	1452	1452
q20	1982	1811	1775	1775
q21	4865	4656	4766	4656
q22	571	491	483	483
Total cold run time: 70563 ms
Total hot run time: 53962 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 172967 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 9cdc963d96f5288677197ebf3e4568c966433c7e, data reload: false

query1	913	377	369	369
query2	6455	1881	1809	1809
query3	6657	207	216	207
query4	27934	17740	17488	17488
query5	4286	508	528	508
query6	289	191	175	175
query7	4600	310	306	306
query8	252	212	210	210
query9	8506	2387	2387	2387
query10	447	293	282	282
query11	12710	9989	10043	9989
query12	137	90	89	89
query13	1639	373	355	355
query14	10136	7312	7755	7312
query15	224	169	165	165
query16	7836	332	318	318
query17	1785	556	527	527
query18	1965	279	297	279
query19	203	153	153	153
query20	90	82	85	82
query21	212	126	130	126
query22	4278	4083	4023	4023
query23	33980	33381	33096	33096
query24	11900	2947	2890	2890
query25	663	381	381	381
query26	1842	151	154	151
query27	2899	270	274	270
query28	7642	1973	1961	1961
query29	1148	632	659	632
query30	287	149	148	148
query31	959	742	751	742
query32	101	56	53	53
query33	770	312	300	300
query34	918	502	492	492
query35	689	565	589	565
query36	1097	962	942	942
query37	205	80	84	80
query38	2863	2776	2767	2767
query39	891	824	836	824
query40	280	123	125	123
query41	48	44	45	44
query42	120	102	102	102
query43	517	465	466	465
query44	1194	748	740	740
query45	194	166	162	162
query46	1109	748	755	748
query47	1858	1790	1769	1769
query48	373	306	301	301
query49	1187	435	424	424
query50	821	403	403	403
query51	6881	6757	6836	6757
query52	100	94	91	91
query53	363	305	303	303
query54	942	444	444	444
query55	78	75	74	74
query56	295	273	290	273
query57	1149	1064	1098	1064
query58	265	275	261	261
query59	2981	2568	2713	2568
query60	302	286	294	286
query61	106	101	103	101
query62	841	642	651	642
query63	338	299	307	299
query64	10548	2272	1700	1700
query65	3211	3148	3116	3116
query66	1380	355	332	332
query67	15298	14847	14849	14847
query68	5462	548	528	528
query69	644	476	360	360
query70	1142	1161	1183	1161
query71	446	291	311	291
query72	7393	5500	5478	5478
query73	797	326	326	326
query74	6209	5687	5754	5687
query75	3768	2718	2727	2718
query76	3621	904	970	904
query77	709	329	315	315
query78	9547	9177	8979	8979
query79	3161	533	531	531
query80	2191	487	493	487
query81	574	228	220	220
query82	746	145	146	145
query83	272	183	180	180
query84	274	89	99	89
query85	1451	314	307	307
query86	446	312	314	312
query87	3280	3152	3084	3084
query88	4497	2445	2473	2445
query89	509	409	415	409
query90	1919	198	198	198
query91	135	102	107	102
query92	68	50	54	50
query93	4108	515	495	495
query94	1321	223	215	215
query95	404	318	322	318
query96	626	274	270	270
query97	3244	2986	3035	2986
query98	227	202	203	202
query99	1635	1283	1292	1283
Total cold run time: 294013 ms
Total hot run time: 172967 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.04
query2	0.08	0.04	0.04
query3	0.22	0.06	0.05
query4	1.66	0.08	0.08
query5	0.51	0.50	0.50
query6	1.14	0.73	0.72
query7	0.03	0.02	0.01
query8	0.05	0.05	0.05
query9	0.55	0.50	0.49
query10	0.52	0.55	0.55
query11	0.16	0.12	0.11
query12	0.14	0.12	0.13
query13	0.59	0.59	0.60
query14	0.78	0.78	0.79
query15	0.86	0.82	0.82
query16	0.37	0.36	0.37
query17	0.98	0.97	0.96
query18	0.22	0.22	0.22
query19	1.79	1.70	1.69
query20	0.01	0.01	0.02
query21	15.41	0.75	0.65
query22	3.85	7.08	1.94
query23	18.26	1.43	1.37
query24	2.14	0.22	0.22
query25	0.16	0.09	0.08
query26	0.29	0.21	0.21
query27	0.45	0.23	0.23
query28	13.27	1.02	1.01
query29	12.66	3.39	3.39
query30	0.26	0.06	0.06
query31	2.86	0.39	0.40
query32	3.27	0.49	0.48
query33	2.92	2.89	2.93
query34	17.17	4.39	4.40
query35	4.48	4.45	4.40
query36	0.66	0.49	0.47
query37	0.18	0.16	0.16
query38	0.16	0.15	0.15
query39	0.05	0.03	0.04
query40	0.15	0.12	0.13
query41	0.10	0.05	0.05
query42	0.05	0.05	0.04
query43	0.04	0.04	0.04
Total cold run time: 109.54 s
Total hot run time: 30.95 s

@eldenmoon
Copy link
Member Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

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

@eldenmoon eldenmoon added the p0_c label Jul 15, 2024
@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17991	5190	4880	4880
q2	2658	196	186	186
q3	11511	1279	1217	1217
q4	10751	927	878	878
q5	7971	2941	2891	2891
q6	248	144	142	142
q7	1079	623	614	614
q8	9365	2294	2325	2294
q9	9103	6972	6985	6972
q10	8905	3963	3963	3963
q11	464	245	246	245
q12	438	233	223	223
q13	18738	2972	2981	2972
q14	291	250	238	238
q15	544	505	499	499
q16	515	385	385	385
q17	1039	739	684	684
q18	8160	7601	7481	7481
q19	8726	1499	1609	1499
q20	722	320	335	320
q21	5284	3426	3564	3426
q22	377	284	284	284
Total cold run time: 124880 ms
Total hot run time: 42293 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4787	4709	4676	4676
q2	408	266	253	253
q3	3174	2851	2853	2851
q4	2029	1705	1700	1700
q5	5505	5503	5472	5472
q6	244	132	135	132
q7	2213	1750	1736	1736
q8	3440	3683	3656	3656
q9	8576	8526	8555	8526
q10	4092	3781	3819	3781
q11	636	495	482	482
q12	824	621	607	607
q13	17518	3014	3016	3014
q14	308	281	269	269
q15	548	482	487	482
q16	491	414	426	414
q17	1928	1560	1572	1560
q18	7732	7638	7354	7354
q19	1911	1701	1674	1674
q20	2055	1852	1788	1788
q21	5032	4934	4841	4841
q22	615	481	507	481
Total cold run time: 74066 ms
Total hot run time: 55749 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 172753 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 4434a8359edbb6c6ee1c60e084cd8de94194f11d, data reload: false

query1	929	382	373	373
query2	6452	1818	1872	1818
query3	6648	212	216	212
query4	28282	17520	17392	17392
query5	4165	485	488	485
query6	268	177	168	168
query7	4596	295	304	295
query8	253	193	199	193
query9	8662	2436	2398	2398
query10	434	283	268	268
query11	10821	10024	10099	10024
query12	128	82	81	81
query13	1629	358	366	358
query14	9798	7290	7655	7290
query15	211	170	167	167
query16	7809	305	310	305
query17	1657	549	526	526
query18	1974	286	287	286
query19	232	145	151	145
query20	91	85	77	77
query21	202	126	125	125
query22	4424	4029	4106	4029
query23	33779	34050	33297	33297
query24	12103	2871	2845	2845
query25	552	364	368	364
query26	1364	154	149	149
query27	2865	276	284	276
query28	7255	2069	2022	2022
query29	863	627	616	616
query30	287	146	148	146
query31	962	726	754	726
query32	96	56	54	54
query33	774	308	299	299
query34	932	493	488	488
query35	713	574	584	574
query36	1082	954	941	941
query37	137	82	80	80
query38	2879	2744	2738	2738
query39	853	829	827	827
query40	264	123	120	120
query41	51	41	43	41
query42	118	98	103	98
query43	509	471	460	460
query44	1179	747	724	724
query45	194	161	161	161
query46	1113	710	708	708
query47	1878	1781	1769	1769
query48	358	294	298	294
query49	1172	426	420	420
query50	774	399	396	396
query51	6933	6922	6876	6876
query52	109	93	95	93
query53	360	302	299	299
query54	916	454	451	451
query55	75	76	76	76
query56	292	259	267	259
query57	1182	1060	1033	1033
query58	270	243	246	243
query59	2891	2550	2548	2548
query60	292	276	285	276
query61	99	160	91	91
query62	827	652	648	648
query63	321	297	286	286
query64	10390	2215	1662	1662
query65	3231	3101	3119	3101
query66	1276	330	323	323
query67	15419	15036	15028	15028
query68	4621	558	548	548
query69	471	330	340	330
query70	1140	1037	1101	1037
query71	454	278	282	278
query72	6936	5533	6043	5533
query73	741	333	327	327
query74	6265	5681	5701	5681
query75	3436	2684	2721	2684
query76	2780	920	868	868
query77	481	311	315	311
query78	9554	8946	9015	8946
query79	2020	521	519	519
query80	2021	479	497	479
query81	595	220	227	220
query82	994	137	130	130
query83	299	168	166	166
query84	266	87	86	86
query85	1344	304	302	302
query86	472	313	301	301
query87	3272	3188	3119	3119
query88	3710	2388	2402	2388
query89	483	386	394	386
query90	1784	202	196	196
query91	132	101	105	101
query92	70	51	50	50
query93	1927	529	517	517
query94	1249	208	214	208
query95	416	324	325	324
query96	591	277	276	276
query97	3220	3021	3016	3016
query98	222	199	192	192
query99	1703	1294	1293	1293
Total cold run time: 283323 ms
Total hot run time: 172753 ms

@doris-robot
Copy link

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

query1	0.04	0.03	0.03
query2	0.07	0.04	0.03
query3	0.23	0.05	0.06
query4	1.67	0.07	0.08
query5	0.50	0.50	0.50
query6	1.13	0.73	0.72
query7	0.02	0.02	0.01
query8	0.05	0.04	0.06
query9	0.57	0.49	0.49
query10	0.54	0.54	0.56
query11	0.15	0.11	0.11
query12	0.14	0.11	0.12
query13	0.59	0.59	0.58
query14	0.76	0.77	0.83
query15	0.84	0.83	0.81
query16	0.36	0.37	0.38
query17	0.96	1.04	1.03
query18	0.23	0.22	0.21
query19	1.75	1.74	1.76
query20	0.01	0.01	0.01
query21	15.40	0.76	0.65
query22	3.86	5.67	3.14
query23	18.33	1.48	1.24
query24	2.09	0.24	0.24
query25	0.15	0.09	0.09
query26	0.30	0.20	0.21
query27	0.46	0.23	0.24
query28	13.20	1.03	0.99
query29	12.67	3.27	3.28
query30	0.25	0.06	0.05
query31	2.86	0.39	0.39
query32	3.28	0.49	0.47
query33	2.88	2.90	2.91
query34	16.97	4.35	4.37
query35	4.40	4.49	4.42
query36	0.65	0.47	0.47
query37	0.18	0.17	0.16
query38	0.16	0.16	0.14
query39	0.05	0.04	0.03
query40	0.15	0.13	0.12
query41	0.08	0.05	0.04
query42	0.06	0.05	0.05
query43	0.05	0.04	0.04
Total cold run time: 109.09 s
Total hot run time: 31.9 s

xiaokang
xiaokang approved these changes Jul 16, 2024
View reviewed changes
Copy link
Contributor

@xiaokang xiaokang 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 bot added the approved Indicates a PR has been approved by one committer. label Jul 16, 2024
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@yiguolei yiguolei merged commit 2020d01 into apache:master Jul 16, 2024
25 of 29 checks passed
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 16, 2024
@eldenmoon
[Refactor](Variant) should not call finalize in const functions (apac…
a6aad0a 
…he#37839)

`finalize` may change the inner data structure in ColumnObject, and lead
to heap use after free in some scenario

Issue Number: close #xxx

<!--Describe your changes.-->
@eldenmoon eldenmoon mentioned this pull request Jul 16, 2024
Merged
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Jul 16, 2024
@eldenmoon
[Refactor](Variant) should not call finalize in const functions (apac…
1e5e4cb 
…he#37839)

`finalize` may change the inner data structure in ColumnObject, and lead
to heap use after free in some scenario

## Proposed changes

Issue Number: close #xxx

<!--Describe your changes.-->
@eldenmoon eldenmoon mentioned this pull request Jul 16, 2024
Merged
yiguolei pushed a commit that referenced this pull request Jul 16, 2024
@eldenmoon
[Pick](Variant) pick some fix (#37922)
b15ccdb 
#37674
#37839
#37883 
#37857 
#37794
xiaokang pushed a commit that referenced this pull request Jul 17, 2024
@eldenmoon
[Pick](Variant) pick some fix (#37926)
2994232 
#37674
#37839
#37883
#37857
#37794
#37640
#37557
#37217
#37121
seawinde pushed a commit to seawinde/doris that referenced this pull request Jul 17, 2024
@eldenmoon @seawinde
[Refactor](Variant) should not call finalize in const functions (apac…
3f897ce 
…he#37839)

`finalize` may change the inner data structure in ColumnObject, and lead
to heap use after free in some scenario

## Proposed changes

Issue Number: close #xxx

<!--Describe your changes.-->
@yiguolei yiguolei mentioned this pull request Jul 19, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaokang xiaokang xiaokang approved these changes

@yiguolei yiguolei yiguolei approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.5-merged dev/3.0.0-merged p0_c reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@eldenmoon @doris-robot @xiaokang @yiguolei