Skip to content

[fix](memory) Fix ThreadMemTrackerMgr::limiter_mem_tracker() performance #50462

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
Merged
xinyiZzz merged 1 commit into from
Apr 28, 2025
Merged

[fix](memory) Fix ThreadMemTrackerMgr::limiter_mem_tracker() performance #50462

xinyiZzz merged 1 commit into from
Apr 28, 2025

Conversation

@xinyiZzz
Copy link
Contributor

@xinyiZzz xinyiZzz commented Apr 27, 2025

What problem does this PR solve?

Prefer use limiter_mem_tracker, which is faster than limiter_mem_tracker_sptr.
when multiple threads hold the same std::shared_ptr at the same time,
modifying the std::shared_ptr reference count will be expensive when there is high concurrency.

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

@hello-stephen
Copy link
Contributor

hello-stephen commented Apr 27, 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?

@xinyiZzz
Copy link
Contributor Author

xinyiZzz commented Apr 27, 2025

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Apr 27, 2025

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

@github-actions github-actions bot added approved Indicates a PR has been approved by one committer. reviewed labels Apr 27, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 27, 2025

PR approved by anyone and no changes requested.

@doris-robot
Copy link

doris-robot commented Apr 27, 2025

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

------ Round 1 ----------------------------------
q1	26533	5293	5069	5069
q2	2091	287	180	180
q3	10386	1266	713	713
q4	10227	1035	538	538
q5	7573	2413	2356	2356
q6	191	165	135	135
q7	930	768	632	632
q8	9326	1335	1133	1133
q9	6867	5075	5123	5075
q10	6838	2334	1889	1889
q11	502	290	285	285
q12	360	351	220	220
q13	17782	3698	3210	3210
q14	230	225	213	213
q15	533	495	483	483
q16	416	429	383	383
q17	610	888	373	373
q18	7588	7311	7134	7134
q19	1556	968	561	561
q20	354	344	230	230
q21	4443	3562	2534	2534
q22	1053	1031	975	975
Total cold run time: 116389 ms
Total hot run time: 34321 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5320	5059	5544	5059
q2	248	335	239	239
q3	2169	2648	2281	2281
q4	1395	1951	1485	1485
q5	4495	4434	4422	4422
q6	236	177	131	131
q7	2043	1906	1743	1743
q8	2617	2659	2626	2626
q9	7304	7132	7234	7132
q10	2993	3189	2774	2774
q11	590	511	490	490
q12	700	753	615	615
q13	3599	3900	3443	3443
q14	285	292	275	275
q15	535	494	503	494
q16	455	490	446	446
q17	1192	1595	1372	1372
q18	7810	7518	7411	7411
q19	823	831	934	831
q20	1937	1967	1865	1865
q21	5221	4797	4792	4792
q22	1121	1102	1018	1018
Total cold run time: 53088 ms
Total hot run time: 50944 ms

@doris-robot
Copy link

doris-robot commented Apr 27, 2025

TPC-DS: Total hot run time: 193099 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 d83cead647a01c48726ad08cbc3cdfef1371a781, data reload: false

query1	1431	1098	1047	1047
query2	6395	1888	1870	1870
query3	11096	4763	4712	4712
query4	25967	23444	22999	22999
query5	3476	587	454	454
query6	308	202	197	197
query7	3990	490	292	292
query8	304	257	244	244
query9	8466	2580	2606	2580
query10	464	355	266	266
query11	15368	15218	14888	14888
query12	166	110	113	110
query13	1565	544	409	409
query14	8874	6452	6530	6452
query15	201	191	170	170
query16	7295	655	531	531
query17	1206	717	611	611
query18	2036	421	327	327
query19	208	196	168	168
query20	125	126	124	124
query21	209	133	118	118
query22	4603	4533	4347	4347
query23	34966	33815	33488	33488
query24	8614	2440	2428	2428
query25	511	478	401	401
query26	731	277	160	160
query27	2874	512	339	339
query28	4604	2135	2125	2125
query29	666	586	447	447
query30	269	230	187	187
query31	931	868	795	795
query32	70	70	64	64
query33	517	374	335	335
query34	805	850	543	543
query35	797	835	750	750
query36	989	987	897	897
query37	111	106	75	75
query38	4165	4217	4160	4160
query39	1472	1500	1434	1434
query40	216	124	105	105
query41	60	58	61	58
query42	124	106	113	106
query43	515	538	502	502
query44	1386	810	813	810
query45	185	182	167	167
query46	854	1033	666	666
query47	1817	1857	1757	1757
query48	386	451	307	307
query49	714	510	432	432
query50	665	711	410	410
query51	4274	4321	4173	4173
query52	107	107	99	99
query53	241	260	195	195
query54	595	607	537	537
query55	83	78	81	78
query56	316	309	285	285
query57	1150	1183	1143	1143
query58	274	268	264	264
query59	2864	2871	2770	2770
query60	328	327	313	313
query61	134	157	126	126
query62	791	764	678	678
query63	226	189	188	188
query64	2986	1045	712	712
query65	4459	4390	4367	4367
query66	1090	403	308	308
query67	16234	15517	15223	15223
query68	8982	885	530	530
query69	488	306	263	263
query70	1230	1133	1086	1086
query71	458	333	296	296
query72	5885	4758	4827	4758
query73	717	643	341	341
query74	9027	9050	9029	9029
query75	4229	3197	2677	2677
query76	3616	1191	770	770
query77	803	472	281	281
query78	10040	10119	9359	9359
query79	2400	807	560	560
query80	627	500	436	436
query81	480	261	220	220
query82	429	123	97	97
query83	276	253	237	237
query84	287	109	95	95
query85	769	409	319	319
query86	334	294	303	294
query87	4357	4406	4235	4235
query88	3239	2184	2175	2175
query89	399	304	290	290
query90	1816	206	210	206
query91	143	146	110	110
query92	76	59	58	58
query93	1727	958	587	587
query94	650	416	310	310
query95	396	297	286	286
query96	490	570	270	270
query97	3193	3205	3130	3130
query98	234	202	208	202
query99	1432	1402	1269	1269
Total cold run time: 279148 ms
Total hot run time: 193099 ms

