-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[libc++] Introduce __product_iterator_traits
and optimise flat_map::insert
#139454
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
Open
huixie90
wants to merge
4
commits into
llvm:main
Choose a base branch
from
huixie90:hxie/product_iterator
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
@llvm/pr-subscribers-libcxx Author: Hui (huixie90) ChangesFull diff: https://github.com/llvm/llvm-project/pull/139454.diff 7 Files Affected:
diff --git a/libcxx/include/CMakeLists.txt b/libcxx/include/CMakeLists.txt
index 8cb05857ea8d7..c03dd10ec1159 100644
--- a/libcxx/include/CMakeLists.txt
+++ b/libcxx/include/CMakeLists.txt
@@ -497,6 +497,7 @@ set(files
__iterator/ostreambuf_iterator.h
__iterator/permutable.h
__iterator/prev.h
+ __iterator/product_iterator.h
__iterator/projected.h
__iterator/ranges_iterator_traits.h
__iterator/readable_traits.h
diff --git a/libcxx/include/__flat_map/key_value_iterator.h b/libcxx/include/__flat_map/key_value_iterator.h
index 3ebb653deb197..968d354de8350 100644
--- a/libcxx/include/__flat_map/key_value_iterator.h
+++ b/libcxx/include/__flat_map/key_value_iterator.h
@@ -14,6 +14,7 @@
#include <__concepts/convertible_to.h>
#include <__config>
#include <__iterator/iterator_traits.h>
+#include <__iterator/product_iterator.h>
#include <__memory/addressof.h>
#include <__type_traits/conditional.h>
#include <__utility/move.h>
@@ -57,6 +58,8 @@ struct __key_value_iterator {
template <class, class, class, bool>
friend struct __key_value_iterator;
+ friend struct __product_iterator_traits<__key_value_iterator>;
+
public:
using iterator_concept = random_access_iterator_tag;
// `__key_value_iterator` only satisfy "Cpp17InputIterator" named requirements, because
@@ -167,6 +170,23 @@ struct __key_value_iterator {
}
};
+template <class _Owner, class _KeyContainer, class _MappedContainer, bool _Const>
+struct __product_iterator_traits<__key_value_iterator<_Owner, _KeyContainer, _MappedContainer, _Const>> {
+ static constexpr size_t __size = 2;
+
+ template <size_t _N>
+ _LIBCPP_HIDE_FROM_ABI static auto
+ __get_iterator_element(__key_value_iterator<_Owner, _KeyContainer, _MappedContainer, _Const> __it)
+ requires(_N == 0 || _N == 1)
+ {
+ if constexpr (_N == 0) {
+ return __it.__key_iter_;
+ } else {
+ return __it.__mapped_iter_;
+ }
+ }
+};
+
_LIBCPP_END_NAMESPACE_STD
#endif // _LIBCPP_STD_VER >= 23
diff --git a/libcxx/include/__flat_map/utils.h b/libcxx/include/__flat_map/utils.h
index acb7dca7ffe96..2c0e6ada31a5e 100644
--- a/libcxx/include/__flat_map/utils.h
+++ b/libcxx/include/__flat_map/utils.h
@@ -11,6 +11,7 @@
#define _LIBCPP___FLAT_MAP_UTILS_H
#include <__config>
+#include <__iterator/product_iterator.h>
#include <__type_traits/container_traits.h>
#include <__utility/exception_guard.h>
#include <__utility/forward.h>
@@ -93,6 +94,25 @@ struct __flat_map_utils {
}
return __num_appended;
}
+
+ template <class _Map, class _InputIterator>
+ _LIBCPP_HIDE_FROM_ABI static typename _Map::size_type
+ __append(_Map& __map, _InputIterator __first, _InputIterator __last)
+ requires __is_product_iterator_of_size<_InputIterator, 2>::value
+ {
+ auto __s1 = __map.__containers_.keys.size();
+ __map.__containers_.keys.insert(
+ __map.__containers_.keys.end(),
+ __product_iterator_traits<_InputIterator>::template __get_iterator_element<0>(__first),
+ __product_iterator_traits<_InputIterator>::template __get_iterator_element<0>(__last));
+
+ __map.__containers_.values.insert(
+ __map.__containers_.values.end(),
+ __product_iterator_traits<_InputIterator>::template __get_iterator_element<1>(__first),
+ __product_iterator_traits<_InputIterator>::template __get_iterator_element<1>(__last));
+
+ return __map.__containers_.keys.size() - __s1;
+ }
};
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/include/__iterator/product_iterator.h b/libcxx/include/__iterator/product_iterator.h
new file mode 100644
index 0000000000000..70b7acce456fb
--- /dev/null
+++ b/libcxx/include/__iterator/product_iterator.h
@@ -0,0 +1,68 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___PRODUCT_ITERATOR_H
+#define _LIBCPP___PRODUCT_ITERATOR_H
+
+// Product iterators are iterators that contain two or more underlying iterators.
+//
+// For example, std::flat_map stores its data into two separate containers, and its iterator
+// is a proxy over two separate underlying iterators. The concept of product iterators
+// allows algorithms to operate over these underlying iterators separately, opening the
+// door to various optimizations.
+//
+// If __product_iterator_traits can be instantiated, the following functions and associated types must be provided:
+// - static constexpr size_t Traits::__size
+// The number of underlying iterators inside the product iterator.
+//
+// - template <size_t _N>
+// static auto Traits::__get_iterator_element(It __it)
+// Returns the _Nth iterator element of the given product iterator.
+
+#include <__config>
+#include <__cstddef/size_t.h>
+#include <__type_traits/enable_if.h>
+#include <__type_traits/integral_constant.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+# pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+template <class _Iterator>
+struct __product_iterator_traits;
+/* exposition-only:
+{
+ static constexpr size_t __size = ...;
+
+ template <size_t _N>
+ static auto __get_iterator_element(_Iterator);
+};
+*/
+
+template <class _Tp, size_t = 0>
+struct __is_product_iterator : false_type {};
+
+template <class _Tp>
+struct __is_product_iterator<_Tp, sizeof(__product_iterator_traits<_Tp>) * 0> : true_type {};
+
+template <class _Tp, size_t _Size, class = void>
+struct __is_product_iterator_of_size : false_type {};
+
+template <class _Tp, size_t _Size>
+struct __is_product_iterator_of_size<_Tp, _Size, __enable_if_t<__product_iterator_traits<_Tp>::__size == _Size>>
+ : true_type {};
+
+template <class _Iterator, size_t _N>
+using __product_iterator_element_t =
+ decltype(__product_iterator_traits<_Iterator>::__get_iterator_element<_N>(declval<_Iterator>()));
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___PRODUCT_ITERATOR_H
diff --git a/libcxx/include/__ranges/zip_view.h b/libcxx/include/__ranges/zip_view.h
index 6d31f5ce0993c..b074e69832535 100644
--- a/libcxx/include/__ranges/zip_view.h
+++ b/libcxx/include/__ranges/zip_view.h
@@ -23,6 +23,7 @@
#include <__iterator/iter_move.h>
#include <__iterator/iter_swap.h>
#include <__iterator/iterator_traits.h>
+#include <__iterator/product_iterator.h>
#include <__ranges/access.h>
#include <__ranges/all.h>
#include <__ranges/concepts.h>
@@ -251,6 +252,10 @@ class zip_view<_Views...>::__iterator : public __zip_view_iterator_category_base
friend class zip_view<_Views...>;
+ using __is_zip_view_iterator _LIBCPP_NODEBUG = true_type;
+
+ friend struct __product_iterator_traits<__iterator>;
+
public:
using iterator_concept = decltype(ranges::__get_zip_view_iterator_tag<_Const, _Views...>());
using value_type = tuple<range_value_t<__maybe_const<_Const, _Views>>...>;
@@ -468,6 +473,18 @@ inline constexpr auto zip = __zip::__fn{};
} // namespace views
} // namespace ranges
+template <class _Iter>
+ requires _Iter::__is_zip_view_iterator::value
+struct __product_iterator_traits<_Iter> {
+ static constexpr size_t __size = tuple_size<decltype(std::declval<_Iter>().__current_)>::value;
+
+ template <size_t _N>
+ requires(_N < __size)
+ _LIBCPP_HIDE_FROM_ABI static constexpr auto __get_iterator_element(_Iter __it) {
+ return std::get<_N>(__it.__current_);
+ }
+};
+
#endif // _LIBCPP_STD_VER >= 23
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/benchmarks/containers/associative/flat_map.bench.cpp b/libcxx/test/benchmarks/containers/associative/flat_map.bench.cpp
index 82902d50f31e6..7351e79758fa8 100644
--- a/libcxx/test/benchmarks/containers/associative/flat_map.bench.cpp
+++ b/libcxx/test/benchmarks/containers/associative/flat_map.bench.cpp
@@ -10,6 +10,7 @@
#include <flat_map>
#include <utility>
+#include <ranges>
#include "associative_container_benchmarks.h"
#include "../../GenerateInput.h"
@@ -26,9 +27,54 @@ struct support::adapt_operations<std::flat_map<K, V>> {
static auto get_iterator(InsertionResult const& result) { return result.first; }
};
+void product_iterator_benchmark_flat_map(benchmark::State& state) {
+ const std::size_t size = state.range(0);
+
+ using M = std::flat_map<int, int>;
+
+ const M source =
+ std::views::iota(0, static_cast<int>(size)) | std::views::transform([](int i) { return std::pair(i, i); }) |
+ std::ranges::to<std::flat_map<int, int>>();
+
+ for (auto _ : state) {
+ M m;
+ m.insert(std::sorted_unique, source.begin(), source.end());
+ benchmark::DoNotOptimize(m);
+ benchmark::ClobberMemory();
+ }
+}
+
+void product_iterator_benchmark_zip_view(benchmark::State& state) {
+ const std::size_t size = state.range(0);
+
+ using M = std::flat_map<int, int>;
+
+ const std::vector<int> keys = std::views::iota(0, static_cast<int>(size)) | std::ranges::to<std::vector<int>>();
+ const std::vector<int> values = keys;
+
+ auto source = std::views::zip(keys, values);
+ for (auto _ : state) {
+ M m;
+ m.insert(std::sorted_unique, source.begin(), source.end());
+ benchmark::DoNotOptimize(m);
+ benchmark::ClobberMemory();
+ }
+}
+
int main(int argc, char** argv) {
support::associative_container_benchmarks<std::flat_map<int, int>>("std::flat_map<int, int>");
+ benchmark::RegisterBenchmark("flat_map::insert_product_iterator_flat_map", product_iterator_benchmark_flat_map)
+ ->Arg(32)
+ ->Arg(1024)
+ ->Arg(8192)
+ ->Arg(65536);
+ benchmark::RegisterBenchmark("flat_map::insert_product_iterator_zip", product_iterator_benchmark_zip_view)
+ ->Arg(32)
+ ->Arg(1024)
+ ->Arg(8192)
+ ->Arg(65536);
+
benchmark::Initialize(&argc, argv);
benchmark::RunSpecifiedBenchmarks();
benchmark::Shutdown();
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
index 8455b19475fe4..8bbb7caa82545 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.modifiers/insert_iter_iter.pass.cpp
@@ -17,6 +17,7 @@
#include <cassert>
#include <functional>
#include <deque>
+#include <ranges>
#include "MinSequenceContainer.h"
#include "../helpers.h"
@@ -75,12 +76,33 @@ void test() {
M expected2{{0, 1}, {1, 1}, {2, 1}, {3, 1}, {4, 1}};
assert(m == expected2);
}
+
+void test_product_iterator() {
+ using M = std::flat_map<int, int>;
+ {
+ M m1{{1, 1}, {2, 1}, {3, 1}};
+ M m2{{4, 1}, {5, 1}, {6, 1}};
+ m1.insert(m2.begin(), m2.end());
+ M expected{{1, 1}, {2, 1}, {3, 1}, {4, 1}, {5, 1}, {6, 1}};
+ assert(m1 == expected);
+ }
+ {
+ std::vector<int> keys{1, 2, 3};
+ std::vector<int> values{1, 1, 1};
+ auto zv = std::views::zip(keys, values);
+ M m;
+ m.insert(zv.begin(), zv.end());
+ M expected{{1, 1}, {2, 1}, {3, 1}};
+ assert(m == expected);
+ }
+}
+
int main(int, char**) {
test<std::vector<int>, std::vector<double>>();
test<std::deque<int>, std::vector<double>>();
test<MinSequenceContainer<int>, MinSequenceContainer<double>>();
test<std::vector<int, min_allocator<int>>, std::vector<double, min_allocator<double>>>();
-
+ test_product_iterator();
{
auto insert_func = [](auto& m, const auto& newValues) { m.insert(newValues.begin(), newValues.end()); };
test_insert_range_exception_guarantee(insert_func);
|
__product_iterator_traits
and optimise flat_map::insert
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #108624
This allows
flat_map::insert(Iter, Iter)
to directly forward to underlying containers'insert(Iter, Iter)
, instead of inserting one element at a time, when input models "product iterator". atm,flat_map::iterator
andzip_view::iterator
are "product iterator"s.This gives about 2x speed up in my benchmark