From a8623a9b23090c65fe2e805bb6c1d2123c123f5a Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Sun, 8 Feb 2015 18:01:29 +0000 Subject: [PATCH] request partial pieces in rarest-first order --- include/libtorrent/piece_picker.hpp | 7 +++- src/piece_picker.cpp | 65 +++++++++++++++++++++++++---- test/test_piece_picker.cpp | 38 +++++++++++++---- 3 files changed, 93 insertions(+), 17 deletions(-) diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 1e577945b..8310bff4b 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -480,6 +480,8 @@ namespace libtorrent std::pair distributed_copies() const; + void set_num_pad_files(int n) { m_num_pad_files = n; } + private: friend struct piece_pos; @@ -705,8 +707,6 @@ namespace libtorrent { return index == p.index && peer_count == p.peer_count; } }; - void set_num_pad_files(int n) { m_num_pad_files = n; } - private: #ifndef TORRENT_DEBUG_REFCOUNTS @@ -717,6 +717,9 @@ namespace libtorrent #endif #endif + bool partial_compare_rarest_first(downloading_piece const* lhs + , downloading_piece const* rhs) const; + void break_one_seed(); void update_pieces() const; diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index f1b654d87..024e9d389 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -1888,6 +1888,28 @@ namespace libtorrent src.clear(); return num_blocks - to_copy; } + + } + + // lower availability comes first. This is a less-than comparison, it returns + // true if lhs has lower availability than rhs + bool piece_picker::partial_compare_rarest_first(downloading_piece const* lhs + , downloading_piece const* rhs) const + { + int lhs_availability = m_piece_map[lhs->index].peer_count; + int rhs_availability = m_piece_map[rhs->index].peer_count; + if (lhs_availability != rhs_availability) + return lhs_availability < rhs_availability; + + // if the availability is the same, prefer the piece that's closest to + // being complete. + int lhs_blocks_left = m_blocks_per_piece - lhs->finished - lhs->writing + - lhs->requested; + TORRENT_ASSERT(lhs_blocks_left > 0); + int rhs_blocks_left = m_blocks_per_piece - rhs->finished - rhs->writing + - rhs->requested; + TORRENT_ASSERT(rhs_blocks_left > 0); + return lhs_blocks_left < rhs_blocks_left; } // pieces describes which pieces the peer we're requesting from has. @@ -1983,29 +2005,58 @@ namespace libtorrent if (options & prioritize_partials) { - // TODO: 3 prioritize partials correctly. either by rarity or by which - // one is closest to being complete + // first, allocate a small array on the stack of all the partial + // pieces (downloading_piece). We'll then sort this list by + // availability or by some other condition. The list of partial pieces + // in m_downloads is ordered by piece index, this is to have O(log n) + // lookups when finding a downloading_piece for a specific piece index. + // this is important and needs to stay sorted that way, that's why + // we're copying it here + downloading_piece const** ordered_partials = TORRENT_ALLOCA( + downloading_piece const*, m_downloads[piece_pos::piece_downloading].size()); + int num_ordered_partials = 0; + + // now, copy over the pointers. We also apply a filter here to not + // include ineligible pieces in certain modes. For instance, a piece + // that the current peer doesn't have is not included. for (std::vector::const_iterator i = m_downloads[piece_pos::piece_downloading].begin() , end(m_downloads[piece_pos::piece_downloading].end()); i != end; ++i) { + pc.inc_stats_counter(counters::piece_picker_partial_loops); + // in time critical mode, only pick high priority pieces if ((options & time_critical_mode) && piece_priority(i->index) != priority_levels - 1) continue; - pc.inc_stats_counter(counters::piece_picker_partial_loops); if (!is_piece_free(i->index, pieces)) continue; + TORRENT_ASSERT(m_piece_map[i->index].download_queue() == piece_pos::piece_downloading); - if (int(backup_blocks.size()) >= num_blocks - && int(backup_blocks2.size()) >= num_blocks) - break; - num_blocks = add_blocks_downloading(*i, pieces + ordered_partials[num_ordered_partials++] = &*i; + } + + // now, sort the list. + // TODO: this could probably be optimized by incrementally + // calling partial_sort to sort one more element in the list. Because + // chances are that we'll just need a single piece, and once we've + // picked from it we're done. Sorting the rest of the list in that + // case is a waste of time. + std::sort(ordered_partials, ordered_partials + num_ordered_partials + , boost::bind(&piece_picker::partial_compare_rarest_first, this + , _1, _2)); + + for (int i = 0; i < num_ordered_partials; ++i) + { + num_blocks = add_blocks_downloading(*ordered_partials[i], pieces , interesting_blocks, backup_blocks, backup_blocks2 , num_blocks, prefer_contiguous_blocks, peer, speed, options); if (num_blocks <= 0) return; + if (int(backup_blocks.size()) >= num_blocks + && int(backup_blocks2.size()) >= num_blocks) + break; } num_blocks = append_blocks(interesting_blocks, backup_blocks diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 82a7c2f69..ab8c40000 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -611,14 +611,36 @@ int test_main() // ======================================================== - // make sure downloading pieces closer to completion have higher priority - // piece 3 has only 1 block from being completed, and should be picked -// print_title("test downloading piece order"); -// p = setup_picker("1111111", " ", "", "013700f"); -// picked = pick_pieces(p, "*******", 1, 0, 0, piece_picker::fast -// , options | piece_picker::prioritize_partials, empty_vector); -// TEST_CHECK(int(picked.size()) > 0); -// TEST_CHECK(picked.front() == piece_block(3, 3)); + // when we're prioritizing partial pieces, make sure to first pick the + // rarest of them. The blocks in this test are: + // 0: [ ] avail: 1 + // 1: [x ] avail: 1 + // 2: [xx ] avail: 1 + // 3: [xxx ] avail: 2 + // 4: [ ] avail: 1 + // 5: [ ] avail: 1 + // 6: [xxxx] avail: 1 + // piece 6 does not have any blocks left to pick, even though piece 3 only + // has a single block left before it completes, it is less rare than piece + // 2. Piece 2 is the best pick in this case. + print_title("test partial piece order (rarest first)"); + p = setup_picker("1112111", " ", "", "013700f"); + picked = pick_pieces(p, "*******", 1, 0, 0, piece_picker::fast + , options | piece_picker::prioritize_partials, empty_vector); + TEST_CHECK(int(picked.size()) > 0); + TEST_CHECK(picked.front() == piece_block(2, 2) + || picked.front() == piece_block(2, 3)); + + // as a tie breaker, make sure downloading pieces closer to completion have + // higher priority. piece 3 is only 1 block from being completed, and should + // be picked + + print_title("test partial piece order (most complete)"); + p = setup_picker("1111111", " ", "", "013700f"); + picked = pick_pieces(p, "*******", 1, 0, 0, piece_picker::fast + , options | piece_picker::prioritize_partials, empty_vector); + TEST_CHECK(int(picked.size()) > 0); + TEST_CHECK(picked.front() == piece_block(3, 3)); // ========================================================