@doris-robot
Copy link

doris-robot commented Apr 27, 2025

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

query1	0.03	0.04	0.03
query2	0.13	0.10	0.12
query3	0.25	0.19	0.19
query4	1.60	0.19	0.19
query5	0.60	0.60	0.61
query6	1.21	0.72	0.73
query7	0.02	0.02	0.02
query8	0.04	0.03	0.03
query9	0.57	0.52	0.53
query10	0.58	0.58	0.57
query11	0.16	0.12	0.11
query12	0.16	0.11	0.12
query13	0.62	0.61	0.59
query14	0.78	0.81	0.81
query15	0.89	0.86	0.87
query16	0.41	0.39	0.39
query17	1.01	1.04	1.02
query18	0.21	0.20	0.20
query19	1.90	1.80	1.83
query20	0.01	0.01	0.01
query21	15.43	0.92	0.57
query22	0.78	1.14	0.66
query23	14.93	1.38	0.63
query24	7.29	1.18	0.91
query25	0.45	0.22	0.15
query26	0.55	0.18	0.13
query27	0.06	0.06	0.05
query28	9.33	0.90	0.45
query29	12.54	3.99	3.34
query30	0.26	0.11	0.08
query31	2.80	0.60	0.38
query32	3.24	0.54	0.47
query33	3.00	3.08	3.02
query34	15.87	5.12	4.52
query35	4.55	4.56	4.60
query36	0.67	0.51	0.48
query37	0.09	0.07	0.06
query38	0.05	0.04	0.03
query39	0.03	0.02	0.03
query40	0.16	0.14	0.12
query41	0.08	0.03	0.03
query42	0.03	0.02	0.02
query43	0.04	0.03	0.03
Total cold run time: 103.41 s
Total hot run time: 29.58 s

@hello-stephen
Copy link
Contributor

hello-stephen commented Apr 27, 2025

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 54.39% (14743/27108)
Line Coverage 43.46% (128982/296761)
Region Coverage 42.17% (65879/156221)
Branch Coverage 36.68% (33175/90454)

@hello-stephen
Copy link
Contributor

hello-stephen commented Apr 27, 2025

BE Regression P0 && UT Coverage Report

Increment line coverage 94.44% (17/18) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 55.31% (14719/26613)
Line Coverage 44.88% (132975/296268)
Region Coverage 41.81% (76353/182617)
Branch Coverage 35.82% (36889/102994)

wangbo
wangbo approved these changes Apr 28, 2025
View reviewed changes
Copy link
Contributor

@wangbo wangbo left a comment

Choose a reason for hiding this comment

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

LGTM

@xinyiZzz xinyiZzz merged commit c4a6091 into apache:master Apr 28, 2025
27 of 29 checks passed
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Apr 28, 2025
@xinyiZzz
[fix](memory) Fix ThreadMemTrackerMgr::limiter_mem_tracker() perfor…
70cb99a 
…mance (apache#50462)

Prefer use `limiter_mem_tracker`, which is faster than
`limiter_mem_tracker_sptr`.
when multiple threads hold the same `std::shared_ptr` at the same time,
modifying the `std::shared_ptr` reference count will be expensive when
there is high concurrency.
@xinyiZzz xinyiZzz mentioned this pull request Apr 28, 2025
Merged
16 tasks
xinyiZzz added a commit to xinyiZzz/incubator-doris that referenced this pull request Apr 28, 2025
@xinyiZzz
[fix](memory) Fix ThreadMemTrackerMgr::limiter_mem_tracker() perfor…
ca21f28 
…mance (apache#50462)

Prefer use `limiter_mem_tracker`, which is faster than
`limiter_mem_tracker_sptr`.
when multiple threads hold the same `std::shared_ptr` at the same time,
modifying the `std::shared_ptr` reference count will be expensive when
there is high concurrency.
dataroaring pushed a commit that referenced this pull request May 21, 2025
@xinyiZzz
branch-3.0: [fix](memory) Fix `ThreadMemTrackerMgr::limiter_mem_track…
42a3466 
…er()` performance (#50525)

pick #50462
koarz pushed a commit to koarz/doris that referenced this pull request Jun 4, 2025
@xinyiZzz @koarz
[fix](memory) Fix ThreadMemTrackerMgr::limiter_mem_tracker() perfor…
19e9027 
…mance (apache#50462)

### What problem does this PR solve?

Prefer use `limiter_mem_tracker`, which is faster than
`limiter_mem_tracker_sptr`.
when multiple threads hold the same `std::shared_ptr` at the same time,
modifying the `std::shared_ptr` reference count will be expensive when
there is high concurrency.
@gavinchou gavinchou mentioned this pull request Jun 11, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yiguolei yiguolei yiguolei approved these changes

@wangbo wangbo wangbo approved these changes

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/3.0.6-merged reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@xinyiZzz @hello-stephen @doris-robot @yiguolei @wangbo @dataroaring