electronicarts / EASTL

EASTL stands for Electronic Arts Standard Template Library. It is an extensive and robust implementation that has an emphasis on high performance.
BSD 3-Clause "New" or "Revised" License
8.09k stars 929 forks source link

Reduce complexity when inputer iterator meet RandomAccessIterator #515

Closed ChaiByte closed 1 year ago

ChaiByte commented 1 year ago

As described in https://github.com/electronicarts/EASTL/pull/514#discussion_r1255556282 , It's a X-Y problem when discussing the correct method fixing ring buffer comparation operator. Another issue to become evident.

Current eastl::identical is not effient enough when input iterators meet LegacyRandomAccessIterator requirements.

Refs: https://en.cppreference.com/w/cpp/algorithm/equal

Complexity 5,7) At most min(last1 - first1, last2 - first2) applications of the predicate. However, if InputIt1 and InputIt2 meet the requirements of [LegacyRandomAccessIterator]> (https://en.cppreference.com/w/cpp/named_req/RandomAccessIterator) and last1 - first1 != last2 - first2 then no applications of the predicate are made (size mismatch is detected without looking at any elements). 2,4,6,8) same, but the complexity is specified as O(x), rather than "at most x".

By the way, I want to know why did EASTL choose to implement the equal and identical interfaces separately at the beginning of the design, is there any reference to explain the reason?

grojo-ea commented 1 year ago

I agree that there's room for optimizations here in the case of random access iterators, but this implementation is not the right one for iterators which are not random access.

This optimization needs to branch behaviour when the iterator is random access vs when it isn't. EASTL doesn't yet require C++17 to compile so at this point this should probably be done with tagged dispatch (similar to what we do with eastl::distance) or with a "normal-not-constexpr" if on a compile time constant check, as opposed to using if constexpr, which would be the way to do this in C++17.

ChaiByte commented 1 year ago

I'll close this PR temporarily until we find a better solution.