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.

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

[refactor](local exchange) Simplify block wrapper #47620

Merged
merged 3 commits into from
Feb 8, 2025

Conversation

Gabriel39
Copy link
Contributor

@Gabriel39 Gabriel39 commented Feb 8, 2025

What problem does this PR solve?

Block wrapper which is used by local exchanger maintains a data block and a reference count which indicates if the block could be freed.
This PR simplify the control logics. A block wrapper will be hold by a shared pointer and we just clear the data block in the de-constructor instead of relying another ref count.

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@Gabriel39
Copy link
Contributor Author

run buildall

@Thearas
Copy link
Contributor

Thearas commented Feb 8, 2025

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

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 42.60% (11157/26193)
Line Coverage: 32.63% (93877/287675)
Region Coverage: 31.79% (48136/151415)
Branch Coverage: 27.72% (24309/87710)
Coverage Report: http://coverage.selectdb-in.cc/coverage/cfb6f1c6e82d7a2f8a5d6fd0d892068a145452da_cfb6f1c6e82d7a2f8a5d6fd0d892068a145452da/report/index.html

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
region	Doris	NULL	NULL	0	0	0	NULL	NULL	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:27:24	NULL	utf-8	NULL	NULL	
revenue0	View	NULL	NULL	-1	0	0	NULL	NULL	NULL	NULL	2023-12-26 18:27:24	NULL	NULL	utf-8	NULL	NULL	
supplier	Doris	NULL	NULL	0	0	0	NULL	NULL	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:27:25	NULL	utf-8	NULL	NULL	
============================================
q1	17601	5171	5091	5091
q2	2047	333	177	177
q3	10391	1256	741	741
q4	10215	1009	541	541
q5	7532	2404	2320	2320
q6	192	165	131	131
q7	885	744	582	582
q8	q9	q10	q11	q12	526	205	204	204
q13	17790	3184	3046	3046
q14	215	226	203	203
q15	476	428	448	428
q16	620	593	573	573
q17	561	888	338	338
q18	6746	6267	6140	6140
q19	1074	934	518	518
q20	803	310	180	180
q21	2721	2138	1912	1912
q22	821	315	289	289
Total cold run time: 81216 ms
Total hot run time: 23414 ms

----- Round 2, with runtime_filter_mode=off -----
region	Doris	NULL	NULL	5	240	1201	NULL	NULL	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:27:24	NULL	utf-8	NULL	NULL	
revenue0	View	NULL	NULL	-1	0	0	NULL	NULL	NULL	NULL	2023-12-26 18:27:24	NULL	NULL	utf-8	NULL	NULL	
supplier	Doris	NULL	NULL	1000000	87	87519212	NULL	NULL	NULL	NULL	2023-12-26 18:27:23	2023-12-26 18:27:25	NULL	utf-8	NULL	NULL	
============================================
q1	5102	5106	5317	5106
q2	234	322	222	222
q3	2099	2669	2225	2225
q4	1392	1801	1358	1358
q5	4225	4070	4162	4070
q6	202	159	123	123
q7	1860	1769	1619	1619
q8	q9	q10	q11	q12	851	567	567	567
q13	3594	3170	3018	3018
q14	275	270	265	265
q15	501	458	446	446
q16	620	672	613	613
q17	1116	1495	1321	1321
q18	7038	6980	6885	6885
q19	750	814	772	772
q20	1884	1935	1796	1796
q21	5186	4707	4639	4639
q22	652	549	581	549
Total cold run time: 37581 ms
Total hot run time: 35594 ms

@Gabriel39
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17609	5219	5089	5089
q2	2063	312	175	175
q3	10465	1313	712	712
q4	10250	1020	512	512
q5	7783	2359	2381	2359
q6	189	164	130	130
q7	895	750	626	626
q8	9299	1330	1022	1022
q9	4862	4662	4844	4662
q10	6839	2285	1887	1887
q11	474	268	261	261
q12	345	353	204	204
q13	17791	3672	3101	3101
q14	223	220	209	209
q15	515	484	455	455
q16	619	622	593	593
q17	568	880	332	332
q18	6947	6333	6251	6251
q19	1237	955	545	545
q20	308	327	191	191
q21	3112	2162	1913	1913
q22	361	322	311	311
Total cold run time: 102754 ms
Total hot run time: 31540 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5177	5137	5106	5106
q2	234	331	230	230
q3	2123	2690	2302	2302
q4	1472	1824	1359	1359
q5	4262	4112	4158	4112
q6	204	161	123	123
q7	1892	1828	1634	1634
q8	2527	2457	2469	2457
q9	6820	6719	6686	6686
q10	2903	3091	2642	2642
q11	544	502	482	482
q12	673	705	571	571
q13	3347	3754	3098	3098
q14	266	284	256	256
q15	492	465	452	452
q16	647	665	624	624
q17	1113	1579	1304	1304
q18	7266	7083	6948	6948
q19	780	749	804	749
q20	1912	1911	1791	1791
q21	5189	4826	4670	4670
q22	658	567	548	548
Total cold run time: 50501 ms
Total hot run time: 48144 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 182942 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 4d8043fc30de7cadaa478711cf1403ba82d016cf, data reload: false

