From ca3ea591df71866c5019fad8f85f0087562a6e4a Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 23 Jan 2018 01:44:29 +0100 Subject: [PATCH] fix pad-file scalability issue --- ChangeLog | 1 + include/libtorrent/piece_picker.hpp | 15 +++-- src/piece_picker.cpp | 53 ++++++++++++++--- src/torrent.cpp | 6 +- test/test_piece_picker.cpp | 90 +++++++++++++++++++++++++++++ 5 files changed, 147 insertions(+), 18 deletions(-) diff --git a/ChangeLog b/ChangeLog index ef32f3bff..17b3d1518 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,5 @@ + * fix pad-file scalability issue * made coalesce_reads/coalesce_writes settings take effect on linux and windows * restore support for incoming connections over SOCKS5 (disabled by default) * use unique peer_ids per connection diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 5f726f2f4..562e833aa 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -43,19 +43,12 @@ POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include #include #include -#ifdef TORRENT_DEBUG_REFCOUNTS -#include -#endif - -#if TORRENT_USE_ASSERTS -#include -#endif - #include "libtorrent/aux_/disable_warnings_pop.hpp" #include "libtorrent/peer_id.hpp" @@ -353,6 +346,7 @@ namespace libtorrent void mark_as_canceled(piece_block block, torrent_peer* peer); void mark_as_finished(piece_block block, torrent_peer* peer); + void mark_as_pad(piece_block block); // prevent blocks from being picked from this piece. // to unlock the piece, call restore_piece() on it @@ -756,6 +750,11 @@ namespace libtorrent // TODO: should this be allocated lazily? mutable std::vector m_piece_map; + // this maps pieces to a range of blocks that are pad files and should not + // be picked + // TOOD: this could be a much more efficient data structure + std::set m_pad_blocks; + // the number of seeds. These are not added to // the availability counters of the pieces int m_seeds; diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index ac759699a..d88ebf76b 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -238,7 +238,15 @@ namespace libtorrent for (int i = 0; i < m_blocks_per_piece; ++i) { info[i].num_peers = 0; - info[i].state = block_info::state_none; + if (m_pad_blocks.count(piece_block(piece, i))) + { + info[i].state = block_info::state_finished; + ++ret.finished; + } + else + { + info[i].state = block_info::state_none; + } info[i].peer = 0; #ifdef TORRENT_USE_VALGRIND VALGRIND_CHECK_VALUE_IS_DEFINED(info[i].peer); @@ -252,8 +260,13 @@ namespace libtorrent VALGRIND_CHECK_VALUE_IS_DEFINED(ret.info_idx); VALGRIND_CHECK_VALUE_IS_DEFINED(ret.index); #endif + downloading_iter = m_downloads[download_state].insert(downloading_iter, ret); + // in case every block was a pad block, we need to make sure the piece + // structure is correctly categorised + downloading_iter = update_piece_state(downloading_iter); + #if TORRENT_USE_INVARIANT_CHECKS check_piece_state(); #endif @@ -2948,16 +2961,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; @@ -3154,6 +3167,9 @@ get_out: block_info* binfo = blocks_for_piece(*dp); block_info& info = binfo[block.block_index]; TORRENT_ASSERT(info.piece_index == block.piece_index); + if (info.state == block_info::state_finished) + return false; + info.state = block_info::state_requested; info.peer = peer; info.num_peers = 1; @@ -3289,7 +3305,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_boundries.size()) || m_dirty); p.download_state = piece_pos::piece_downloading; @@ -3303,6 +3319,11 @@ get_out: TORRENT_ASSERT(&info >= &m_block_info[0]); TORRENT_ASSERT(&info < &m_block_info[0] + m_block_info.size()); TORRENT_ASSERT(info.piece_index == block.piece_index); + + TORRENT_ASSERT(info.state == block_info::state_none); + if (info.state == block_info::state_finished) + return false; + info.state = block_info::state_writing; info.peer = peer; info.num_peers = 0; @@ -3543,7 +3564,7 @@ get_out: TORRENT_PIECE_PICKER_INVARIANT_CHECK; #endif - int prio = p.priority(this); + int const prio = p.priority(this); TORRENT_ASSERT(prio < int(m_priority_boundries.size()) || m_dirty); p.download_state = piece_pos::piece_downloading; @@ -3555,6 +3576,8 @@ get_out: TORRENT_ASSERT(&info >= &m_block_info[0]); TORRENT_ASSERT(&info < &m_block_info[0] + m_block_info.size()); TORRENT_ASSERT(info.piece_index == block.piece_index); + if (info.state == block_info::state_finished) + return; info.peer = peer; TORRENT_ASSERT(info.state == block_info::state_none); TORRENT_ASSERT(info.num_peers == 0); @@ -3615,6 +3638,22 @@ get_out: } + void piece_picker::mark_as_pad(piece_block block) + { + m_pad_blocks.insert(block); + // if we mark and entire piece as a pad file, we need to also + // consder that piece as "had" and increment some counters + typedef std::set::iterator iter; + iter begin = m_pad_blocks.lower_bound(piece_block(block.piece_index, 0)); + int const blocks = blocks_in_piece(block.piece_index); + iter end = m_pad_blocks.upper_bound(piece_block(block.piece_index, blocks)); + if (std::distance(begin, end) == blocks) + { + // the entire piece is a pad file + we_have(block.piece_index); + } + } + /* void piece_picker::mark_as_checking(int index) { @@ -3748,7 +3787,7 @@ get_out: TORRENT_ASSERT(prev_prio < int(m_priority_boundries.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/src/torrent.cpp b/src/torrent.cpp index 1fd5ada27..6cc5f992b 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -2044,7 +2044,7 @@ namespace libtorrent for (; pr.length >= block; pr.length -= block, ++pb.block_index) { if (int(pb.block_index) == blocks_per_piece) { pb.block_index = 0; ++pb.piece_index; } - m_picker->mark_as_finished(pb, 0); + m_picker->mark_as_pad(pb); } // ugly edge case where padfiles are not used they way they're // supposed to be. i.e. added back-to back or at the end @@ -3939,7 +3939,7 @@ namespace { } // fills in total_wanted, total_wanted_done and total_done - void torrent::bytes_done(torrent_status& st, bool accurate) const + void torrent::bytes_done(torrent_status& st, bool const accurate) const { INVARIANT_CHECK; @@ -4004,7 +4004,7 @@ namespace { if (m_picker->has_piece_passed(last_piece)) { TORRENT_ASSERT(st.total_done >= piece_size); - int corr = m_torrent_file->piece_size(last_piece) + int const corr = m_torrent_file->piece_size(last_piece) - piece_size; TORRENT_ASSERT(corr <= 0); TORRENT_ASSERT(corr > -piece_size); diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 062b98bd8..91cf411e1 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -1865,5 +1865,95 @@ TORRENT_TEST(piece_picker) TEST_EQUAL(picked.size(), blocks_per_piece); for (int i = 0; i < int(picked.size()); ++i) TEST_EQUAL(picked[0].piece_index, 4); + + { + print_title("test mark_as_pad"); + + p = setup_picker("1111111", " ", "4444444", ""); + piece_block const bl(2, 0); + p->mark_as_pad(bl); + + bool ret = p->mark_as_downloading(piece_block(2, 1), NULL); + TEST_EQUAL(ret, true); + + std::vector dl + = p->get_download_queue(); + + TEST_EQUAL(dl.size(), 1); + TEST_EQUAL(dl[0].finished, 1); + TEST_EQUAL(dl[0].writing, 0); + TEST_EQUAL(dl[0].requested, 1); + TEST_EQUAL(dl[0].index, 2); + + piece_picker::block_info* blocks = p->blocks_for_piece(dl[0]); + TEST_EQUAL(blocks[0].state, piece_picker::block_info::state_finished); + TEST_EQUAL(blocks[1].state, piece_picker::block_info::state_requested); + TEST_EQUAL(blocks[2].state, piece_picker::block_info::state_none); + TEST_EQUAL(blocks[3].state, piece_picker::block_info::state_none); + } + + { + print_title("test mark_as_pad downloading"); + + p = setup_picker("1111111", " ", "4444444", ""); + piece_block const bl(2, 0); + p->mark_as_pad(bl); + + bool ret = p->mark_as_downloading(piece_block(2, 0), NULL); + TEST_EQUAL(ret, false); + + std::vector dl + = p->get_download_queue(); + + TEST_EQUAL(dl.size(), 1); + TEST_EQUAL(dl[0].finished, 1); + TEST_EQUAL(dl[0].writing, 0); + TEST_EQUAL(dl[0].requested, 0); + TEST_EQUAL(dl[0].index, 2); + + piece_picker::block_info* blocks = p->blocks_for_piece(dl[0]); + TEST_EQUAL(blocks[0].state, piece_picker::block_info::state_finished); + TEST_EQUAL(blocks[1].state, piece_picker::block_info::state_none); + TEST_EQUAL(blocks[2].state, piece_picker::block_info::state_none); + TEST_EQUAL(blocks[3].state, piece_picker::block_info::state_none); + } + + { + print_title("test mark_as_pad seeding"); + + p = setup_picker("1", " ", "4", ""); + p->mark_as_pad(piece_block(0, 0)); + p->mark_as_pad(piece_block(0, 1)); + p->mark_as_pad(piece_block(0, 2)); + + TEST_CHECK(!p->is_seeding()); + + p->mark_as_finished(piece_block(0, 3), NULL); + + TEST_CHECK(!p->is_seeding()); + p->piece_passed(0); + TEST_CHECK(p->is_seeding()); + } + + { + print_title("test mark_as_pad whole pad piece, seeding"); + + p = setup_picker("11", " ", "44", ""); + p->mark_as_pad(piece_block(0, 0)); + p->mark_as_pad(piece_block(0, 1)); + p->mark_as_pad(piece_block(0, 2)); + p->mark_as_pad(piece_block(0, 3)); + + TEST_CHECK(!p->is_seeding()); + + p->mark_as_finished(piece_block(1, 0), NULL); + p->mark_as_finished(piece_block(1, 1), NULL); + p->mark_as_finished(piece_block(1, 2), NULL); + p->mark_as_finished(piece_block(1, 3), NULL); + + TEST_CHECK(!p->is_seeding()); + p->piece_passed(1); + TEST_CHECK(p->is_seeding()); + } }