fix overflow in sliding_average in the case of very high download rates

This commit is contained in:
Arvid Norberg 2018-10-02 14:42:40 +02:00 committed by Arvid Norberg
parent 949867eca9
commit a4267d61e7
10 changed files with 40 additions and 27 deletions

View File

@ -119,7 +119,7 @@ private:
// have higher priority // have higher priority
vector<piece_index_t, int> m_priority_pieces; vector<piece_index_t, int> m_priority_pieces;
sliding_average<30> m_availability; sliding_average<int, 30> m_availability;
}; };
}} }}

View File

@ -47,7 +47,6 @@ POSSIBILITY OF SUCH DAMAGE.
#include "libtorrent/error_code.hpp" #include "libtorrent/error_code.hpp"
#include "libtorrent/io_service_fwd.hpp" #include "libtorrent/io_service_fwd.hpp"
#include "libtorrent/hasher.hpp" #include "libtorrent/hasher.hpp"
#include "libtorrent/sliding_average.hpp"
#include "libtorrent/tailqueue.hpp" #include "libtorrent/tailqueue.hpp"
#include "libtorrent/linked_list.hpp" #include "libtorrent/linked_list.hpp"
#include "libtorrent/disk_buffer_pool.hpp" #include "libtorrent/disk_buffer_pool.hpp"

View File

@ -39,7 +39,6 @@ POSSIBILITY OF SUCH DAMAGE.
#include "libtorrent/storage.hpp" #include "libtorrent/storage.hpp"
#include "libtorrent/allocator.hpp" #include "libtorrent/allocator.hpp"
#include "libtorrent/io_service.hpp" #include "libtorrent/io_service.hpp"
#include "libtorrent/sliding_average.hpp"
#include "libtorrent/disk_io_thread_pool.hpp" #include "libtorrent/disk_io_thread_pool.hpp"
#include "libtorrent/disk_io_job.hpp" #include "libtorrent/disk_io_job.hpp"
#include "libtorrent/disk_job_pool.hpp" #include "libtorrent/disk_job_pool.hpp"

View File

@ -842,7 +842,7 @@ namespace aux {
// outstanding request, the time since the piece was requested. It // outstanding request, the time since the piece was requested. It
// is essentially an estimate of the time it will take to completely // is essentially an estimate of the time it will take to completely
// receive a payload message after it has been requested. // receive a payload message after it has been requested.
sliding_average<20> m_request_time; sliding_average<int, 20> m_request_time;
// keep the io_service running as long as we // keep the io_service running as long as we
// have peer connections // have peer connections

View File

@ -36,6 +36,7 @@ POSSIBILITY OF SUCH DAMAGE.
#include "libtorrent/buffer.hpp" #include "libtorrent/buffer.hpp"
#include "libtorrent/disk_buffer_holder.hpp" #include "libtorrent/disk_buffer_holder.hpp"
#include "libtorrent/sliding_average.hpp" #include "libtorrent/sliding_average.hpp"
#include "libtorrent/aux_/numeric_cast.hpp"
#include <climits> #include <climits>
@ -57,8 +58,8 @@ struct TORRENT_EXTRA_EXPORT receive_buffer
bool packet_finished() const { return m_packet_size <= m_recv_pos; } bool packet_finished() const { return m_packet_size <= m_recv_pos; }
int pos() const { return m_recv_pos; } int pos() const { return m_recv_pos; }
int capacity() const { return int(m_recv_buffer.size()); } int capacity() const { return aux::numeric_cast<int>(m_recv_buffer.size()); }
int watermark() const { return m_watermark.mean(); } int watermark() const { return aux::numeric_cast<int>(m_watermark.mean()); }
span<char> reserve(int size); span<char> reserve(int size);
void grow(int limit); void grow(int limit);
@ -154,7 +155,7 @@ private:
// keep track of how much of the receive buffer we use, if we're not using // keep track of how much of the receive buffer we use, if we're not using
// enough of it we shrink it // enough of it we shrink it
sliding_average<20> m_watermark; sliding_average<std::int64_t, 20> m_watermark;
buffer m_recv_buffer; buffer m_recv_buffer;
}; };

View File

