From cc7ff1606ce91691f7e3a11367df1cb1cedb31c9 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Mon, 14 May 2012 04:48:23 +0000 Subject: [PATCH] add unit test for seed optimization in piece picker and make it reliable in the presence of dont-have messages --- include/libtorrent/piece_picker.hpp | 2 + src/piece_picker.cpp | 52 ++++++++++++++++++++++- test/test_piece_picker.cpp | 64 +++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 313a480d2..877964e51 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -495,6 +495,8 @@ namespace libtorrent BOOST_STATIC_ASSERT(sizeof(piece_pos) == sizeof(char) * 8); #endif + void break_one_seed(); + void update_pieces() const; // fills in the range [start, end) of pieces in diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index cfcc3e6e7..6b6c5f5c0 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -915,6 +915,28 @@ namespace libtorrent update(prev_priority, p.index); } + // this function decrements the m_seeds counter + // and increments the peer counter on every piece + // instead. Sometimes of we connect to a seed that + // later sends us a dont-have message, we'll need to + // turn that m_seed into counts on the pieces since + // they can't be negative + void piece_picker::break_one_seed() + { + TORRENT_PIECE_PICKER_INVARIANT_CHECK; + + TORRENT_ASSERT(m_seeds > 0); + --m_seeds; + + for (std::vector::iterator i = m_piece_map.begin() + , end(m_piece_map.end()); i != end; ++i) + { + ++i->peer_count; + } + + m_dirty = true; + } + void piece_picker::dec_refcount(int index) { #ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS @@ -922,6 +944,17 @@ namespace libtorrent #endif piece_pos& p = m_piece_map[index]; + + if (p.peer_count == 0) + { + TORRENT_ASSERT(m_seeds > 0); + // this is the case where we have one or more + // seeds, and one of them saying: I don't have this + // piece anymore. we need to break up one of the seed + // counters into actual peer counters on the pieces + break_one_seed(); + } + int prev_priority = p.priority(this); TORRENT_ASSERT(p.peer_count > 0); --p.peer_count; @@ -960,12 +993,27 @@ namespace libtorrent int index = 0; bool updated = false; + bool seed_broken = false; for (bitfield::const_iterator i = bitmask.begin() , end(bitmask.end()); i != end; ++i, ++index) { if (*i) { - --m_piece_map[index].peer_count; + piece_pos& p = m_piece_map[index]; + + if (p.peer_count == 0) + { + TORRENT_ASSERT(!seed_broken); + TORRENT_ASSERT(m_seeds > 0); + // this is the case where we have one or more + // seeds, and one of them saying: I don't have this + // piece anymore. we need to break up one of the seed + // counters into actual peer counters on the pieces + break_one_seed(); + seed_broken = true; + } + + --p.peer_count; updated = true; } } @@ -1693,7 +1741,7 @@ namespace libtorrent for (int i = 0; i < num_pieces(); ++i) { if (!pieces[i]) continue; - if (piece_priority(i) == 0) continue; + if (m_piece_map[i].priority(this) <= 0) continue; if (have_piece(i)) continue; std::vector::const_iterator k = find_dl_piece(i); diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index b1d485a0e..4070e710a 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -184,6 +184,31 @@ bool verify_pick(boost::shared_ptr p return picked.size() == blocks.size(); } +void print_availability(boost::shared_ptr const& p) +{ + std::vector avail; + p->get_availability(avail); + printf("[ "); + for (std::vector::iterator i = avail.begin() + , end(avail.end()); i != end; ++i) + { + printf("%d ", *i); + } + printf("]\n"); +} + +bool verify_availability(boost::shared_ptr const& p, char const* a) +{ + std::vector avail; + p->get_availability(avail); + for (std::vector::iterator i = avail.begin() + , end(avail.end()); i != end; ++i, ++a) + { + if (*a - '0' != *i) return false; + } + return true; +} + void print_pick(std::vector const& picked) { for (int i = 0; i < int(picked.size()); ++i) @@ -954,6 +979,45 @@ int test_main() for (int i = 1; i < int(picked.size()); ++i) TEST_CHECK(picked[i] == piece_block(5, i)); +// ======================================================== + + // test seed optimizaton + print_title("test seed optimization"); + p = setup_picker("0000000000000000", " ", "", ""); + + // make sure it's not dirty + pick_pieces(p, "****************", 1, 1, 0, piece_picker::fast, options, empty_vector); + + p->inc_refcount_all(); + print_availability(p); + TEST_CHECK(verify_availability(p, "1111111111111111")); + + // make sure it's not dirty + pick_pieces(p, "****************", 1, 1, 0, piece_picker::fast, options, empty_vector); + p->dec_refcount(string2vec(" **** ** ")); + print_availability(p); + TEST_CHECK(verify_availability(p, "1100001100111111")); + + // make sure it's not dirty + pick_pieces(p, "****************", 1, 1, 0, piece_picker::fast, options, empty_vector); + p->inc_refcount(string2vec(" **** ** ")); + TEST_CHECK(verify_availability(p, "1111111111111111")); + + // make sure it's not dirty + pick_pieces(p, "****************", 1, 1, 0, piece_picker::fast, options, empty_vector); + p->dec_refcount_all(); + TEST_CHECK(verify_availability(p, "0000000000000000")); + + p->inc_refcount_all(); + print_availability(p); + TEST_CHECK(verify_availability(p, "1111111111111111")); + + // make sure it's not dirty + pick_pieces(p, "****************", 1, 1, 0, piece_picker::fast, options, empty_vector); + p->dec_refcount(3); + print_availability(p); + TEST_CHECK(verify_availability(p, "1110111111111111")); + // MISSING TESTS: // 1. abort_download // 2. write_failed