Skip to content

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

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −13
Open

[libc++] Optimize multi{map,set}::insert(InputIterator, InputIterator) #152691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions libcxx/docs/ReleaseNotes/22.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ Improvements and New Features

- The performance of ``map::map(const map&)`` has been improved up to 2.3x
- The performance of ``map::operator=(const map&)`` has been improved by up to 11x
- The performance of the ``(iterator, iterator)`` constructors of ``multimap`` and ``multiset``
has been improved by up to 3x
- The performance of ``insert(iterator, iterator)`` of ``multimap`` and ``multiset`` has been improved by up to 2.5x

Deprecations and Removals
-------------------------
Expand Down
29 changes: 29 additions & 0 deletions libcxx/include/__tree
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -970,6 +970,35 @@ 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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document what this function does and in particular what optimizations it performs (i.e. it is optimized for already-sorted inputs).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What sort of documentation do you want? I'm not sure there is much to say beyond "inserts the range [first, last) into *this", which I think is rather obvious.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also suggest taking a tag here and handling the (very similar) unique/multi cases uniformly with a if constexpr. Then we can report improvements for map and set as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It turns out that adding a unique implementation will be a lot more complex, since we have a lot more complex handling in __emplace_unique to avoid unnecessary allocations if the node already exists. I think it would be better to refactor __emplace_unique to make that simpler first and then tackle __insert_range_unique (or a merged version) separately.

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())));

for (; __first != __last; ++__first) {
__node_holder __node = __construct_node(*__first);
// Always check the max node first. This optimizes for sorted ranges inserted at the end.
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_high(__parent, __node->__value_);
__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);
Expand Down
9 changes: 3 additions & 6 deletions libcxx/include/map
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -1749,17 +1750,13 @@ 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.__i_, *__f);
__tree_.__insert_range_multi(__f, __l);
}

# 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.__i_, std::forward<decltype(__element)>(__element));
}
__tree_.__insert_range_multi(ranges::begin(__range), ranges::end(__range));
}
# endif

Expand Down
11 changes: 4 additions & 7 deletions libcxx/include/set
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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

Expand Down
Loading