-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libc++] Optimize multi{map,set}::insert(InputIterator, InputIterator) #152691
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
[libc++] Optimize multi{map,set}::insert(InputIterator, InputIterator) #152691
Conversation
✅ With the latest revision this PR passed the C/C++ code formatter. |
2f03769
to
8bed2d3
Compare
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) Changes
Full diff: https://github.com/llvm/llvm-project/pull/152691.diff 3 Files Affected:
diff --git a/libcxx/include/__tree b/libcxx/include/__tree
index 6dadd0915c984..5baadca719ac4 100644
--- a/libcxx/include/__tree
+++ b/libcxx/include/__tree
@@ -970,6 +970,36 @@ public:
__emplace_hint_multi(__p, std::move(__value));
}
+ template <class _InIter, class _Sent>
+ _LIBCPP_HIDE_FROM_ABI void __insert_range_multi(_InIter __first, _Sent __last) {
+ if (__first == __last)
+ return;
+
+ if (__root() == nullptr) { // Make sure we always have a root node
+ __node_holder __node = __construct_node(*__first);
+ __insert_node_at(__end_node(), __end_node()->__left_, static_cast<__node_base_pointer>(__node.release()));
+ ++__first;
+ }
+
+ auto __max_node = static_cast<__node_pointer>(std::__tree_max(static_cast<__node_base_pointer>(__root())));
+ __node_pointer __last_insertion = __root();
+
+ for (; __first != __last; ++__first) {
+ __node_holder __node = __construct_node(*__first);
+ if (!value_comp()(__node->__get_value(), __max_node->__get_value())) { // __node >= __max_val
+ __insert_node_at(static_cast<__end_node_pointer>(__max_node),
+ __max_node->__right_,
+ static_cast<__node_base_pointer>(__node.get()));
+ __max_node = __node.release();
+ } else {
+ __end_node_pointer __parent;
+ __node_base_pointer& __child = __find_leaf(++iterator(__last_insertion), __parent, __node->__value_);
+ __last_insertion = __node.get();
+ __insert_node_at(__parent, __child, static_cast<__node_base_pointer>(__node.release()));
+ }
+ }
+ }
+
_LIBCPP_HIDE_FROM_ABI pair<iterator, bool> __node_assign_unique(const value_type& __v, __node_pointer __dest);
_LIBCPP_HIDE_FROM_ABI iterator __node_insert_multi(__node_pointer __nd);
diff --git a/libcxx/include/map b/libcxx/include/map
index 9f98abef9afe0..a84b5592244fa 100644
--- a/libcxx/include/map
+++ b/libcxx/include/map
@@ -593,6 +593,7 @@ erase_if(multimap<Key, T, Compare, Allocator>& c, Predicate pred); // C++20
# include <__memory/unique_ptr.h>
# include <__memory_resource/polymorphic_allocator.h>
# include <__node_handle>
+# include <__ranges/access.h>
# include <__ranges/concepts.h>
# include <__ranges/container_compatible_range.h>
# include <__ranges/from_range.h>
diff --git a/libcxx/include/set b/libcxx/include/set
index c77345bc5dc1f..18488b56e7dd2 100644
--- a/libcxx/include/set
+++ b/libcxx/include/set
@@ -530,6 +530,7 @@ erase_if(multiset<Key, Compare, Allocator>& c, Predicate pred); // C++20
# include <__memory/allocator_traits.h>
# include <__memory_resource/polymorphic_allocator.h>
# include <__node_handle>
+# include <__ranges/access.h>
# include <__ranges/concepts.h>
# include <__ranges/container_compatible_range.h>
# include <__ranges/from_range.h>
@@ -1205,18 +1206,14 @@ public:
}
template <class _InputIterator>
- _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __f, _InputIterator __l) {
- for (const_iterator __e = cend(); __f != __l; ++__f)
- __tree_.__emplace_hint_multi(__e, *__f);
+ _LIBCPP_HIDE_FROM_ABI void insert(_InputIterator __first, _InputIterator __last) {
+ __tree_.__insert_range_multi(__first, __last);
}
# if _LIBCPP_STD_VER >= 23
template <_ContainerCompatibleRange<value_type> _Range>
_LIBCPP_HIDE_FROM_ABI void insert_range(_Range&& __range) {
- const_iterator __end = cend();
- for (auto&& __element : __range) {
- __tree_.__emplace_hint_multi(__end, std::forward<decltype(__element)>(__element));
- }
+ __tree_.__insert_range_multi(ranges::begin(__range), ranges::end(__range));
}
# endif
|
0e5d4f1
to
691222a
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
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.
It seems like we forgot to look at the existing tests. The current coverage is very basic. Let's add at least:
- Inserting an empty range
- Inserting a range with 1 element (existing AND non-existing)
- Inserting a range with no existing elements
- Inserting a range with existing elements
- Inserting a range with sorted and non-sorted elements
Requesting changes until we have that.
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.
More test cases:
- Inserting into an empty / non-empty / 1-element multimap
21c073a
to
dd319b1
Compare
dd319b1
to
e890a85
Compare
Uh oh!
There was an error while loading. Please reload this page.