query1	976	385	377	377
query2	6515	1893	1903	1893
query3	6790	213	208	208
query4	26410	23297	22917	22917
query5	4335	668	482	482
query6	311	201	188	188
query7	4601	501	313	313
query8	299	255	242	242
query9	8602	2559	2592	2559
query10	450	319	264	264
query11	15875	15071	14942	14942
query12	174	109	109	109
query13	1658	523	413	413
query14	11315	6702	6432	6432
query15	248	193	183	183
query16	7772	623	484	484
query17	1606	735	589	589
query18	2002	408	317	317
query19	224	189	170	170
query20	128	129	115	115
query21	214	122	105	105
query22	4327	4240	4097	4097
query23	33935	32880	33172	32880
query24	6857	2392	2408	2392
query25	523	429	366	366
query26	1213	266	152	152
query27	2118	479	340	340
query28	3970	2418	2370	2370
query29	710	567	408	408
query30	237	197	159	159
query31	951	853	791	791
query32	71	66	63	63
query33	597	348	304	304
query34	789	860	494	494
query35	788	815	767	767
query36	969	973	896	896
query37	131	99	78	78
query38	4086	4222	4029	4029
query39	1451	1390	1394	1390
query40	207	115	104	104
query41	54	51	52	51
query42	121	101	103	101
query43	497	523	476	476
query44	1302	788	783	783
query45	174	167	158	158
query46	851	1032	652	652
query47	1747	1806	1678	1678
query48	377	416	305	305
query49	781	504	427	427
query50	699	726	407	407
query51	4185	4148	4187	4148
query52	109	104	96	96
query53	222	274	188	188
query54	491	469	422	422
query55	88	81	79	79
query56	298	273	255	255
query57	1133	1147	1061	1061
query58	248	232	239	232
query59	2735	2779	2606	2606
query60	288	257	249	249
query61	118	111	109	109
query62	842	728	671	671
query63	226	192	190	190
query64	4307	1010	686	686
query65	3216	3178	3120	3120
query66	1057	405	303	303
query67	15672	15434	15480	15434
query68	7971	765	518	518
query69	469	305	265	265
query70	1193	1136	1081	1081
query71	420	303	274	274
query72	5720	3482	3793	3482
query73	751	734	344	344
query74	9168	8957	8715	8715
query75	3363	3171	2695	2695
query76	3279	1176	751	751
query77	620	385	289	289
query78	10009	10131	9206	9206
query79	3251	811	594	594
query80	597	543	472	472
query81	506	271	237	237
query82	696	153	120	120
query83	172	176	150	150
query84	248	103	72	72
query85	778	360	316	316
query86	374	328	302	302
query87	4368	4463	4376	4376
query88	3797	2195	2164	2164
query89	387	311	286	286
query90	1934	192	193	192
query91	130	146	106	106
query92	79	64	60	60
query93	2088	1033	579	579
query94	694	410	302	302
query95	350	264	261	261
query96	480	572	265	265
query97	2787	2838	2675	2675
query98	220	201	200	200
query99	1417	1392	1324	1324
Total cold run time: 274589 ms
Total hot run time: 182942 ms

Copy link
Contributor

@yiguolei yiguolei left a comment

Choose a reason for hiding this comment

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

LGTM

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.07	0.03	0.03
query3	0.23	0.07	0.06
query4	1.62	0.10	0.10
query5	0.42	0.41	0.39
query6	1.17	0.68	0.65
query7	0.03	0.02	0.01
query8	0.04	0.03	0.03
query9	0.58	0.51	0.52
query10	0.56	0.57	0.57
query11	0.14	0.10	0.11
query12	0.14	0.11	0.11
query13	0.62	0.60	0.60
query14	2.84	2.83	2.70
query15	0.92	0.86	0.83
query16	0.37	0.37	0.36
query17	1.03	1.00	1.01
query18	0.21	0.19	0.20
query19	1.88	1.74	1.98
query20	0.01	0.01	0.02
query21	15.38	0.89	0.53
query22	0.77	1.28	0.62
query23	14.88	1.38	0.64
query24	6.72	0.70	1.52
query25	0.53	0.18	0.13
query26	0.66	0.16	0.13
query27	0.05	0.05	0.05
query28	9.20	0.86	0.43
query29	12.54	4.00	3.32
query30	0.24	0.09	0.06
query31	2.84	0.58	0.38
query32	3.22	0.55	0.46
query33	2.96	3.03	3.02
query34	15.81	5.13	4.49
query35	4.58	4.54	4.56
query36	0.70	0.50	0.48
query37	0.09	0.07	0.06
query38	0.05	0.04	0.03
query39	0.02	0.03	0.03
query40	0.16	0.14	0.13
query41	0.09	0.04	0.02
query42	0.04	0.02	0.02
query43	0.04	0.03	0.02
Total cold run time: 104.48 s
Total hot run time: 30.31 s

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 8, 2025
Copy link
Contributor

github-actions bot commented Feb 8, 2025

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

Copy link
Contributor

github-actions bot commented Feb 8, 2025

PR approved by anyone and no changes requested.

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 42.64% (11167/26192)
Line Coverage: 32.65% (93922/287667)
Region Coverage: 31.82% (48174/151414)
Branch Coverage: 27.74% (24327/87710)
Coverage Report: http://coverage.selectdb-in.cc/coverage/4d8043fc30de7cadaa478711cf1403ba82d016cf_4d8043fc30de7cadaa478711cf1403ba82d016cf/report/index.html

@Gabriel39 Gabriel39 merged commit 34ef39e into apache:master Feb 8, 2025
24 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants