perf(SeqManager): Use std::vector rather than std::set

[jmillan]

It increases performance greatly[*] thanks to the cache locality of the vector.

Scenarios where this is noticeable is when there are SeqManager drops due to, ie: Codec implementation or SimulcastConsumer dropping a packet.

[*]: Using mediasoup-demo with 1 audio|video producer and 100 audio|video consumers the CPU usage of SeqManager went down from >10% to <2%

[github-actions]

clang-tidy made some suggestions

[penguinol]

Not sure whether this works properly in all situations.
It seems std::binary_search and std::lower_bound have different requirement to compare function.
https://www.cppreference.com/cpp/algorithm/lower_bound

Unlike std::binary_search, std::lower_bound does not require operator< or comp to be asymmetric (i.e., a < b and b < a always have different results). In fact, it does not even require value < *iter or comp(value, *iter) to be well-formed for any iterator iter in [first, last).

https://en.cppreference.com/cpp/named_req/Compare

Maybe realted to #1370

[ibc]

What about if we used the new Utils::UnwrappedSequenceNumber class that I borrowed from libwebrtc/dcsctp?

• https://github.com/versatica/mediasoup/blob/v3/worker/include/Utils/UnwrappedSequenceNumber.hpp
• https://github.com/versatica/mediasoup/blob/v3/worker/test/src/Utils/TestUnwrappedSequenceNumber.cpp

[jmillan]

What about if we used the new Utils::UnwrappedSequenceNumber class that I borrowed from libwebrtc/dcsctp?

• https://github.com/versatica/mediasoup/blob/v3/worker/include/Utils/UnwrappedSequenceNumber.hpp
• https://github.com/versatica/mediasoup/blob/v3/worker/test/src/Utils/TestUnwrappedSequenceNumber.cpp

That is totally unrelated to this PR. Here I'm just using a more efficient container for the job after measuring the performance.

I believe there is an open issue with your suggestion which is another different topic than this one.

[jmillan]

Not sure whether this works properly in all situations.
It seems std::binary_search and std::lower_bound have different requirement to compare function.
https://www.cppreference.com/cpp/algorithm/lower_bound

Unlike std::binary_search, std::lower_bound does not require operator< or comp to be asymmetric (i.e., a < b and b < a always have different results). In fact, it does not even require value < *iter or comp(value, *iter) to be well-formed for any iterator iter in [first, last).

https://en.cppreference.com/cpp/named_req/Compare

Maybe realted to #1370

Thanks, I'll check this info.

[jmillan]

Unlike std::binary_search, std::lower_bound does not require operator< or comp to be asymmetric (i.e., a < b and b < a always have different results). In fact, it does not even require value < *iter or comp(value, *iter) to be well-formed for any iterator iter in [first, last).

This this no trouble. In our case if a<b b>a, so I don't see any problem. Besides, al tests pass. Do you see a specific issue that is not present in current v3?

[penguinol]

std::set uses a red-black tree while std::vector uses binary search, so they use different algorithms.
I am concerned about whether they behave the same with SeqLowerThan, which does not satisfy the C++ Compare standard.

[jmillan]

std::set uses a red-black tree while std::vector uses binary search, so they use different algorithms.
I am concerned about whether they behave the same with SeqLowerThan, which does not satisfy the C++ Compare standard.

We've plenty of tests, all passing: https://github.com/versatica/mediasoup/blob/v3/worker/test/src/RTC/TestSeqManager.cpp#L1

Both use the same comp. The difference is that binary_search requires it to be asymmetric (i.e., a < b and b < a always have different results), which is the case of SeqLowerThan anyways.

Any specific failing case/test on top of your mind?

[jmillan]

Ok, so IsLowerThan not strictly asymmetric: for some values, both IsLowerThan(a, b) and IsLowerThan(b, a) can be false (e.g., when a == b or when the difference is exactly half the range).

I'll avoid the binary_search.

[jmillan]

@penguinol please check the latest commit.