-
Notifications
You must be signed in to change notification settings - Fork 15k
[libc++] Optimize ranges::{for_each, for_each_n} for segmented iterators #132896
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
Conversation
49011aa to
ba1d5d4
Compare
|
@llvm/pr-subscribers-libcxx Author: Peng Liu (winner245) ChangesThis patch extends segmented iterator optimizations, previously applied to Addresses a subtask of #102817.
|
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/libcxx/algorithms/ranges_robust_against_copying_comparators.pass.cpp
Outdated
Show resolved
Hide resolved
ba1d5d4 to
c113266
Compare
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
a7041cc to
a2e451d
Compare
16438be to
047acfd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the patch! I left some comments but I think this is going to be a nice optimization.
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
0aad396 to
5a7b6eb
Compare
libcxx/test/benchmarks/algorithms/nonmodifying/for_each_join_view.bench.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/benchmarks/algorithms/nonmodifying/for_each_n.bench.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/std/algorithms/alg.nonmodifying/alg.foreach/for_each_n.pass.cpp
Outdated
Show resolved
Hide resolved
198fe3b to
f5d13ab
Compare
d14bde4 to
8a5bcdc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like the scope of this patch is getting a bit out of hand. The title says that you're optimizing ranges::for_each{,_n}, but you're also back-porting the std::for_each optimization to C++03, adding and adding an optimization to std::for_each_n. Could we split this up to make it clear what changes are required for what optimizations? Also, why do we want to back-port the std::for_each optimization now? Do we think the extra complexity is worth the improved performance?
Thank you for your feedback! I agree that the scope of the patch has expanded beyond its original intent. Initially, the goal was simple: only to extend the optimization for However, I agree that this made the patch diverge from its original purpose and may complicate the review process. Following your suggestion, I will work on splitting it to make it clear what this patch focuses on. -------------- Update --------------
This separation allows the current PR to focus exclusively on the optimization of the ranges algorithms. I will rebase my current patch on the above split pieces once they are landed. |
8a5bcdc to
5a225dd
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
5a225dd to
b366e93
Compare
b366e93 to
216b957
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once comments are addressed. Thanks a lot for this series of refactorings / optimizations!
libcxx/test/benchmarks/algorithms/nonmodifying/for_each.bench.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/benchmarks/algorithms/nonmodifying/for_each.bench.cpp
Outdated
Show resolved
Hide resolved
libcxx/test/benchmarks/algorithms/nonmodifying/for_each_n.bench.cpp
Outdated
Show resolved
Hide resolved
275c254 to
df7ac69
Compare
df7ac69 to
b525b74
Compare
b525b74 to
18bf207
Compare
Previously, the segmented iterator optimization was limited to
std::{for_each, for_each_n}. This patch aims to extend the optimization tostd::ranges::for_eachandstd::ranges::for_each_n, ensuring consistent optimizations across these algorithms. This patch first generalizes thestdalgorithms by introducing aProjectionparameter, which is set to__identityfor thestdalgorithms. Then we let therangesalgorithms to directly call theirstdcounterparts with a general__projargument. Benchmarks demonstrate performance improvements of up to 21.4x forstd::deque::iteratorand 22.3x forjoin_viewofvector<vector<char>>.Addresses a subtask of #102817.
Summary of speedups for
dequeiteratorsSummary of speedups for
join_viewiteratorsBenchmarks:
std::ranges::for_eachwithdequeiteratorsstd::ranges::for_each_nwithdequeiteratorsstd::ranges::for_eachwithjoin_viewiteratorsstd::ranges::for_each_nwithjoin_viewiterators