[libcxx-commits] [libcxx] Optimize std::__tree::__assign_multi to insert the provided range at the end of the tree every time (PR #131030)

[Nikolas Klauser via libcxx-commits]

https://github.com/philnik777 approved this pull request.

> > Please do not merge libc++ patches without approval from @llvm/reviewers-libcxx, as that is a general requirement for libc++ patches to be landed.
> 
> Huh, I didn't realize. Is this documented anywhere? I was just going by the official [LLVM Developer Policy](https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access), which appeared to specifically give permission to do this:
> 
> > _2. You are allowed to commit patches without approval which you think are obvious.

I'm not sure it's documented anywhere. We should certainly do that if it's not.

> > Given that you haven't provided any benchmarks for non-optimal cases that's not exactly surprising. How does the performance look if you e.g. assign a range that contains random elements or is sorted inversely?
> 
> That sounds impossible -- am I perhaps not understanding what you mean? This is in `__assign_multi`. So far as I'm aware, it is only used in `__tree::operator=`, like this:
> 
> ```
> template <class _Tp, class _Compare, class _Allocator>
> __tree<_Tp, _Compare, _Allocator>& __tree<_Tp, _Compare, _Allocator>::operator=(const __tree& __t) {
>   if (this != std::addressof(__t)) {
>     value_comp() = __t.value_comp();
>     __copy_assign_alloc(__t);
>     __assign_multi(__t.begin(), __t.end());
>   }
>   return *this;
> }
> ```
> 
> The comparators are identical and the target of the assignment is empty at the type of the assignment. How could the inputs have any ordering other than the correctly-sorted one?

Never mind. I was absolutely sure that it's used in `map(InputIterator, InputIterator)`, but it's not. That does indeed make it impossible.

> > Edit: I've raised this in my previous comment already, so there has been review feedback that hasn't been addressed.
> 
> I'm not seeing the feedback you're referring to unfortunately. The only previous feedback I see on my side is the objection that the performance improvement was small (not negative), and requires large numbers (both of which I responded to), as well as the one asking me to run your existing benchmarks -- which you yourself did, and said you didn't see much difference. I'm not seeing anything about benchmarking with random elements, inversion, or other feedback that I didn't address. Did you perhaps type more comments and forget to publish them?

I'm talking about

> so the hint could be completely bogus when you assign from something other than a container of the same type

that might not have been obvious though.

https://github.com/llvm/llvm-project/pull/131030