@ -35,26 +35,29 @@ POSSIBILITY OF SUCH DAMAGE.
#include <cstdint> #include <cstdint>
#include <cstdlib> // for std::abs #include <cstdlib> // for std::abs
#include <limits>
#include "libtorrent/assert.hpp"
namespace libtorrent { namespace libtorrent {
// an exponential moving average accumulator. Add samples to it and it keeps // an exponential moving average accumulator. Add samples to it and it keeps
// track of a moving mean value and an average deviation // track of a moving mean value and an average deviation
template <int inverted_gain> template <typename Int, Int inverted_gain>
struct sliding_average struct sliding_average
{ {
static_assert(std::is_integral<Int>::value, "template argument must be integral");
sliding_average(): m_mean(0), m_average_deviation(0), m_num_samples(0) {} sliding_average(): m_mean(0), m_average_deviation(0), m_num_samples(0) {}
sliding_average(sliding_average const&) = default; sliding_average(sliding_average const&) = default;
sliding_average& operator=(sliding_average const&) = default; sliding_average& operator=(sliding_average const&) = default;
void add_sample(int s) void add_sample(Int s)
{ {
TORRENT_ASSERT(s < std::numeric_limits<Int>::max() / 64);
// fixed point // fixed point
s *= 64; s *= 64;
int deviation = 0; Int const deviation = (m_num_samples > 0) ? std::abs(m_mean - s) : 0;
if (m_num_samples > 0)
deviation = std::abs(m_mean - s);
if (m_num_samples < inverted_gain) if (m_num_samples < inverted_gain)
++m_num_samples; ++m_num_samples;
@ -70,14 +73,14 @@ struct sliding_average
} }
} }
int mean() const { return m_num_samples > 0 ? (m_mean + 32) / 64 : 0; } Int mean() const { return m_num_samples > 0 ? (m_mean + 32) / 64 : 0; }
int avg_deviation() const { return m_num_samples > 1 ? (m_average_deviation + 32) / 64 : 0; } Int avg_deviation() const { return m_num_samples > 1 ? (m_average_deviation + 32) / 64 : 0; }
int num_samples() const { return m_num_samples; } int num_samples() const { return m_num_samples; }
private: private:
// both of these are fixed point values (* 64) // both of these are fixed point values (* 64)
int m_mean = 0; Int m_mean = 0;
int m_average_deviation = 0; Int m_average_deviation = 0;
// the number of samples we have received, but no more than inverted_gain // the number of samples we have received, but no more than inverted_gain
// this is the effective inverted_gain // this is the effective inverted_gain
int m_num_samples = 0; int m_num_samples = 0;

View File

@ -60,7 +60,7 @@ span<char> receive_buffer::reserve(int const size)
// since we just increased the size of the buffer, reset the watermark to // since we just increased the size of the buffer, reset the watermark to
// start at our new size (avoid flapping the buffer size) // start at our new size (avoid flapping the buffer size)
m_watermark = sliding_average<20>(); m_watermark = {};
} }
return aux::typed_span<char>(m_recv_buffer).subspan(m_recv_end, size); return aux::typed_span<char>(m_recv_buffer).subspan(m_recv_end, size);
@ -83,7 +83,7 @@ void receive_buffer::grow(int const limit)
// since we just increased the size of the buffer, reset the watermark to // since we just increased the size of the buffer, reset the watermark to
// start at our new size (avoid flapping the buffer size) // start at our new size (avoid flapping the buffer size)
m_watermark = sliding_average<20>(); m_watermark = {};
} }
int receive_buffer::advance_pos(int const bytes) int receive_buffer::advance_pos(int const bytes)
@ -180,7 +180,7 @@ void receive_buffer::normalize(int const force_shrink)
// if the running average drops below half of the current buffer size, // if the running average drops below half of the current buffer size,
// reallocate a smaller one. // reallocate a smaller one.
bool const shrink_buffer = int(m_recv_buffer.size()) / 2 > m_watermark.mean() bool const shrink_buffer = std::int64_t(m_recv_buffer.size()) / 2 > m_watermark.mean()
&& m_watermark.mean() > (m_recv_end - m_recv_start); && m_watermark.mean() > (m_recv_end - m_recv_start);
span<char const> bytes_to_shift(m_recv_buffer.data() + m_recv_start span<char const> bytes_to_shift(m_recv_buffer.data() + m_recv_start

View File

@ -472,7 +472,7 @@ struct utp_socket_impl
std::int32_t m_recv_delay = 0; std::int32_t m_recv_delay = 0;
// average RTT // average RTT
sliding_average<16> m_rtt; sliding_average<int, 16> m_rtt;
// if this is != 0, it means the upper layer provided a reason for why // if this is != 0, it means the upper layer provided a reason for why
// the connection is being closed. The reason is indicated by this // the connection is being closed. The reason is indicated by this

View File

@ -211,6 +211,17 @@ TORRENT_TEST(receive_buffer_max_receive)
TEST_EQUAL(b.max_receive(), max_receive - 20); TEST_EQUAL(b.max_receive(), max_receive - 20);
} }
TORRENT_TEST(receive_buffer_watermark)
{
receive_buffer b;
b.reset(0x4000);
b.reserve(35000000);
b.received(35000000);
b.normalize();
TEST_EQUAL(b.watermark(), 35000000);
}
#if !defined(TORRENT_DISABLE_ENCRYPTION) && !defined(TORRENT_DISABLE_EXTENSIONS) #if !defined(TORRENT_DISABLE_ENCRYPTION) && !defined(TORRENT_DISABLE_EXTENSIONS)
TORRENT_TEST(recv_buffer_mutable_buffers) TORRENT_TEST(recv_buffer_mutable_buffers)

