From 3bdf01778ad0c38f147770b0395d1931d6bfd9e1 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Wed, 4 Jul 2007 02:16:49 +0000 Subject: [PATCH] chamged peer representation from tcp::endpoint to policy::peer pointer in piece_picker's downloading piece. Saves memory, removes the need for lookup and improves the hash check fail/pass handling for closed connections --- include/libtorrent/piece_picker.hpp | 30 ++++++++------ src/peer_connection.cpp | 15 ++----- src/piece_picker.cpp | 54 +++++++++++++------------ src/policy.cpp | 35 ++++++++++++++++- src/torrent.cpp | 61 ++++++++++++++--------------- src/torrent_handle.cpp | 6 ++- test/test_piece_picker.cpp | 41 ++++++++++--------- 7 files changed, 138 insertions(+), 104 deletions(-) diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index c52521d0a..636f7e530 100755 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -42,7 +42,6 @@ POSSIBILITY OF SUCH DAMAGE. #pragma warning(push, 1) #endif -#include #include #ifdef _MSC_VER @@ -90,10 +89,11 @@ namespace libtorrent struct block_info { - block_info(): num_downloads(0), state(state_none) {} + block_info(): peer(0), num_downloads(0), state(state_none) {} // the peer this block was requested or - // downloaded from - tcp::endpoint peer; + // downloaded from. This is a pointer to + // a policy::peer object + void* peer; // the number of times this block has been downloaded unsigned num_downloads:14; enum { state_none, state_requested, state_writing, state_finished }; @@ -185,12 +185,16 @@ namespace libtorrent // decides to download a piece, it must mark it as being downloaded // itself, by using the mark_as_downloading() member function. // THIS IS DONE BY THE peer_connection::send_request() MEMBER FUNCTION! - // The last argument is the tcp::endpoint of the peer that we'll download - // from. + // The last argument is the policy::peer pointer for the peer that + // we'll download from. void pick_pieces(const std::vector& pieces , std::vector& interesting_blocks , int num_pieces, bool prefer_whole_pieces - , tcp::endpoint peer, piece_state_t speed) const; + , void* peer, piece_state_t speed) const; + + // clears the peer pointer in all downloading pieces with this + // peer pointer + void clear_peer(void* peer); // returns true if any client is currently downloading this // piece-block, or if it's queued for downloading by some client @@ -202,10 +206,10 @@ namespace libtorrent bool is_finished(piece_block block) const; // marks this piece-block as queued for downloading - void mark_as_downloading(piece_block block, tcp::endpoint const& peer + void mark_as_downloading(piece_block block, void* peer , piece_state_t s); - void mark_as_writing(piece_block block, tcp::endpoint const& peer); - void mark_as_finished(piece_block block, tcp::endpoint const& peer); + void mark_as_writing(piece_block block, void* peer); + void mark_as_finished(piece_block block, void* peer); // if a piece had a hash-failure, it must be restored and // made available for redownloading @@ -224,12 +228,12 @@ namespace libtorrent // the hash-check yet int unverified_blocks() const; - void get_downloaders(std::vector& d, int index) const; + void get_downloaders(std::vector& d, int index) const; std::vector const& get_download_queue() const { return m_downloads; } - boost::optional get_downloader(piece_block block) const; + void* get_downloader(piece_block block) const; // the number of filtered pieces we don't have int num_filtered() const { return m_num_filtered; } @@ -348,7 +352,7 @@ namespace libtorrent , std::vector& interesting_blocks , std::vector& backup_blocks , int num_blocks, bool prefer_whole_pieces - , tcp::endpoint peer, piece_state_t speed) const; + , void* peer, piece_state_t speed) const; downloading_piece& add_download_piece(); void erase_download_piece(std::vector::iterator i); diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index 8a2ac0ae3..739378c85 100755 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -397,15 +397,6 @@ namespace libtorrent try { (*i)->on_piece_pass(index); } catch (std::exception&) {} } #endif - - if (peer_info_struct()) - { - peer_info_struct()->on_parole = false; - int& trust_points = peer_info_struct()->trust_points; - trust_points++; - // TODO: make this limit user settable - if (trust_points > 20) trust_points = 20; - } } void peer_connection::received_invalid_data(int index) @@ -1185,7 +1176,7 @@ namespace libtorrent fs.async_write(p, data, bind(&peer_connection::on_disk_write_complete , self(), _1, _2, p, t)); - picker.mark_as_writing(block_finished, m_remote); + picker.mark_as_writing(block_finished, peer_info_struct()); if (request_peer && !request_peer->has_peer_choked() && !t->is_seed()) { @@ -1224,7 +1215,7 @@ namespace libtorrent assert(p.piece == j.piece); assert(p.start == j.offset); piece_block block_finished(p.piece, p.start / t->block_size()); - picker.mark_as_finished(block_finished, m_remote); + picker.mark_as_finished(block_finished, peer_info_struct()); if (!has_peer_choked() && !t->is_seed() && !m_torrent.expired()) { @@ -1336,7 +1327,7 @@ namespace libtorrent else if (speed == medium) state = piece_picker::medium; else state = piece_picker::slow; - t->picker().mark_as_downloading(block, m_remote, state); + t->picker().mark_as_downloading(block, peer_info_struct(), state); m_request_queue.push_back(block); } diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index bd568210f..a1afc6bc3 100755 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -116,11 +116,10 @@ namespace libtorrent for (std::vector::const_iterator i = unfinished.begin(); i != unfinished.end(); ++i) { - tcp::endpoint peer; for (int j = 0; j < m_blocks_per_piece; ++j) { if (i->info[j].state == block_info::state_finished) - mark_as_finished(piece_block(i->index, j), peer); + mark_as_finished(piece_block(i->index, j), 0); } if (is_piece_finished(i->index)) { @@ -212,7 +211,7 @@ namespace libtorrent { ret.info[i].num_downloads = 0; ret.info[i].state = block_info::state_none; - ret.info[i].peer = tcp::endpoint(); + ret.info[i].peer = 0; } return ret; } @@ -1001,10 +1000,10 @@ namespace libtorrent // prefer_whole_pieces can be set if this peer should download // whole pieces rather than trying to download blocks from the // same piece as other peers. - // the endpoint is the address of the peer we're picking pieces - // from. This is used when downloading whole pieces, to only - // pick from the same piece the same peer is downloading from. - // state is supposed to be set to fast if the peer is downloading + // the void* is the pointer to the policy::peer of the peer we're + // picking pieces from. This is used when downloading whole pieces, + // to only pick from the same piece the same peer is downloading + // from. state is supposed to be set to fast if the peer is downloading // relatively fast, by some notion. Slow peers will prefer not // to pick blocks from the same pieces as fast peers, and vice // versa. Downloading pieces are marked as being fast, medium @@ -1012,7 +1011,7 @@ namespace libtorrent void piece_picker::pick_pieces(const std::vector& pieces , std::vector& interesting_blocks , int num_blocks, bool prefer_whole_pieces - , tcp::endpoint peer, piece_state_t speed) const + , void* peer, piece_state_t speed) const { TORRENT_PIECE_PICKER_INVARIANT_CHECK; assert(num_blocks > 0); @@ -1063,17 +1062,25 @@ namespace libtorrent + (std::min)(num_blocks, (int)backup_blocks.size())); } + void piece_picker::clear_peer(void* peer) + { + for (std::vector::iterator i = m_block_info.begin() + , end(m_block_info.end()); i != end; ++i) + if (i->peer == peer) i->peer = 0; + } + namespace { bool exclusively_requested_from(piece_picker::downloading_piece const& p - , int num_blocks_in_piece, tcp::endpoint peer) + , int num_blocks_in_piece, void* peer) { for (int j = 0; j < num_blocks_in_piece; ++j) { piece_picker::block_info const& info = p.info[j]; if (info.state != piece_picker::block_info::state_none && info.peer != peer - && info.peer != tcp::endpoint()) + && info.peer != 0 + && static_cast(info.peer)->connection) { return false; } @@ -1087,7 +1094,7 @@ namespace libtorrent , std::vector& interesting_blocks , std::vector& backup_blocks , int num_blocks, bool prefer_whole_pieces - , tcp::endpoint peer, piece_state_t speed) const + , void* peer, piece_state_t speed) const { // if we have less than 1% of the pieces, ignore speed priorities and just try // to finish any downloading piece @@ -1281,7 +1288,7 @@ namespace libtorrent void piece_picker::mark_as_downloading(piece_block block - , const tcp::endpoint& peer, piece_state_t state) + , void* peer, piece_state_t state) { TORRENT_PIECE_PICKER_INVARIANT_CHECK; @@ -1330,7 +1337,7 @@ namespace libtorrent *j = i->peer_count; } - void piece_picker::mark_as_writing(piece_block block, tcp::endpoint const& peer) + void piece_picker::mark_as_writing(piece_block block, void* peer) { TORRENT_PIECE_PICKER_INVARIANT_CHECK; @@ -1367,7 +1374,7 @@ namespace libtorrent = std::find_if(m_downloads.begin(), m_downloads.end(), has_index(block.piece_index)); assert(i != m_downloads.end()); block_info& info = i->info[block.block_index]; - info.peer == peer; + info.peer = peer; assert(info.state == block_info::state_requested); if (info.state == block_info::state_requested) --i->requested; assert(i->requested >= 0); @@ -1384,7 +1391,7 @@ namespace libtorrent } } - void piece_picker::mark_as_finished(piece_block block, tcp::endpoint const& peer) + void piece_picker::mark_as_finished(piece_block block, void* peer) { assert(block.piece_index >= 0); assert(block.block_index >= 0); @@ -1397,7 +1404,7 @@ namespace libtorrent { TORRENT_PIECE_PICKER_INVARIANT_CHECK; - assert(peer == tcp::endpoint()); + assert(peer == 0); int prio = p.priority(m_sequenced_download_threshold); p.downloading = 1; if (prio > 0) move(prio, p.index); @@ -1424,7 +1431,7 @@ namespace libtorrent block_info& info = i->info[block.block_index]; info.peer = peer; assert(info.state == block_info::state_writing - || peer == tcp::endpoint()); + || peer == 0); if (info.state == block_info::state_writing) --i->writing; assert(i->writing >= 0); ++i->finished; @@ -1439,7 +1446,7 @@ namespace libtorrent } } - void piece_picker::get_downloaders(std::vector& d, int index) const + void piece_picker::get_downloaders(std::vector& d, int index) const { assert(index >= 0 && index <= (int)m_piece_map.size()); std::vector::const_iterator i @@ -1453,22 +1460,21 @@ namespace libtorrent } } - boost::optional piece_picker::get_downloader(piece_block block) const + void* piece_picker::get_downloader(piece_block block) const { std::vector::const_iterator i = std::find_if( m_downloads.begin() , m_downloads.end() , has_index(block.piece_index)); - if (i == m_downloads.end()) - return boost::optional(); + if (i == m_downloads.end()) return 0; assert(block.block_index >= 0); if (i->info[block.block_index].state == block_info::state_none) - return boost::optional(); + return 0; - return boost::optional(i->info[block.block_index].peer); + return i->info[block.block_index].peer; } void piece_picker::abort_download(piece_block block) @@ -1505,7 +1511,7 @@ namespace libtorrent --i->requested; // clear the downloader of this block - i->info[block.block_index].peer = tcp::endpoint(); + i->info[block.block_index].peer = 0; // if there are no other blocks in this piece // that's being downloaded, remove it from the list diff --git a/src/policy.cpp b/src/policy.cpp index eca5ba613..e40374784 100755 --- a/src/policy.cpp +++ b/src/policy.cpp @@ -54,6 +54,7 @@ POSSIBILITY OF SUCH DAMAGE. #include "libtorrent/invariant_check.hpp" #include "libtorrent/time.hpp" #include "libtorrent/aux_/session_impl.hpp" +#include "libtorrent/piece_picker.hpp" namespace libtorrent { @@ -233,7 +234,7 @@ namespace libtorrent // for this peer. If we're downloading one piece in 20 seconds // then use this mode. p.pick_pieces(c.get_bitfield(), interesting_pieces - , num_requests, prefer_whole_pieces, c.remote(), state); + , num_requests, prefer_whole_pieces, c.peer_info_struct(), state); // this vector is filled with the interesting pieces // that some other peer is currently downloading @@ -680,6 +681,10 @@ namespace libtorrent if (m_torrent->is_paused()) return; + piece_picker* p = 0; + if (m_torrent->has_picker()) + p = &m_torrent->picker(); + ptime now = time_now(); // remove old disconnected peers from the list for (iterator i = m_peers.begin(); i != m_peers.end();) @@ -689,6 +694,7 @@ namespace libtorrent && i->connected != min_time() && now - i->connected > minutes(120)) { + if (p) p->clear_peer(&(*i)); m_peers.erase(i++); } else @@ -1419,6 +1425,33 @@ namespace libtorrent ++num_torrent_peers; } + if (m_torrent->has_picker()) + { + piece_picker& p = m_torrent->picker(); + std::vector downloaders = p.get_download_queue(); + + std::set peer_set; + std::vector peers; + for (std::vector::iterator i = downloaders.begin() + , end(downloaders.end()); i != end; ++i) + { + p.get_downloaders(peers, i->index); + std::copy(peers.begin(), peers.end() + , std::insert_iterator >(peer_set, peer_set.begin())); + } + + for (std::set::iterator i = peer_set.begin() + , end(peer_set.end()); i != end; ++i) + { + policy::peer* p = static_cast(*i); + if (p == 0) continue; + std::list::const_iterator k = m_peers.begin(); + for (; k != m_peers.end(); ++k) + if (&(*k) == p) break; + assert(k != m_peers.end()); + } + } + // this invariant is a bit complicated. // the usual case should be that connected_peers // == num_torrent_peers. But when there's an incoming diff --git a/src/torrent.cpp b/src/torrent.cpp index d173f8943..9a611502c 100755 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -914,13 +914,13 @@ namespace libtorrent // increase the total amount of failed bytes m_total_failed_bytes += m_torrent_file.piece_size(index); - std::vector downloaders; + std::vector downloaders; m_picker->get_downloaders(downloaders, index); // decrease the trust point of all peers that sent // parts of this piece. // first, build a set of all peers that participated - std::set peers; + std::set peers; std::copy(downloaders.begin(), downloaders.end(), std::inserter(peers, peers.begin())); #ifndef TORRENT_DISABLE_EXTENSIONS @@ -931,19 +931,21 @@ namespace libtorrent } #endif - for (std::set::iterator i = peers.begin() + for (std::set::iterator i = peers.begin() , end(peers.end()); i != end; ++i) { - peer_iterator p = m_connections.find(*i); - if (p == m_connections.end()) continue; - peer_connection& peer = *p->second; - peer.received_invalid_data(index); + policy::peer* p = static_cast(*i); + if (p == 0) continue; +#ifndef NDEBUG + peer_iterator pi = m_connections.find(p->ip); + assert((pi != m_connections.end()) == bool(p->connection)); +#endif + if (p->connection) p->connection->received_invalid_data(index); // either, we have received too many failed hashes // or this was the only peer that sent us this piece. // TODO: make this a changable setting - if ((peer.peer_info_struct() - && peer.peer_info_struct()->trust_points <= -7) + if (p->trust_points <= -7 || peers.size() == 1) { // we don't trust this peer anymore @@ -951,31 +953,22 @@ namespace libtorrent if (m_ses.m_alerts.should_post(alert::info)) { m_ses.m_alerts.post_alert(peer_ban_alert( - p->first + p->ip , get_handle() , "banning peer because of too many corrupt pieces")); } // mark the peer as banned - policy::peer* peerinfo = p->second->peer_info_struct(); - if (peerinfo) - { - peerinfo->banned = true; - } - else - { - // it might be a web seed - if (web_peer_connection const* wpc - = dynamic_cast(p->second)) - { - remove_url_seed(wpc->url()); - } - } + p->banned = true; + if (p->connection) + { #if defined(TORRENT_VERBOSE_LOGGING) - (*p->second->m_logger) << "*** BANNING PEER 'too many corrupt pieces'\n"; + (*p->connection->m_logger) << "*** BANNING PEER [ " << p->ip + << " ] 'too many corrupt pieces'\n"; #endif - p->second->disconnect(); + p->connection->disconnect(); + } } } @@ -1028,12 +1021,12 @@ namespace libtorrent assert(index >= 0); assert(index < m_torrent_file.num_pieces()); - std::vector downloaders; + std::vector downloaders; m_picker->get_downloaders(downloaders, index); // increase the trust point of all peers that sent // parts of this piece. - std::set peers; + std::set peers; std::copy(downloaders.begin(), downloaders.end(), std::inserter(peers, peers.begin())); if (!m_have_pieces[index]) @@ -1047,12 +1040,16 @@ namespace libtorrent for (peer_iterator i = m_connections.begin(); i != m_connections.end(); ++i) try { i->second->announce_piece(index); } catch (std::exception&) {} - for (std::set::iterator i = peers.begin() + for (std::set::iterator i = peers.begin() , end(peers.end()); i != end; ++i) { - peer_iterator p = m_connections.find(*i); - if (p == m_connections.end()) continue; - p->second->received_valid_data(index); + policy::peer* p = static_cast(*i); + if (p == 0) continue; + p->on_parole = false; + ++p->trust_points; + // TODO: make this limit user settable + if (p->trust_points > 20) p->trust_points = 20; + if (p->connection) p->connection->received_valid_data(index); } #ifndef TORRENT_DISABLE_EXTENSIONS diff --git a/src/torrent_handle.cpp b/src/torrent_handle.cpp index c9ade14e9..b10d0f713 100755 --- a/src/torrent_handle.cpp +++ b/src/torrent_handle.cpp @@ -773,7 +773,11 @@ namespace libtorrent pi.blocks_in_piece = p.blocks_in_piece(i->index); for (int j = 0; j < pi.blocks_in_piece; ++j) { - pi.blocks[j].peer = i->info[j].peer; + if (i->info[j].peer == 0) + pi.blocks[j].peer = tcp::endpoint(); + else + pi.blocks[j].peer = static_cast(i->info[j].peer)->ip; + pi.blocks[j].num_downloads = i->info[j].num_downloads; pi.blocks[j].state = i->info[j].state; } diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 9c5a6c447..4b6788c3b 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -1,4 +1,5 @@ #include "libtorrent/piece_picker.hpp" +#include "libtorrent/policy.hpp" #include "test.hpp" @@ -9,6 +10,8 @@ int test_main() using namespace libtorrent; { + policy::peer peer_struct(tcp::endpoint(), policy::peer::connectable, 0); + const int num_pieces = 6; // 4 blocks per piece @@ -94,21 +97,21 @@ int test_main() std::vector picked; picked.clear(); - p.pick_pieces(peer1, picked, 1, false, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer1, picked, 1, false, 0, piece_picker::fast); TEST_CHECK(picked.size() == 1); TEST_CHECK(picked.front().piece_index == 2); // now pick a piece from peer2. The block is supposed to be // from piece 3, since it is the rarest piece that peer has. picked.clear(); - p.pick_pieces(peer2, picked, 1, false, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer2, picked, 1, false, 0, piece_picker::fast); TEST_CHECK(picked.size() == 1); TEST_CHECK(picked.front().piece_index == 3); // same thing for peer3. picked.clear(); - p.pick_pieces(peer3, picked, 1, false, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer3, picked, 1, false, 0, piece_picker::fast); TEST_CHECK(picked.size() == 1); TEST_CHECK(picked.front().piece_index == 5); @@ -118,7 +121,7 @@ int test_main() p.inc_refcount(1); picked.clear(); - p.pick_pieces(peer3, picked, 1, false, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer3, picked, 1, false, 0, piece_picker::fast); TEST_CHECK(picked.size() == 1); TEST_CHECK(picked.front().piece_index == 1); // and the block picked should not be 0 or 2 @@ -142,12 +145,9 @@ int test_main() // we have block 0 and 2 already, so we can't mark // them as begin downloaded. - p.mark_as_downloading(piece_block(1, 1), tcp::endpoint( - address::from_string("1.1.1.1"), 0), piece_picker::fast); - p.mark_as_downloading(piece_block(1, 3), tcp::endpoint( - address::from_string("1.1.1.1"), 0), piece_picker::fast); - p.mark_as_downloading(piece_block(2, 0), tcp::endpoint( - address::from_string("1.1.1.1"), 0), piece_picker::fast); + p.mark_as_downloading(piece_block(1, 1), &peer_struct, piece_picker::fast); + p.mark_as_downloading(piece_block(1, 3), &peer_struct, piece_picker::fast); + p.mark_as_downloading(piece_block(2, 0), &peer_struct, piece_picker::fast); std::vector const& downloads = p.get_download_queue(); TEST_CHECK(downloads.size() == 2); @@ -169,7 +169,7 @@ int test_main() TEST_CHECK(!p.is_requested(piece_block(2, 1))); picked.clear(); - p.pick_pieces(peer1, picked, 1, false, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer1, picked, 1, false, 0, piece_picker::fast); TEST_CHECK(picked.size() == 2); piece_block expected3[] = { piece_block(2, 0), piece_block(2, 1) }; @@ -182,7 +182,7 @@ int test_main() // partially selected) picked.clear(); - p.pick_pieces(peer1, picked, 1, true, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer1, picked, 1, true, 0, piece_picker::fast); // it will pick 4 blocks, since we said we // wanted whole pieces. @@ -190,8 +190,8 @@ int test_main() piece_block expected4[] = { - piece_block(3, 0), piece_block(3, 1) - , piece_block(3, 2), piece_block(3, 3) + piece_block(2, 0), piece_block(2, 1) + , piece_block(2, 2), piece_block(2, 3) }; TEST_CHECK(std::equal(picked.begin() , picked.end(), expected4)); @@ -200,18 +200,18 @@ int test_main() // to make sure it can still fall back on partial pieces picked.clear(); - p.pick_pieces(peer1, picked, 100, true, tcp::endpoint(), piece_picker::fast); + p.pick_pieces(peer1, picked, 100, true, 0, piece_picker::fast); TEST_CHECK(picked.size() == 12); piece_block expected5[] = { - piece_block(3, 0), piece_block(3, 1) + piece_block(2, 0), piece_block(2, 1) + , piece_block(2, 2), piece_block(2, 3) + , piece_block(3, 0), piece_block(3, 1) , piece_block(3, 2), piece_block(3, 3) , piece_block(5, 0), piece_block(5, 1) , piece_block(5, 2), piece_block(5, 3) - , piece_block(2, 0), piece_block(2, 1) - , piece_block(2, 2), piece_block(2, 3) }; TEST_CHECK(std::equal(picked.begin() @@ -221,8 +221,7 @@ int test_main() // to make sure it can still fall back on partial pieces picked.clear(); - p.pick_pieces(peer1, picked, 100, true - , tcp::endpoint(address::from_string("1.1.1.1"), 0), piece_picker::fast); + p.pick_pieces(peer1, picked, 100, true, &peer_struct, piece_picker::fast); TEST_CHECK(picked.size() == 11); @@ -241,7 +240,7 @@ int test_main() // make sure the piece picker allows filtered pieces // to become available - p.mark_as_finished(piece_block(4, 2), tcp::endpoint()); + p.mark_as_finished(piece_block(4, 2), 0); } return 0;