diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 39548d5ce..0cc4f3e41 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -330,8 +330,6 @@ namespace libtorrent void check_peers(); #endif -// int get_block_state(piece_block block) const; - // returns true if any client is currently downloading this // piece-block, or if it's queued for downloading by some client // or if it already has been successfully downloaded @@ -352,7 +350,7 @@ namespace libtorrent void mark_as_canceled(piece_block block, torrent_peer* peer); void mark_as_finished(piece_block block, torrent_peer* peer); - + // prevent blocks from being picked from this piece. // to unlock the piece, call restore_piece() on it void lock_piece(int piece); @@ -362,9 +360,6 @@ namespace libtorrent void piece_passed(int index); -// void mark_as_checking(int index); -// void mark_as_done_checking(int index); - // returns information about the given piece void piece_info(int index, piece_picker::downloading_piece& st) const; diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index a9bf5abb6..68f855e07 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -356,7 +356,9 @@ namespace libtorrent } } - void piece_picker::verify_priority(int range_start, int range_end, int prio) const + void piece_picker::verify_priority(int const range_start + , int const range_end + , int const prio) const { TORRENT_ASSERT(range_start <= range_end); TORRENT_ASSERT(range_end <= int(m_pieces.size())); @@ -616,7 +618,7 @@ namespace libtorrent for (std::vector::const_iterator i = m_piece_map.begin(); i != m_piece_map.end(); ++i) { - int index = static_cast(i - m_piece_map.begin()); + int const index = static_cast(i - m_piece_map.begin()); piece_pos const& p = *i; if (p.filtered()) @@ -642,7 +644,8 @@ namespace libtorrent if (t != nullptr) TORRENT_ASSERT(!t->have_piece(index)); - int prio = p.priority(this); + int const prio = p.priority(this); + #if TORRENT_USE_ASSERTS if (p.downloading()) { @@ -659,7 +662,7 @@ namespace libtorrent if (!m_dirty) { - TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) || m_dirty); + TORRENT_ASSERT(prio < int(m_priority_boundaries.size())); if (prio >= 0) { TORRENT_ASSERT(p.index < m_pieces.size()); @@ -773,7 +776,7 @@ namespace libtorrent return std::make_pair(min_availability + m_seeds, fraction_part * 1000 / num_pieces); } - std::pair piece_picker::priority_range(int prio) + std::pair piece_picker::priority_range(int const prio) { TORRENT_ASSERT(prio >= 0); TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) || m_dirty); @@ -789,7 +792,7 @@ namespace libtorrent TORRENT_ASSERT(!m_dirty); TORRENT_ASSERT(index >= 0); TORRENT_ASSERT(index < int(m_piece_map.size())); - piece_pos& p = m_piece_map[index]; + piece_pos const& p = m_piece_map[index]; TORRENT_ASSERT(!p.filtered()); TORRENT_ASSERT(!p.have()); @@ -802,7 +805,7 @@ namespace libtorrent TORRENT_ASSERT(int(m_priority_boundaries.size()) >= priority); - auto range = priority_range(priority); + auto const range = priority_range(priority); int new_index = (range.second == range.first) ? range.first : random(range.second - range.first) + range.first; @@ -918,7 +921,7 @@ namespace libtorrent piece_pos& p = m_piece_map[index]; TORRENT_ASSERT(int(p.index) == elem_index || p.have()); - int new_priority = p.priority(this); + int const new_priority = p.priority(this); if (new_priority == priority) return; @@ -1069,9 +1072,9 @@ namespace libtorrent i->locked = false; piece_pos& p = m_piece_map[index]; - int prev_priority = p.priority(this); + int const prev_priority = p.priority(this); erase_download_piece(i); - int new_priority = p.priority(this); + int const new_priority = p.priority(this); #if TORRENT_USE_INVARIANT_CHECKS check_piece_state(); @@ -1502,9 +1505,9 @@ namespace libtorrent , end(m_piece_map.end()); i != end; ++i, ++index) { piece_pos& p = *i; - int prio = p.priority(this); + int const prio = p.priority(this); if (prio == -1) continue; - int new_index = (prio == 0 ? 0 : m_priority_boundaries[prio - 1]) + p.index; + int const new_index = (prio == 0 ? 0 : m_priority_boundaries[prio - 1]) + p.index; m_pieces[new_index] = index; } @@ -1631,8 +1634,8 @@ namespace libtorrent << index << ")" << std::endl; #endif piece_pos& p = m_piece_map[index]; - int info_index = p.index; - int priority = p.priority(this); + int const info_index = p.index; + int const priority = p.priority(this); TORRENT_ASSERT(priority < int(m_priority_boundaries.size()) || m_dirty); if (p.have()) return; @@ -1690,11 +1693,9 @@ namespace libtorrent TORRENT_ASSERT(p.priority(this) == -1); } - bool piece_picker::set_piece_priority(int index, int new_piece_priority) + bool piece_picker::set_piece_priority(int const index, int const new_piece_priority) { -#ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS INVARIANT_CHECK; -#endif #ifdef TORRENT_PICKER_LOG std::cerr << "[" << this << "] " << "set_piece_priority(" << index @@ -1780,7 +1781,13 @@ namespace libtorrent TORRENT_ASSERT(m_num_have_filtered >= 0); p.piece_priority = new_piece_priority; - int new_priority = p.priority(this); + int const new_priority = p.priority(this); + + if (prev_priority != new_priority && !m_dirty) + { + if (prev_priority == -1) add(index); + else update(prev_priority, p.index); + } if (p.downloading()) { @@ -1790,17 +1797,6 @@ namespace libtorrent update_piece_state(i); } - if (prev_priority == new_priority) return ret; - - if (m_dirty) return ret; - if (prev_priority == -1) - { - add(index); - } - else - { - update(prev_priority, p.index); - } return ret; } @@ -2132,8 +2128,8 @@ namespace libtorrent { for (int i = int(m_priority_boundaries.size()) - 1; i >= 0; --i) { - int start = (i == 0) ? 0 : m_priority_boundaries[i - 1]; - int end = m_priority_boundaries[i]; + int const start = (i == 0) ? 0 : m_priority_boundaries[i - 1]; + int const end = m_priority_boundaries[i]; for (int p = end - 1; p >= start; --p) { pc.inc_stats_counter(counters::piece_picker_reverse_rare_loops); @@ -2894,16 +2890,16 @@ get_out: } std::vector::iterator - piece_picker::update_piece_state( + piece_picker::update_piece_state( std::vector::iterator dp) { #ifdef TORRENT_PICKER_LOG std::cerr << "[" << this << "] " << "update_piece_state(" << dp->index << ")" << std::endl; #endif - int num_blocks = blocks_in_piece(dp->index); + int const num_blocks = blocks_in_piece(dp->index); piece_pos& p = m_piece_map[dp->index]; - int current_state = p.download_state; + int const current_state = p.download_state; TORRENT_ASSERT(current_state != piece_pos::piece_open); if (current_state == piece_pos::piece_open) return dp; @@ -2952,7 +2948,9 @@ get_out: downloading_piece dp_info = *dp; m_downloads[p.download_queue()].erase(dp); - int prio = p.priority(this); + int const prio = p.priority(this); + TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) + || m_dirty); p.download_state = new_state; #ifdef TORRENT_PICKER_LOG std::cerr << "[" << this << "] " << " " << dp_info.index << " state (" << current_state << " -> " << new_state << ")" << std::endl; @@ -2977,29 +2975,7 @@ get_out: return i; } -/* - int piece_picker::get_block_state(piece_block block) const - { - TORRENT_ASSERT(block.block_index != piece_block::invalid.block_index); - TORRENT_ASSERT(block.piece_index != piece_block::invalid.piece_index); - TORRENT_ASSERT(block.piece_index < m_piece_map.size()); - // if we have the piece, the block state is considered finished - if (m_piece_map[block.piece_index].index == piece_pos::we_have_index) - return block_info::state_finished; - - int state = m_piece_map[block.piece_index].download_queue(); - if (state == piece_pos::piece_open) return block_info::state_none; - std::vector::const_iterator i = find_dl_piece(state - , block.piece_index); - - TORRENT_ASSERT(i != m_downloads[state].end()); - - block_info const* info = blocks_for_piece(*i); - TORRENT_ASSERT(info[block.block_index].piece_index == block.piece_index); - return info[block.block_index].state; - } -*/ bool piece_picker::is_requested(piece_block block) const { TORRENT_ASSERT(block.block_index != piece_block::invalid.block_index); @@ -3056,6 +3032,7 @@ get_out: } // options may be 0 or piece_picker::reverse + // returns false if the block could not be marked as downloading bool piece_picker::mark_as_downloading(piece_block block , torrent_peer* peer, int options) { @@ -3077,7 +3054,7 @@ get_out: #ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS INVARIANT_CHECK; #endif - int prio = p.priority(this); + int const prio = p.priority(this); TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) || m_dirty); @@ -3226,7 +3203,7 @@ get_out: // if we already have this piece, just ignore this if (have_piece(block.piece_index)) return false; - int prio = p.priority(this); + int const prio = p.priority(this); TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) || m_dirty); p.download_state = piece_pos::piece_downloading; @@ -3378,9 +3355,9 @@ get_out: if (i->finished + i->writing + i->requested == 0) { piece_pos& p = m_piece_map[block.piece_index]; - int prev_priority = p.priority(this); + int const prev_priority = p.priority(this); erase_download_piece(i); - int new_priority = p.priority(this); + int const new_priority = p.priority(this); if (m_dirty) return; if (new_priority == prev_priority) return; @@ -3432,9 +3409,9 @@ get_out: if (i->finished + i->writing + i->requested == 0) { - int prev_priority = p.priority(this); + int const prev_priority = p.priority(this); erase_download_piece(i); - int new_priority = p.priority(this); + int const new_priority = p.priority(this); if (m_dirty) return; if (new_priority == prev_priority) return; @@ -3480,7 +3457,7 @@ get_out: INVARIANT_CHECK; #endif - int prio = p.priority(this); + int const prio = p.priority(this); TORRENT_ASSERT(prio < int(m_priority_boundaries.size()) || m_dirty); p.download_state = piece_pos::piece_downloading; @@ -3549,37 +3526,15 @@ get_out: #if TORRENT_USE_INVARIANT_CHECKS check_piece_state(); #endif - } -/* - void piece_picker::mark_as_checking(int index) - { - int state = m_piece_map[index].download_queue(); - if (state == piece_pos::piece_open) return; - std::vector::iterator i = find_dl_piece(state, index); - if (i == m_downloads[state].end()) return; - TORRENT_ASSERT(i->outstanding_hash_check == false); - i->outstanding_hash_check = true; - } - - void piece_picker::mark_as_done_checking(int index) - { - int state = m_piece_map[index].download_queue(); - if (state == piece_pos::piece_open) return; - std::vector::iterator i = find_dl_piece(state, index); - if (i == m_downloads[state].end()) return; - i->outstanding_hash_check = false; - } -*/ - void piece_picker::get_downloaders(std::vector& d, int index) const { TORRENT_ASSERT(index >= 0 && index <= int(m_piece_map.size())); d.clear(); - int state = m_piece_map[index].download_queue(); - const int num_blocks = blocks_in_piece(index); + int const state = m_piece_map[index].download_queue(); + int const num_blocks = blocks_in_piece(index); d.reserve(num_blocks); if (state == piece_pos::piece_open) @@ -3602,7 +3557,7 @@ get_out: torrent_peer* piece_picker::get_downloader(piece_block block) const { - int state = m_piece_map[block.piece_index].download_queue(); + int const state = m_piece_map[block.piece_index].download_queue(); if (state == piece_pos::piece_open) return nullptr; std::vector::const_iterator i = find_dl_piece(state @@ -3685,7 +3640,7 @@ get_out: TORRENT_ASSERT(prev_prio < int(m_priority_boundaries.size()) || m_dirty); erase_download_piece(i); - int prio = p.priority(this); + int const prio = p.priority(this); if (!m_dirty) { if (prev_prio == -1 && prio >= 0) add(block.piece_index); diff --git a/test/test.hpp b/test/test.hpp index 510d9ffb7..1b14cf35f 100644 --- a/test/test.hpp +++ b/test/test.hpp @@ -117,6 +117,12 @@ extern int EXPORT _g_test_failures; s__ << "TEST_ERROR: equal check failed:\n" #x ": " << (x) << "\nexpected: " << (y); \ TEST_REPORT_AUX(s__.str().c_str(), __FILE__, __LINE__); \ } +#define TEST_NE(x, y) \ + if ((x) == (y)) { \ + std::stringstream s__; \ + s__ << "TEST_ERROR: not equal check failed:\n" #x ": " << (x) << "\nexpected not equal to: " << (y); \ + TEST_REPORT_AUX(s__.str().c_str(), __FILE__, __LINE__); \ + } #else #define TEST_CHECK(x) \ try \ @@ -149,6 +155,22 @@ extern int EXPORT _g_test_failures; { \ TEST_ERROR("TEST_ERROR: Exception thrown: " #x); \ } +#define TEST_NE(x, y) \ + try { \ + if ((x) == (y)) { \ + std::stringstream s__; \ + s__ << "TEST_ERROR: " #x ": " << (x) << " expected not equal to: " << (y); \ + TEST_REPORT_AUX(s__.str().c_str(), __FILE__, __LINE__); \ + } \ + } \ + catch (std::exception& e) \ + { \ + TEST_ERROR("TEST_ERROR: Exception thrown: " #x " :" + std::string(e.what())); \ + } \ + catch (...) \ + { \ + TEST_ERROR("TEST_ERROR: Exception thrown: " #x); \ + } #endif #define TEST_ERROR(x) \ diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 45b2a7236..b26ede4ad 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -293,7 +293,7 @@ int test_pick(boost::shared_ptr const& p const std::vector empty_vector; std::vector picked = pick_pieces(p, "*******", 1, 0, nullptr , options, empty_vector); - if (picked.empty()) return -1; + if (picked.size() != 1) return -1; return picked[0].piece_index; } @@ -1032,7 +1032,7 @@ TORRENT_TEST(restore_piece) TEST_CHECK(int(picked.size()) >= 1); TEST_CHECK(picked.front().piece_index == 1); - p->restore_piece(0); + p->restore_piece(0); picked = pick_pieces(p, "*******", 1, 0, nullptr, options, empty_vector); TEST_CHECK(int(picked.size()) >= 1); TEST_CHECK(picked.front().piece_index == 0); @@ -1870,5 +1870,118 @@ TORRENT_TEST(time_critical_mode) TEST_EQUAL(picked[0].piece_index, 4); } +TORRENT_TEST(reprioritize_downloading) +{ + auto p = setup_picker("1111111", " ", "", ""); + bool ret; + + ret = p->mark_as_downloading(piece_block(0, 0), tmp_peer); + TEST_EQUAL(ret, true); + p->mark_as_finished(piece_block(0, 1), tmp_peer); + ret = p->mark_as_writing(piece_block(0, 2), tmp_peer); + TEST_EQUAL(ret, true); + + // make sure we pick the partial piece (i.e. piece 0) + TEST_EQUAL(test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials), 0); + + // set the priority of the piece to 0 (while downloading it) + ret = p->set_piece_priority(0, 0); + TEST_EQUAL(ret, true); + + // make sure we _DON'T_ pick the partial piece, since it has priority zero + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + + // set the priority of the piece back to 1. It should now be the best pick + // again (since it's partial) + ret = p->set_piece_priority(0, 1); + TEST_EQUAL(ret, true); + + // make sure we pick the partial piece + TEST_EQUAL(test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials), 0); +} + +TORRENT_TEST(reprioritize_fully_downloading) +{ + auto p = setup_picker("1111111", " ", "", ""); + bool ret; + + for (int i = 0; i < blocks_per_piece; ++i) + { + ret = p->mark_as_downloading(piece_block(0, i), tmp_peer); + TEST_EQUAL(ret, true); + } + + // make sure we _DON'T_ pick the downloading piece + { + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + } + + // set the priority of the piece to 0 (while downloading it) + ret = p->set_piece_priority(0, 0); + TEST_EQUAL(ret, true); + + // make sure we still _DON'T_ pick the downloading piece + { + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + } + + // set the priority of the piece back to 1. It should now be the best pick + // again (since it's partial) + ret = p->set_piece_priority(0, 1); + TEST_EQUAL(ret, true); + + // make sure we still _DON'T_ pick the downloading piece + { + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + } +} + +TORRENT_TEST(download_filtered_piece) +{ + auto p = setup_picker("1111111", " ", "", ""); + bool ret; + + // set the priority of the piece to 0 + ret = p->set_piece_priority(0, 0); + TEST_EQUAL(ret, true); + + // make sure we _DON'T_ pick piece 0 + { + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + } + + // then mark it for downloading + ret = p->mark_as_downloading(piece_block(0, 0), tmp_peer); + TEST_EQUAL(ret, true); + p->mark_as_finished(piece_block(0, 1), tmp_peer); + ret = p->mark_as_writing(piece_block(0, 2), tmp_peer); + TEST_EQUAL(ret, true); + + { + // we still should not pick it + int const picked_piece = test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials); + TEST_NE(picked_piece, -1); + TEST_NE(picked_piece, 0); + } + + // set the priority of the piece back to 1. It should now be the best pick + // again (since it's partial) + ret = p->set_piece_priority(0, 1); + TEST_EQUAL(ret, true); + + // make sure we pick piece 0 + TEST_EQUAL(test_pick(p, piece_picker::rarest_first | piece_picker::prioritize_partials), 0); +} + //TODO: 2 test picking with partial pieces and other peers present so that both backup_pieces and backup_pieces2 are used