View File

@ -53,7 +53,7 @@ using namespace lt;
// make sure we react quickly for the first few samples // make sure we react quickly for the first few samples
TORRENT_TEST(reaction_time) TORRENT_TEST(reaction_time)
{ {
sliding_average<10> avg; sliding_average<int, 10> avg;
avg.add_sample(-10); avg.add_sample(-10);
avg.add_sample(10); avg.add_sample(10);
@ -63,7 +63,7 @@ TORRENT_TEST(reaction_time)
TORRENT_TEST(reaction_time2) TORRENT_TEST(reaction_time2)
{ {
sliding_average<10> avg; sliding_average<int, 10> avg;
avg.add_sample(10); avg.add_sample(10);
avg.add_sample(20); avg.add_sample(20);
@ -74,7 +74,7 @@ TORRENT_TEST(reaction_time2)
// make sure we converge // make sure we converge
TORRENT_TEST(converge) TORRENT_TEST(converge)
{ {
sliding_average<10> avg; sliding_average<int, 10> avg;
avg.add_sample(100); avg.add_sample(100);
for (int i = 0; i < 20; ++i) for (int i = 0; i < 20; ++i)
avg.add_sample(10); avg.add_sample(10);
@ -83,7 +83,7 @@ TORRENT_TEST(converge)
TORRENT_TEST(converge2) TORRENT_TEST(converge2)
{ {
sliding_average<10> avg; sliding_average<int, 10> avg;
avg.add_sample(-100); avg.add_sample(-100);
for (int i = 0; i < 20; ++i) for (int i = 0; i < 20; ++i)
avg.add_sample(-10); avg.add_sample(-10);
@ -93,7 +93,7 @@ TORRENT_TEST(converge2)
// test with a more realistic input // test with a more realistic input
TORRENT_TEST(random_converge) TORRENT_TEST(random_converge)
{ {
sliding_average<10> avg; sliding_average<int, 10> avg;
for (int i = 0; i < int(sizeof(samples)/sizeof(samples[0])); ++i) for (int i = 0; i < int(sizeof(samples)/sizeof(samples[0])); ++i)
avg.add_sample(samples[i]); avg.add_sample(samples[i]);
TEST_CHECK(abs(avg.mean() - 60) <= 3); TEST_CHECK(abs(avg.mean() - 60) <= 3);
@ -101,7 +101,7 @@ TORRENT_TEST(random_converge)
TORRENT_TEST(sliding_average) TORRENT_TEST(sliding_average)
{ {
sliding_average<4> avg; sliding_average<int, 4> avg;
TEST_EQUAL(avg.mean(), 0); TEST_EQUAL(avg.mean(), 0);
TEST_EQUAL(avg.avg_deviation(), 0); TEST_EQUAL(avg.avg_deviation(), 0);
avg.add_sample(500); avg.add_sample(500);