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

[fix](compile) replace shared_ptr::unique() with std::shared_ptr::use_count() == 1 #37688

Merged
merged 1 commit into from
Jul 12, 2024
Merged

[fix](compile) replace shared_ptr::unique() with std::shared_ptr::use_count() == 1 #37688

merged 1 commit into from
Jul 12, 2024

Conversation

TangSiyang2001
Copy link
Collaborator

Proposed changes

std::shared_ptr::unique is supported until cpp20. It was disabled by default since llvm-18. see.

Therefore, replace it with std::shared_ptr::use_count() == 1 for better compability. (Or we should work with _LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE which is ungraceful.)

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

@TangSiyang2001
Copy link
Collaborator Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

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 Jul 11, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@github-actions GitHub Actions
Copy link
Contributor

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

1 similar comment
@github-actions GitHub Actions
Copy link
Contributor

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

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	18101	4467	4442	4442
q2	2837	195	194	194
q3	12292	1178	1076	1076
q4	10739	770	843	770
q5	7558	2817	2612	2612
q6	217	134	134	134
q7	962	599	603	599
q8	9219	2056	2115	2056
q9	8754	6539	6523	6523
q10	8754	3805	3820	3805
q11	477	246	245	245
q12	412	231	235	231
q13	18667	2986	2948	2948
q14	266	244	237	237
q15	511	480	496	480
q16	516	388	374	374
q17	968	615	769	615
q18	8069	7516	7511	7511
q19	5509	1459	1544	1459
q20	707	321	340	321
q21	4929	3228	3262	3228
q22	400	338	338	338
Total cold run time: 120864 ms
Total hot run time: 40198 ms

----- Round 2, with runtime_filter_mode=off -----
q1	4335	4227	4199	4199
q2	363	266	263	263
q3	2970	2776	2739	2739
q4	1861	1563	1608	1563
q5	5263	5303	5292	5292
q6	217	131	133	131
q7	2135	1730	1677	1677
q8	3199	3327	3310	3310
q9	8406	8372	8364	8364
q10	3885	3732	3686	3686
q11	584	519	474	474
q12	744	612	600	600
q13	17357	2932	2949	2932
q14	309	276	270	270
q15	511	477	469	469
q16	461	433	434	433
q17	1761	1492	1445	1445
q18	7687	7378	7418	7378
q19	1727	1625	1649	1625
q20	1970	1797	1766	1766
q21	4923	4689	4631	4631
q22	611	560	531	531
Total cold run time: 71279 ms
Total hot run time: 53778 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 174591 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 81d484337545f9a7eddc313f3d6881678a21e7a9, data reload: false

query1	914	368	359	359
query2	6449	2529	2371	2371
query3	6650	209	224	209
query4	23817	17620	17458	17458
query5	4199	518	487	487
query6	283	169	170	169
query7	4585	292	286	286
query8	324	306	306	306
query9	8783	2456	2454	2454
query10	424	277	265	265
query11	10753	10040	9956	9956
query12	138	84	85	84
query13	1625	364	361	361
query14	10227	7553	7618	7553
query15	243	186	190	186
query16	7772	311	306	306
query17	1785	573	540	540
query18	1919	290	282	282
query19	202	153	157	153
query20	97	82	80	80
query21	211	129	130	129
query22	4309	4009	4118	4009
query23	33725	32814	33184	32814
query24	11776	2911	2839	2839
query25	656	394	390	390
query26	1631	150	149	149
query27	2962	269	274	269
query28	7813	2131	2073	2073
query29	1023	601	601	601
query30	283	144	145	144
query31	970	731	759	731
query32	95	57	57	57
query33	776	299	315	299
query34	926	478	496	478
query35	668	565	570	565
query36	1103	933	962	933
query37	162	81	78	78
query38	2851	2718	2772	2718
query39	872	814	817	814
query40	278	118	120	118
query41	53	51	55	51
query42	127	101	97	97
query43	600	558	571	558
query44	1227	728	734	728
query45	191	166	165	165
query46	1094	758	723	723
query47	1877	1824	1786	1786
query48	369	298	295	295
query49	1138	421	407	407
query50	774	397	400	397
query51	6930	6836	6716	6716
query52	102	94	95	94
query53	357	294	296	294
query54	985	448	446	446
query55	78	75	74	74
query56	284	270	265	265
query57	1188	1052	1069	1052
query58	255	245	248	245
query59	3564	3369	3298	3298
query60	311	285	275	275
query61	102	120	92	92
query62	843	653	645	645
query63	328	294	289	289
query64	10482	2210	1625	1625
query65	3142	3101	3136	3101
query66	1361	336	338	336
query67	15684	14884	14758	14758
query68	5522	543	534	534
query69	631	459	356	356
query70	1170	1216	1171	1171
query71	431	274	270	270
query72	7127	5779	6040	5779
query73	787	330	328	328
query74	6064	5579	5598	5579
query75	3614	2702	2676	2676
query76	3282	962	888	888
query77	673	311	302	302
query78	9581	9560	9508	9508
query79	3226	520	519	519
query80	2025	477	469	469
query81	578	226	221	221
query82	1429	139	143	139
query83	329	172	171	171
query84	279	91	92	91
query85	1578	310	351	310
query86	339	314	313	313
query87	3315	3156	3123	3123
query88	3931	2459	2464	2459
query89	479	387	387	387
query90	1614	191	193	191
query91	128	103	102	102
query92	57	48	49	48
query93	3739	501	507	501
query94	733	214	213	213
query95	403	321	321	321
query96	606	273	272	272
query97	3196	3039	3007	3007
query98	224	199	201	199
query99	1816	1290	1259	1259
Total cold run time: 286631 ms
Total hot run time: 174591 ms

