-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libcxx] Optimize std::generate for segmented iterators #163006
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
base: main
Are you sure you want to change the base?
Conversation
@llvm/pr-subscribers-libcxx Author: Connector Switch (c8ef) ChangesPart of #102817. Full diff: https://github.com/llvm/llvm-project/pull/163006.diff 2 Files Affected:
diff --git a/libcxx/include/__algorithm/generate.h b/libcxx/include/__algorithm/generate.h
index c95b527402f5d..91e2ada7daf77 100644
--- a/libcxx/include/__algorithm/generate.h
+++ b/libcxx/include/__algorithm/generate.h
@@ -9,7 +9,10 @@
#ifndef _LIBCPP___ALGORITHM_GENERATE_H
#define _LIBCPP___ALGORITHM_GENERATE_H
+#include <__algorithm/for_each_segment.h>
#include <__config>
+#include <__iterator/segmented_iterator.h>
+#include <__type_traits/enable_if.h>
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
# pragma GCC system_header
@@ -17,13 +20,33 @@
_LIBCPP_BEGIN_NAMESPACE_STD
-template <class _ForwardIterator, class _Generator>
+template <class _ForwardIterator, class _Sent, class _Generator>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
-generate(_ForwardIterator __first, _ForwardIterator __last, _Generator __gen) {
+__generate(_ForwardIterator __first, _Sent __last, _Generator __gen) {
for (; __first != __last; ++__first)
*__first = __gen();
}
+#ifndef _LIBCPP_CXX03_LANG
+template <class _SegmentedIterator,
+ class _Generator,
+ __enable_if_t<__is_segmented_iterator_v<_SegmentedIterator>, int> = 0>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20
+_SegmentedIterator __generate(_SegmentedIterator __first, _SegmentedIterator __last, _Generator& __gen) {
+ using __local_iterator_t = typename __segmented_iterator_traits<_SegmentedIterator>::__local_iterator;
+ std::__for_each_segment(__first, __last, [&](__local_iterator_t __lfirst, __local_iterator_t __llast) {
+ std::__generate(__lfirst, __llast, __gen);
+ });
+ return __last;
+}
+#endif // !_LIBCPP_CXX03_LANG
+
+template <class _ForwardIterator, class _Generator>
+inline _LIBCPP_HIDE_FROM_ABI
+_LIBCPP_CONSTEXPR_SINCE_CXX20 void generate(_ForwardIterator __first, _ForwardIterator __last, _Generator __gen) {
+ std::__generate(__first, __last, __gen);
+}
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP___ALGORITHM_GENERATE_H
diff --git a/libcxx/test/std/algorithms/alg.modifying.operations/alg.generate/generate.pass.cpp b/libcxx/test/std/algorithms/alg.modifying.operations/alg.generate/generate.pass.cpp
index 29d32d7156742..4591d7ece4645 100644
--- a/libcxx/test/std/algorithms/alg.modifying.operations/alg.generate/generate.pass.cpp
+++ b/libcxx/test/std/algorithms/alg.modifying.operations/alg.generate/generate.pass.cpp
@@ -16,6 +16,7 @@
#include <algorithm>
#include <cassert>
+#include <deque>
#include "test_macros.h"
#include "test_iterators.h"
@@ -51,12 +52,22 @@ test()
assert(ia[3] == 1);
}
+void deque_test() {
+ int sizes[] = {0, 1, 2, 1023, 1024, 1025, 2047, 2048, 2049};
+ for (const int size : sizes) {
+ std::deque<int> d(size);
+ std::generate(d.begin(), d.end(), gen_test());
+ assert(std::all_of(d.begin(), d.end(), [](int x) { return x == 1; }));
+ }
+}
+
int main(int, char**)
{
test<forward_iterator<int*> >();
test<bidirectional_iterator<int*> >();
test<random_access_iterator<int*> >();
test<int*>();
+ deque_test();
#if TEST_STD_VER > 17
static_assert(test_constexpr());
|
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.
Can we instead just forward to std::for_each
?
You mean like following? template<class ForwardIt, class Generator>
void generate(ForwardIt first, ForwardIt last, Generator gen) {
std::for_each(first, last, [&gen](auto& element) {
element = gen();
});
} Will test this tonight. |
To some extent, I think the current implementation is also acceptable since it uses the for_each_segment utility. |
Forwarding this to template <class _ForwardIterator, class _Generator>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void
generate(_ForwardIterator __first, _ForwardIterator __last, _Generator __gen) {
std::for_each(__first, __last, [&](auto& __element) { __element = __gen(); });
} |
Have you enabled optimizations? |
It seems that the default |
This looks weird...
|
While both implementations offer the same performance, the for_each forward approach cannot properly build the benchmark, failing specifically on regex testing. It's odd because locally, using @philnik777 Could you please help take a look? |
This usually indicates that your code doesn't work in C++14 mode. You should try to run lit with e.g. |
I still haven't figured out why the regex check failed, but one of the test error message is above. I suspect it relates to std::__invoke. Is it possible to revert to the original version without forwarding to std::for_each? |
It's really weird that the regex check still isn't working. Even worse, it isn't producing a CMakeError log that I can use to reproduce the failed compilation. I ran the full test suite and found that the failures are only related to fs(symlink), iostream, and locale, which seems to have little relevance to std::generator.
|
I also checked the GitHub Actions CMakeError log for the REGEX, but it didn't return any hit. |
The final issue is that auto&& is unavailable in C++ standards prior to C++14. : ( |
Since the CI is almost passing now, I'm wondering:
Looking forward to your advice! @philnik777 @frederick-vs-ja |
|
Get a clang ICE on c++11 mode: https://godbolt.org/z/P4j6r5cra 😆 |
I think I prefer the functor approach to make sure it works. |
The windows arm mingw CI failure seems unrelated. |
libcxx/test/std/algorithms/alg.modifying.operations/alg.generate/generate.pass.cpp
Show resolved
Hide resolved
Co-authored-by: A. Jiang <de34@live.cn>
Part of #102817.
This patch attempts to optimize the performance of
std::generate
for segmented iterators. Below are the benchmark numbers fromlibcxx\test\benchmarks\algorithms\modifying\generate.bench.cpp
. Test cases that use segmented iterators have also been added.