diff --git a/Jamfile b/Jamfile index 5a66d780b..09fc26e27 100755 --- a/Jamfile +++ b/Jamfile @@ -28,14 +28,10 @@ VERSION = 1.1.0 ; rule coverage ( properties * ) { local result ; - if ! on - { - return $(result) ; - } - - if gcc in $(properties) + if on in $(properties) + && ( gcc in $(properties) || darwin in $(properties) - || clang in $(properties) + || clang in $(properties) ) { result += -fprofile-arcs -ftest-coverage ; diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index bdbc1d253..634052391 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -173,6 +173,8 @@ namespace libtorrent // info about each block // this is a pointer into the m_block_info // vector owned by the piece_picker + // TODO: 2 if we could make this an index into m_block_info instead + // we would save at least 4 or 5 bytes block_info* info; // the index of the piece @@ -366,8 +368,8 @@ namespace libtorrent void piece_passed(int index); - void mark_as_checking(int index); - void mark_as_done_checking(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; @@ -405,10 +407,6 @@ namespace libtorrent // the hash-check yet int unverified_blocks() const; - // return the peer pointers for all blocks that are currently - // in requested state (i.e. requested but not received) - void get_requestors(std::vector& d, int index) const; - // return the peer pointers to all peers that participated in // this piece void get_downloaders(std::vector& d, int index) const; diff --git a/src/peer_connection.cpp b/src/peer_connection.cpp index d9e71575e..446c0e8d7 100644 --- a/src/peer_connection.cpp +++ b/src/peer_connection.cpp @@ -2790,7 +2790,7 @@ namespace libtorrent if (st.requested > 0 && st.writing + st.finished + st.requested == num_blocks) { std::vector d; - t->picker().get_requestors(d, piece); + t->picker().get_downloaders(d, piece); if (d.size() == 1) { // only make predictions if all remaining diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index 4c0c68d7c..2cb11e7b0 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -3425,7 +3425,9 @@ get_out: void piece_picker::mark_as_canceled(piece_block block, void* peer) { #ifdef TORRENT_PICKER_LOG - std::cerr << "[" << this << "] " << "mark_as_cancelled( {" << block.piece_index << ", " << block.block_index << "} )" << std::endl; + std::cerr << "[" << this << "] " << "mark_as_cancelled( {" + << block.piece_index << ", " << block.block_index + << "} )" << std::endl; #endif #if TORRENT_USE_INVARIANT_CHECKS @@ -3458,6 +3460,21 @@ get_out: { --i->writing; info.state = block_info::state_none; + // i may be invalid after this call + i = update_piece_state(i); + + if (i->finished + i->writing + i->requested == 0) + { + piece_pos& p = m_piece_map[block.piece_index]; + int prev_priority = p.priority(this); + erase_download_piece(i); + int new_priority = p.priority(this); + + if (m_dirty) return; + if (new_priority == prev_priority) return; + if (prev_priority == -1) add(block.piece_index); + else update(prev_priority, p.index); + } } else { @@ -3569,48 +3586,26 @@ get_out: } - // TODO: 3 this doesn't appear to be used for anything. Can it be removed - // or should it be used to plug some race when requesting and checking pieces? +/* 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_requestors(std::vector& d, int index) const - { - TORRENT_ASSERT(index >= 0 && index <= (int)m_piece_map.size()); - - int state = m_piece_map[index].download_queue(); - TORRENT_ASSERT(state != piece_pos::piece_open); - std::vector::const_iterator i = find_dl_piece(state, index); - TORRENT_ASSERT(i != m_downloads[state].end()); - - d.clear(); - for (int j = 0, end(blocks_in_piece(index)); j != end; ++j) - { - if (i->info[j].state != block_info::state_requested) continue; - if (i->info[j].peer == 0) continue; - d.push_back(i->info[j].peer); - } - } void piece_picker::get_downloaders(std::vector& d, int index) const { @@ -3618,18 +3613,21 @@ get_out: d.clear(); int state = m_piece_map[index].download_queue(); + int num_blocks = blocks_in_piece(index); + d.reserve(num_blocks); + if (state == piece_pos::piece_open) { - int num_blocks = blocks_in_piece(index); for (int i = 0; i < num_blocks; ++i) d.push_back(0); return; } - std::vector::const_iterator i = find_dl_piece(state, index); + std::vector::const_iterator i + = find_dl_piece(state, index); TORRENT_ASSERT(i != m_downloads[state].end()); TORRENT_ASSERT(i->info >= &m_block_info[0] && i->info < &m_block_info[0] + m_block_info.size()); - for (int j = 0, end(blocks_in_piece(index)); j != end; ++j) + for (int j = 0; j != num_blocks; ++j) { TORRENT_ASSERT(i->info[j].peer == 0 || static_cast(i->info[j].peer)->in_use); d.push_back(i->info[j].peer); diff --git a/src/torrent.cpp b/src/torrent.cpp index fe98cafa3..221b1a567 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -3897,7 +3897,7 @@ namespace libtorrent TORRENT_ASSERT(!m_picker->have_piece(j->piece)); - picker().mark_as_done_checking(j->piece); +// picker().mark_as_done_checking(j->piece); state_updated(); @@ -10824,7 +10824,7 @@ namespace libtorrent // adds a piece void torrent::verify_piece(int piece) { - picker().mark_as_checking(piece); +// picker().mark_as_checking(piece); inc_refcount("verify_piece"); m_ses.disk_thread().async_hash(m_storage.get(), piece, 0 diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 19c6d7b85..eef7e93bb 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -1550,18 +1550,81 @@ int test_main() TEST_EQUAL(p->has_piece_passed(1), false); TEST_EQUAL(p->have_piece(1), false); -// ======================================================== - // make sure write_failed() and lock_piece() actually // locks the piece, and that it won't be picked. // also make sure restore_piece() unlocks it and makes // it available for picking again. - // test mark_as_canceled() + picked = pick_pieces(p, " * ", 1, blocks_per_piece, 0); + TEST_EQUAL(picked.size(), 0); - // test get_download_queue() + p->restore_piece(1); - // test get_requestors() (similar to get_downloaders()) + picked = pick_pieces(p, " * ", 1, blocks_per_piece, 0); + TEST_EQUAL(picked.size(), blocks_per_piece); + + // locking pieces only works on partial pieces + p->mark_as_writing(piece_block(1, 0), &tmp1); + p->lock_piece(1); + + picked = pick_pieces(p, " * ", 1, blocks_per_piece, 0); + TEST_EQUAL(picked.size(), 0); + +// ======================================================== + + print_title("test write_failed (clear piece)"); + + p = setup_picker("1111111", "* * ", "1101111", ""); + + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 0); + + p->mark_as_writing(piece_block(1, 0), &tmp1); + + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 1); + + p->write_failed(piece_block(1, 0)); + + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 0); + +// ======================================================== + + print_title("test mark_as_canceled"); + + p = setup_picker("1111111", "* * ", "1101111", ""); + + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 0); + + p->mark_as_writing(piece_block(1, 0), &tmp1); + + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 1); + + p->mark_as_canceled(piece_block(1, 0), &tmp1); + stat = p->piece_stats(1); + TEST_EQUAL(stat.downloading, 0); + +// ======================================================== + + print_title("test get_download_queue"); + + p = setup_picker("1111111", " ", "1101111", "0327000"); + + std::vector downloads + = p->get_download_queue(); + + // the download queue should have piece 1, 2 and 3 in it + TEST_EQUAL(downloads.size(), 3); + + TEST_CHECK(std::find_if(downloads.begin(), downloads.end() + , boost::bind(&piece_picker::downloading_piece::index, _1) == 1) != downloads.end()); + TEST_CHECK(std::find_if(downloads.begin(), downloads.end() + , boost::bind(&piece_picker::downloading_piece::index, _1) == 2) != downloads.end()); + TEST_CHECK(std::find_if(downloads.begin(), downloads.end() + , boost::bind(&piece_picker::downloading_piece::index, _1) == 3) != downloads.end()); /*