@doris-robot
Copy link

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

query1	0.04	0.04	0.03
query2	0.08	0.04	0.04
query3	0.22	0.05	0.05
query4	1.66	0.08	0.08
query5	0.51	0.47	0.49
query6	1.14	0.72	0.72
query7	0.02	0.01	0.02
query8	0.05	0.04	0.04
query9	0.56	0.49	0.50
query10	0.55	0.54	0.54
query11	0.14	0.12	0.11
query12	0.15	0.12	0.12
query13	0.58	0.58	0.58
query14	0.77	0.80	0.80
query15	0.84	0.80	0.81
query16	0.35	0.35	0.36
query17	0.96	1.03	0.94
query18	0.22	0.22	0.23
query19	1.73	1.78	1.73
query20	0.01	0.01	0.01
query21	15.38	0.79	0.66
query22	4.53	7.09	1.76
query23	18.29	1.43	1.27
query24	2.10	0.23	0.24
query25	0.15	0.08	0.08
query26	0.30	0.21	0.20
query27	0.46	0.23	0.23
query28	13.25	1.02	0.99
query29	12.65	3.31	3.28
query30	0.26	0.06	0.06
query31	2.88	0.39	0.39
query32	3.26	0.47	0.46
query33	2.85	2.88	2.95
query34	16.89	4.33	4.33
query35	4.42	4.41	4.39
query36	0.64	0.46	0.47
query37	0.19	0.16	0.15
query38	0.15	0.15	0.16
query39	0.04	0.03	0.04
query40	0.16	0.12	0.12
query41	0.09	0.06	0.05
query42	0.06	0.05	0.05
query43	0.04	0.04	0.04
Total cold run time: 109.62 s
Total hot run time: 30.39 s

yiguolei
yiguolei approved these changes Jul 12, 2024
View reviewed changes
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

@dataroaring dataroaring merged commit a8d060a into apache:master Jul 12, 2024
25 of 29 checks passed
seawinde pushed a commit to seawinde/doris that referenced this pull request Jul 17, 2024
@TangSiyang2001 @seawinde
[fix](compile) replace shared_ptr::unique() with std::shared_ptr::use…
314c94d 
…_count() == 1 (apache#37688)

## Proposed changes

`std::shared_ptr::unique` is supported until cpp20. It was disabled by
default since llvm-18.
[see](https://releases.llvm.org/18.1.0/projects/libcxx/docs/ReleaseNotes/18.html#id4).

Therefore, replace it with std::shared_ptr::use_count() == 1 for better
compability. (Or we should work with
_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE which is ungraceful.)
dataroaring pushed a commit that referenced this pull request Jul 17, 2024
@TangSiyang2001 @dataroaring
[fix](compile) replace shared_ptr::unique() with std::shared_ptr::use…
3939804 
…_count() == 1 (#37688)

## Proposed changes

`std::shared_ptr::unique` is supported until cpp20. It was disabled by
default since llvm-18.
[see](https://releases.llvm.org/18.1.0/projects/libcxx/docs/ReleaseNotes/18.html#id4).

Therefore, replace it with std::shared_ptr::use_count() == 1 for better
compability. (Or we should work with
_LIBCPP_ENABLE_CXX20_REMOVED_SHARED_PTR_UNIQUE which is ungraceful.)
@gavinchou gavinchou mentioned this pull request Aug 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gavinchou gavinchou gavinchou 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/3.0.1-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@TangSiyang2001 @doris-robot @gavinchou @yiguolei @dataroaring