From 058419a77c7f0c64b77c5259fdecf931ec3fb103 Mon Sep 17 00:00:00 2001 From: arvidn Date: Tue, 7 Aug 2018 22:23:13 +0200 Subject: [PATCH] simplify total_have/have_want/total_want. make piece_picker track pad blocks and compute byte-progress at block granularity --- include/libtorrent/piece_picker.hpp | 62 +++++- include/libtorrent/torrent.hpp | 13 +- include/libtorrent/torrent_handle.hpp | 2 + include/libtorrent/torrent_status.hpp | 6 + src/piece_picker.cpp | 97 ++++++++- src/torrent.cpp | 285 ++++++-------------------- test/test_piece_picker.cpp | 99 +++++---- test/test_torrent.cpp | 49 +++++ 8 files changed, 335 insertions(+), 278 deletions(-) diff --git a/include/libtorrent/piece_picker.hpp b/include/libtorrent/piece_picker.hpp index 8b95a71f8..faab92161 100644 --- a/include/libtorrent/piece_picker.hpp +++ b/include/libtorrent/piece_picker.hpp @@ -69,6 +69,17 @@ namespace libtorrent { using picker_options_t = flags::bitfield_flag; using download_queue_t = aux::strong_typedef; + struct piece_count + { + // the number of pieces included in the "set" + int num_pieces; + // the number of blocks, out of those pieces, that are pad + // blocks (i.e. entirely part of pad files) + int pad_blocks; + // true if the last piece is part of the set + bool last_piece; + }; + class TORRENT_EXTRA_EXPORT piece_picker { public: @@ -375,16 +386,38 @@ namespace libtorrent { torrent_peer* get_downloader(piece_block block) const; - // the number of filtered pieces we don't have - int num_filtered() const { return m_num_filtered; } - // the number of filtered pieces we already have - int num_have_filtered() const { return m_num_have_filtered; } + // piece states + // + // have: ----------- + // pieces: # # # # # # # # # # # + // filtered: ------- + // pads blk: ^ ^ ^ + // + // want-have: * * * * + // want: * * * * * * * + // total-have: * * * * * * + // + // we only care about: + // 1. pieces we have (less pad blocks we have) + // 2. pieces we have AND want (less pad blocks we have and want) + // 3. pieces we want (less pad blocks we want) - // number of pieces whose hash has passed _and_ they have - // been successfully flushed to disk. Including pieces we have - // also filtered with priority 0 but have anyway. - int num_have() const { return m_num_have; } + // number of pieces not filtered, as well as the number of + // blocks out of those pieces that are pad blocks. + // ``last_piece`` is set if the last piece is one of the + // pieces. + piece_count want() const; + + // number of pieces we have out of the ones we have not filtered + piece_count have_want() const; + + // number of pieces we have (regardless of whether they are filtered) + piece_count have() const; + + piece_count all_pieces() const; + + int pad_blocks_in_piece(piece_index_t const index) const; // number of pieces whose hash has passed (but haven't necessarily // been flushed to disk yet) @@ -431,6 +464,8 @@ namespace libtorrent { private: + int num_pad_blocks() const { return int(m_pad_blocks.size()); } + aux::typed_span mutable_blocks_for_piece(downloading_piece const& dp); std::tuple requested_from( @@ -695,6 +730,15 @@ namespace libtorrent { // TODO: this could be a much more efficient data structure std::set m_pad_blocks; + // the number of pad blocks that we already have + int m_have_pad_blocks = 0; + + // the number of pad blocks part of filtered pieces we don't have + int m_filtered_pad_blocks = 0; + + // the number of pad blocks we have that are also filtered + int m_have_filtered_pad_blocks = 0; + // the number of seeds. These are not added to // the availability counters of the pieces int m_seeds = 0; @@ -742,6 +786,8 @@ namespace libtorrent { // have. total_number_of_pieces - number_of_pieces_we_have // - num_filtered is supposed to the number of pieces // we still want to download + // TODO: it would be more intuitive to account "wanted" pieces + // instead of filtered int m_num_filtered = 0; // the number of pieces we have that also are filtered diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 012f43c79..bd37281f1 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -113,6 +113,8 @@ namespace libtorrent { , max }; + TORRENT_EXTRA_EXPORT std::int64_t calc_bytes(file_storage const& fs, piece_count const& pc); + struct time_critical_piece { // when this piece was first requested @@ -481,8 +483,8 @@ namespace libtorrent { stat statistics() const { return m_stat; } boost::optional bytes_left() const; - int block_bytes_wanted(piece_block const& p) const; - void bytes_done(torrent_status& st, bool accurate) const; + + void bytes_done(torrent_status& st, status_flags_t) const; void sent_bytes(int bytes_payload, int bytes_protocol); void received_bytes(int bytes_payload, int bytes_protocol); @@ -825,7 +827,7 @@ namespace libtorrent { } return has_picker() - ? m_picker->num_have() + ? m_picker->have().num_pieces : m_have_all ? m_torrent_file->num_pieces() : 0; } @@ -1600,8 +1602,9 @@ namespace libtorrent { // ---- - // the number of bytes of padding files - std::uint32_t m_padding:24; + // the number of (16kiB) blocks that fall entirely in pad files + // i.e. blocks that we consider we have on start-up + std::uint16_t m_padding_blocks = 0; // this is set to the connect boost quota for this torrent. // After having received this many priority peer connection attempts, it diff --git a/include/libtorrent/torrent_handle.hpp b/include/libtorrent/torrent_handle.hpp index 618a319fe..e5aa28ee7 100644 --- a/include/libtorrent/torrent_handle.hpp +++ b/include/libtorrent/torrent_handle.hpp @@ -304,9 +304,11 @@ namespace aux { // calculates ``distributed_copies``, ``distributed_full_copies`` and // ``distributed_fraction``. static constexpr status_flags_t query_distributed_copies = 0_bit; + // includes partial downloaded blocks in ``total_done`` and // ``total_wanted_done``. static constexpr status_flags_t query_accurate_download_counters = 1_bit; + // includes ``last_seen_complete``. static constexpr status_flags_t query_last_seen_complete = 2_bit; // populate the ``pieces`` field in torrent_status. diff --git a/include/libtorrent/torrent_status.hpp b/include/libtorrent/torrent_status.hpp index d877ed1ff..fc8b425b3 100644 --- a/include/libtorrent/torrent_status.hpp +++ b/include/libtorrent/torrent_status.hpp @@ -250,6 +250,12 @@ TORRENT_VERSION_NAMESPACE_2 // ``total_payload_download``). std::int64_t total_done = 0; + // the total number of bytes to download for this torrent. This + // may be less than the size of the torrent in case there are + // pad files. This number only counts bytes that will actually + // be requested from peers. + std::int64_t total = 0; + // the number of bytes we have downloaded, only counting the pieces that // we actually want to download. i.e. excluding any pieces that we have // but have priority 0 (i.e. not wanted). diff --git a/src/piece_picker.cpp b/src/piece_picker.cpp index 37981200a..f7368b078 100644 --- a/src/piece_picker.cpp +++ b/src/piece_picker.cpp @@ -122,6 +122,9 @@ namespace libtorrent { m_num_filtered += m_num_have_filtered; m_num_have_filtered = 0; m_num_have = 0; + m_have_pad_blocks = 0; + m_filtered_pad_blocks = 0; + m_have_filtered_pad_blocks = 0; m_num_passed = 0; m_dirty = true; for (auto& m : m_piece_map) @@ -445,8 +448,18 @@ namespace libtorrent { { TORRENT_ASSERT(m_num_have >= 0); TORRENT_ASSERT(m_num_have_filtered >= 0); + TORRENT_ASSERT(m_num_have_filtered <= m_num_have); + TORRENT_ASSERT(m_num_have_filtered + m_num_filtered <= num_pieces()); TORRENT_ASSERT(m_num_filtered >= 0); TORRENT_ASSERT(m_seeds >= 0); + TORRENT_ASSERT(m_have_pad_blocks <= num_pad_blocks()); + TORRENT_ASSERT(m_have_pad_blocks >= 0); + TORRENT_ASSERT(m_filtered_pad_blocks <= num_pad_blocks()); + TORRENT_ASSERT(m_filtered_pad_blocks >= 0); + TORRENT_ASSERT(m_have_filtered_pad_blocks <= num_pad_blocks()); + TORRENT_ASSERT(m_have_filtered_pad_blocks >= 0); + TORRENT_ASSERT(m_have_filtered_pad_blocks + m_filtered_pad_blocks <= num_pad_blocks()); + TORRENT_ASSERT(m_have_filtered_pad_blocks <= m_have_pad_blocks); // make sure the priority boundaries are monotonically increasing. The // difference between two cursors cannot be negative, but ranges are @@ -607,6 +620,9 @@ namespace libtorrent { int num_filtered = 0; int num_have_filtered = 0; int num_have = 0; + int num_have_pad_blocks = 0; + int num_filtered_pad_blocks = 0; + int num_have_filtered_pad_blocks = 0; piece_index_t piece(0); for (auto i = m_piece_map.begin(); i != m_piece_map.end(); ++i, ++piece) { @@ -615,16 +631,25 @@ namespace libtorrent { if (p.filtered()) { if (p.index != piece_pos::we_have_index) + { ++num_filtered; + num_filtered_pad_blocks += pad_blocks_in_piece(piece); + } else + { ++num_have_filtered; + num_have_filtered_pad_blocks += pad_blocks_in_piece(piece); + } } #ifdef TORRENT_DEBUG_REFCOUNTS TORRENT_ASSERT(int(p.have_peers.size()) == p.peer_count + m_seeds); #endif if (p.index == piece_pos::we_have_index) + { ++num_have; + num_have_pad_blocks += pad_blocks_in_piece(piece); + } if (p.index == piece_pos::we_have_index) { @@ -637,7 +662,6 @@ namespace libtorrent { int const prio = p.priority(this); -#if TORRENT_USE_ASSERTS if (p.downloading()) { if (p.reverse()) @@ -649,7 +673,6 @@ namespace libtorrent { { TORRENT_ASSERT(prio == -1 || (prio % piece_picker::prio_factor == 1)); } -#endif if (!m_dirty) { @@ -714,6 +737,9 @@ namespace libtorrent { TORRENT_ASSERT(num_have == m_num_have); TORRENT_ASSERT(num_filtered == m_num_filtered); TORRENT_ASSERT(num_have_filtered == m_num_have_filtered); + TORRENT_ASSERT(num_have_pad_blocks == m_have_pad_blocks); + TORRENT_ASSERT(num_filtered_pad_blocks == m_filtered_pad_blocks); + TORRENT_ASSERT(num_have_filtered_pad_blocks == m_have_filtered_pad_blocks); if (!m_dirty) { @@ -1588,7 +1614,12 @@ namespace libtorrent { --m_num_passed; if (p.filtered()) { + m_filtered_pad_blocks += pad_blocks_in_piece(index); ++m_num_filtered; + + TORRENT_ASSERT(m_have_filtered_pad_blocks >= pad_blocks_in_piece(index)); + m_have_filtered_pad_blocks -= pad_blocks_in_piece(index); + TORRENT_ASSERT(m_num_have_filtered > 0); --m_num_have_filtered; } else @@ -1604,6 +1635,8 @@ namespace libtorrent { } --m_num_have; + m_have_pad_blocks -= pad_blocks_in_piece(index); + TORRENT_ASSERT(m_have_pad_blocks >= 0); p.set_not_have(); if (m_dirty) return; @@ -1643,11 +1676,18 @@ namespace libtorrent { if (p.filtered()) { + TORRENT_ASSERT(m_filtered_pad_blocks >= pad_blocks_in_piece(index)); + m_filtered_pad_blocks -= pad_blocks_in_piece(index); + TORRENT_ASSERT(m_num_filtered > 0); --m_num_filtered; + + m_have_filtered_pad_blocks += pad_blocks_in_piece(index); ++m_num_have_filtered; } ++m_num_have; ++m_num_passed; + m_have_pad_blocks += pad_blocks_in_piece(index); + TORRENT_ASSERT(m_have_pad_blocks <= num_pad_blocks()); p.set_have(); if (m_cursor == prev(m_reverse_cursor) && m_cursor == index) @@ -1711,10 +1751,12 @@ namespace libtorrent { // the piece just got filtered if (p.have()) { + m_have_filtered_pad_blocks += pad_blocks_in_piece(index); ++m_num_have_filtered; } else { + m_filtered_pad_blocks += pad_blocks_in_piece(index); ++m_num_filtered; // update m_cursor @@ -1748,10 +1790,16 @@ namespace libtorrent { // the piece just got unfiltered if (p.have()) { + TORRENT_ASSERT(m_have_filtered_pad_blocks >= pad_blocks_in_piece(index)); + m_have_filtered_pad_blocks -= pad_blocks_in_piece(index); + TORRENT_ASSERT(m_num_have_filtered > 0); --m_num_have_filtered; } else { + TORRENT_ASSERT(m_filtered_pad_blocks >= pad_blocks_in_piece(index)); + m_filtered_pad_blocks -= pad_blocks_in_piece(index); + TORRENT_ASSERT(m_num_filtered > 0); --m_num_filtered; // update cursors if (index < m_cursor) m_cursor = index; @@ -3461,17 +3509,22 @@ get_out: 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 - using iter = std::set::const_iterator; - iter const begin = m_pad_blocks.lower_bound(piece_block(block.piece_index, 0)); int const blocks = blocks_in_piece(block.piece_index); - iter const end = m_pad_blocks.upper_bound(piece_block(block.piece_index, blocks)); - if (std::distance(begin, end) == blocks) + if (pad_blocks_in_piece(block.piece_index) == blocks) { // the entire piece is a pad file we_have(block.piece_index); } } + int piece_picker::pad_blocks_in_piece(piece_index_t const index) const + { + int const blocks = blocks_in_piece(index); + auto const begin = m_pad_blocks.lower_bound(piece_block(index, 0)); + auto const end = m_pad_blocks.upper_bound(piece_block(index, blocks)); + return int(std::distance(begin, end)); + } + void piece_picker::get_downloaders(std::vector& d , piece_index_t const index) const { @@ -3590,4 +3643,36 @@ get_out: i = update_piece_state(i); } + piece_count piece_picker::want() const + { + bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download; + return { num_pieces() - m_num_filtered - m_num_have_filtered + , num_pad_blocks() - m_filtered_pad_blocks - m_have_filtered_pad_blocks + , want_last }; + } + + piece_count piece_picker::have_want() const + { + bool const have_last = have_piece(piece_index_t(num_pieces() - 1)); + bool const want_last = piece_priority(piece_index_t(num_pieces() - 1)) != dont_download; + return { m_num_have - m_num_have_filtered + , m_have_pad_blocks - m_have_filtered_pad_blocks + , have_last && want_last }; + } + + piece_count piece_picker::have() const + { + bool const have_last = have_piece(piece_index_t(num_pieces() - 1)); + return { m_num_have + , m_have_pad_blocks + , have_last }; + } + + piece_count piece_picker::all_pieces() const + { + return { num_pieces() + , num_pad_blocks() + , true}; + } + } diff --git a/src/torrent.cpp b/src/torrent.cpp index bf2d949e5..540180cc7 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -211,7 +211,6 @@ bool is_downloading_state(int const st) , m_magnet_link(false) , m_apply_ip_filter(p.flags & torrent_flags::apply_ip_filter) , m_pending_active_change(false) - , m_padding(0) , m_connect_boost_counter(static_cast(settings().get_int(settings_pack::torrent_connect_boost))) , m_incomplete(0xffffff) , m_announce_to_dht(!(p.flags & torrent_flags::paused)) @@ -1807,7 +1806,6 @@ bool is_downloading_state(int const st) for (auto const i : fs.file_range()) { if (!fs.pad_file_at(i) || fs.file_size(i) == 0) continue; - m_padding += std::uint32_t(fs.file_size(i)); peer_request pr = m_torrent_file->map_file(i, 0, int(fs.file_size(i))); int off = pr.start & (block_size() - 1); @@ -1821,6 +1819,7 @@ bool is_downloading_state(int const st) { if (pb.block_index == blocks_per_piece) { pb.block_index = 0; ++pb.piece_index; } m_picker->mark_as_pad(pb); + ++m_padding_blocks; } // 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 @@ -1832,7 +1831,7 @@ bool is_downloading_state(int const st) } } - if (m_padding > 0) + if (m_padding_blocks > 0) { // if we marked an entire piece as finished, we actually // need to consider it finished @@ -3457,32 +3456,31 @@ bool is_downloading_state(int const st) return left; } - // returns the number of bytes we are interested - // in for the given block. This returns block_size() - // for all blocks except the last one (if it's smaller - // than block_size()) and blocks that overlap a padding - // file - int torrent::block_bytes_wanted(piece_block const& p) const + // we assume the last block is never a pad block. Should be a fairly + // safe assumption, and you just get a few kiB off if it is + std::int64_t calc_bytes(file_storage const& fs, piece_count const& pc) { - file_storage const& fs = m_torrent_file->files(); - int const piece_size = m_torrent_file->piece_size(p.piece_index); - int const offset = p.block_index * block_size(); - if (m_padding == 0) return std::min(piece_size - offset, block_size()); + // it's an impossible combination to have 0 pieces, but still have one of them be the last piece + TORRENT_ASSERT(!(pc.num_pieces == 0 && pc.last_piece == true)); - std::vector const files = fs.map_block( - p.piece_index, offset, std::min(piece_size - offset, block_size())); - std::int64_t ret = 0; - for (auto const& i : files) - { - if (fs.pad_file_at(i.file_index)) continue; - ret += i.size; - } - TORRENT_ASSERT(ret <= std::min(piece_size - offset, block_size())); - return aux::numeric_cast(ret); + // if we have 0 pieces, we can't have any pad blocks either + TORRENT_ASSERT(!(pc.num_pieces == 0 && pc.pad_blocks > 0)); + + // if we have all pieces, we must also have the last one + TORRENT_ASSERT(!(pc.num_pieces == fs.num_pieces() && pc.last_piece == false)); + int const block_size = std::min(default_block_size, fs.piece_length()); + + // every block should not be a pad block + TORRENT_ASSERT(pc.pad_blocks <= pc.num_pieces * fs.piece_length() / block_size); + + return std::int64_t(pc.num_pieces) * fs.piece_length() + - (pc.last_piece ? 1 : 0) * (fs.piece_length() - fs.piece_size(fs.last_piece())) + - std::int64_t(pc.pad_blocks) * block_size; } // fills in total_wanted, total_wanted_done and total_done - void torrent::bytes_done(torrent_status& st, bool const accurate) const +// TODO: 3 this could probably be pulled out into a free function + void torrent::bytes_done(torrent_status& st, status_flags_t const flags) const { INVARIANT_CHECK; @@ -3490,23 +3488,21 @@ bool is_downloading_state(int const st) st.total_wanted_done = 0; st.total_wanted = m_torrent_file->total_size(); - TORRENT_ASSERT(st.total_wanted >= m_padding); + TORRENT_ASSERT(st.total_wanted >= m_padding_blocks * default_block_size); TORRENT_ASSERT(st.total_wanted >= 0); - if (!valid_metadata() || m_torrent_file->num_pieces() == 0) - return; + TORRENT_ASSERT(!valid_metadata() || m_torrent_file->num_pieces() > 0); + if (!valid_metadata()) return; TORRENT_ASSERT(st.total_wanted >= std::int64_t(m_torrent_file->piece_length()) * (m_torrent_file->num_pieces() - 1)); - piece_index_t const last_piece = prev(m_torrent_file->end_piece()); - int const piece_size = m_torrent_file->piece_length(); - // if any piece hash fails, we'll be taken out of seed mode // and m_seed_mode will be false if (m_seed_mode || is_seed()) { - st.total_done = m_torrent_file->total_size() - m_padding; + st.total_done = m_torrent_file->total_size() + - m_padding_blocks * default_block_size; st.total_wanted_done = st.total_done; st.total_wanted = st.total_done; return; @@ -3515,84 +3511,34 @@ bool is_downloading_state(int const st) { st.total_done = 0; st.total_wanted_done = 0; - st.total_wanted = m_torrent_file->total_size() - m_padding; + st.total_wanted = m_torrent_file->total_size() + - m_padding_blocks * default_block_size; return; } - TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered()); - st.total_wanted_done = std::int64_t(num_have() - m_picker->num_have_filtered()) - * piece_size; + TORRENT_ASSERT(has_picker()); + + file_storage const& files = m_torrent_file->files(); + + st.total_wanted = calc_bytes(files, m_picker->want()); + st.total_wanted_done = calc_bytes(files, m_picker->have_want()); + st.total_done = calc_bytes(files, m_picker->have()); + st.total = calc_bytes(files, m_picker->all_pieces()); + + TORRENT_ASSERT(st.total_done <= calc_bytes(files, m_picker->all_pieces())); + TORRENT_ASSERT(st.total_wanted <= calc_bytes(files, m_picker->all_pieces())); + TORRENT_ASSERT(st.total_wanted_done >= 0); - - st.total_done = std::int64_t(num_passed()) * piece_size; - // if num_passed() == num_pieces(), we should be a seed, and taken the - // branch above - TORRENT_ASSERT(num_passed() <= m_torrent_file->num_pieces()); - - int num_filtered_pieces = m_picker->num_filtered() - + m_picker->num_have_filtered(); - piece_index_t const last_piece_index = m_torrent_file->last_piece(); - if (m_picker->piece_priority(last_piece_index) == dont_download) - { - st.total_wanted -= m_torrent_file->piece_size(last_piece_index); - TORRENT_ASSERT(st.total_wanted >= 0); - --num_filtered_pieces; - } - st.total_wanted -= std::int64_t(num_filtered_pieces) * piece_size; TORRENT_ASSERT(st.total_wanted >= 0); - - // if we have the last piece, we have to correct - // the amount we have, since the first calculation - // assumed all pieces were of equal size - if (m_picker->has_piece_passed(last_piece)) - { - TORRENT_ASSERT(st.total_done >= piece_size); - int const corr = m_torrent_file->piece_size(last_piece) - - piece_size; - TORRENT_ASSERT(corr <= 0); - TORRENT_ASSERT(corr > -piece_size); - st.total_done += corr; - if (m_picker->piece_priority(last_piece) != dont_download) - { - TORRENT_ASSERT(st.total_wanted_done >= -corr); - st.total_wanted_done += corr; - } - } TORRENT_ASSERT(st.total_wanted >= st.total_wanted_done); - - // this is expensive, we might not want to do it all the time - if (!accurate) return; - - // subtract padding files - if (m_padding > 0) - { - file_storage const& files = m_torrent_file->files(); - for (auto const i : files.file_range()) - { - if (!files.pad_file_at(i)) continue; - peer_request p = files.map_file(i, 0, int(files.file_size(i))); - for (piece_index_t j = p.piece; p.length > 0; ++j) - { - int const deduction = std::min(p.length, piece_size - p.start); - bool const done = m_picker->have_piece(j); - bool const wanted = m_picker->piece_priority(j) > dont_download; - if (done) st.total_done -= deduction; - if (wanted) st.total_wanted -= deduction; - if (wanted && done) st.total_wanted_done -= deduction; - TORRENT_ASSERT(st.total_done >= 0); - TORRENT_ASSERT(st.total_wanted >= 0); - TORRENT_ASSERT(st.total_wanted_done >= 0); - p.length -= piece_size - p.start; - p.start = 0; - ++p.piece; - } - } - } - - TORRENT_ASSERT(!accurate || st.total_done <= m_torrent_file->total_size() - m_padding); - TORRENT_ASSERT(st.total_wanted_done >= 0); + TORRENT_ASSERT(st.total_done >= 0); TORRENT_ASSERT(st.total_done >= st.total_wanted_done); + // this is expensive, we might not want to do it all the time + if (!(flags & torrent_handle::query_accurate_download_counters)) return; + + // to get higher accuracy of the download progress, include + // blocks from currently downloading pieces as well std::vector const dl_queue = m_picker->get_download_queue(); @@ -3600,120 +3546,22 @@ bool is_downloading_state(int const st) // blocks to our 'done' counter for (auto i = dl_queue.begin(); i != dl_queue.end(); ++i) { - int corr = 0; piece_index_t const index = i->index; + // completed pieces are already accounted for - if (m_picker->has_piece_passed(index)) continue; - TORRENT_ASSERT(i->finished <= m_picker->blocks_in_piece(index)); + if (m_picker->have_piece(index)) continue; -#if TORRENT_USE_ASSERTS - TORRENT_ASSERT(std::count_if(std::next(i), dl_queue.end() - , [index](piece_picker::downloading_piece const& p) { return p.index == index; }) == 0); -#endif + TORRENT_ASSERT(i->finished + i->writing <= m_picker->blocks_in_piece(index)); + TORRENT_ASSERT(i->finished + i->writing >= m_picker->pad_blocks_in_piece(index)); - int idx = -1; - for (auto const& info : m_picker->blocks_for_piece(*i)) - { - ++idx; -#ifdef TORRENT_EXPENSIVE_INVARIANT_CHECKS - TORRENT_ASSERT(m_picker->is_finished(piece_block(index, idx)) - == (info.state == piece_picker::block_info::state_finished)); -#endif - if (info.state == piece_picker::block_info::state_finished) - { - corr += block_bytes_wanted(piece_block(index, idx)); - } - TORRENT_ASSERT(corr >= 0); - TORRENT_ASSERT(index != last_piece || idx < m_picker->blocks_in_last_piece() - || info.state != piece_picker::block_info::state_finished); - } + int const blocks = i->finished + i->writing - m_picker->pad_blocks_in_piece(index); + TORRENT_ASSERT(blocks >= 0); - st.total_done += corr; + auto const additional_bytes = std::int64_t(blocks) * block_size(); + st.total_done += additional_bytes; if (m_picker->piece_priority(index) > dont_download) - st.total_wanted_done += corr; + st.total_wanted_done += additional_bytes; } - - TORRENT_ASSERT(st.total_wanted <= m_torrent_file->total_size() - m_padding); - TORRENT_ASSERT(st.total_done <= m_torrent_file->total_size() - m_padding); - TORRENT_ASSERT(st.total_wanted_done <= m_torrent_file->total_size() - m_padding); - TORRENT_ASSERT(st.total_wanted_done >= 0); - TORRENT_ASSERT(st.total_done >= st.total_wanted_done); - - std::map downloading_piece; - for (auto pc : *this) - { - piece_block_progress p = pc->downloading_piece_progress(); - if (p.piece_index == piece_block_progress::invalid_index) - continue; - - if (m_picker->has_piece_passed(p.piece_index)) - continue; - - piece_block block(p.piece_index, p.block_index); - if (m_picker->is_finished(block)) - continue; - - auto dp = downloading_piece.find(block); - if (dp != downloading_piece.end()) - { - if (dp->second < p.bytes_downloaded) - dp->second = p.bytes_downloaded; - } - else - { - downloading_piece[block] = p.bytes_downloaded; - } - TORRENT_ASSERT(p.bytes_downloaded <= p.full_block_bytes); - TORRENT_ASSERT(p.full_block_bytes == to_req(piece_block( - p.piece_index, p.block_index)).length); - } - for (auto const& p : downloading_piece) - { - int const done = std::min(block_bytes_wanted(p.first), p.second); - st.total_done += done; - if (m_picker->piece_priority(p.first.piece_index) != dont_download) - st.total_wanted_done += done; - } - - TORRENT_ASSERT(st.total_done <= m_torrent_file->total_size() - m_padding); - TORRENT_ASSERT(st.total_wanted_done <= m_torrent_file->total_size() - m_padding); - -#if TORRENT_USE_INVARIANT_CHECKS - - if (st.total_done >= m_torrent_file->total_size()) - { - // This happens when a piece has been downloaded completely - // but not yet verified against the hash - std::fprintf(stderr, "num_have: %d\nunfinished:\n", num_have()); - for (auto const& dp : dl_queue) - { - std::fprintf(stderr, " %d ", static_cast(dp.index)); - for (auto const& info : m_picker->blocks_for_piece(dp)) - { - char const* state = info.state - == piece_picker::block_info::state_finished ? "1" : "0"; - fputs(state, stderr); - } - fputs("\n", stderr); - } - - fputs("downloading pieces:\n", stderr); - - for (auto const& p : downloading_piece) - { - std::fprintf(stderr, " %d:%d %d\n" - , static_cast(p.first.piece_index) - , p.first.block_index, p.second); - } - - } - - TORRENT_ASSERT(st.total_done <= m_torrent_file->total_size()); - TORRENT_ASSERT(st.total_wanted_done <= m_torrent_file->total_size()); - -#endif - - TORRENT_ASSERT(st.total_done >= st.total_wanted_done); } void torrent::on_piece_verified(piece_index_t const piece @@ -4815,8 +4663,7 @@ bool is_downloading_state(int const st) need_picker(); bool const was_finished = is_finished(); - bool filter_updated = m_picker->set_piece_priority(index, priority); - TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered()); + bool const filter_updated = m_picker->set_piece_priority(index, priority); update_gauge(); @@ -4876,7 +4723,6 @@ bool is_downloading_state(int const st) } filter_updated |= m_picker->set_piece_priority(p.first, p.second); - TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered()); } update_gauge(); if (filter_updated) @@ -4917,7 +4763,6 @@ bool is_downloading_state(int const st) , "we need assert prio >= dont_download"); TORRENT_ASSERT(prio <= top_priority); filter_updated |= m_picker->set_piece_priority(index, prio); - TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered()); ++index; } update_gauge(); @@ -7633,10 +7478,8 @@ bool is_downloading_state(int const st) // this is slightly different from m_picker->is_finished() // because any piece that has *passed* is considered here, // which may be more than the piece we *have* (i.e. written to disk) - // keep in mind that num_filtered() does not include pieces we - // have that are filtered return valid_metadata() && has_picker() - && m_torrent_file->num_pieces() - m_picker->num_filtered() - m_picker->num_passed() == 0; + && m_picker->want().num_pieces - m_picker->num_passed() == 0; } bool torrent::is_inactive() const @@ -7957,7 +7800,6 @@ bool is_downloading_state(int const st) } } } - TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered()); } if (valid_metadata()) @@ -9238,7 +9080,7 @@ bool is_downloading_state(int const st) TORRENT_ASSERT(share_mode()); if (is_seed()) return; - int pieces_in_torrent = m_torrent_file->num_pieces(); + int const pieces_in_torrent = m_torrent_file->num_pieces(); int num_seeds = 0; int num_peers = 0; int num_downloaders = 0; @@ -9305,8 +9147,8 @@ bool is_downloading_state(int const st) // now, download at least one piece, otherwise download one more // piece if our downloaded (and downloading) pieces is less than 50% // of the uploaded bytes - int const num_downloaded_pieces = std::max(m_picker->num_have() - , pieces_in_torrent - m_picker->num_filtered()); + int const num_downloaded_pieces = std::max(m_picker->have().num_pieces + , m_picker->want().num_pieces); if (std::int64_t(num_downloaded_pieces) * m_torrent_file->piece_length() * settings().get_int(settings_pack::share_mode_target) > m_total_uploaded @@ -10453,8 +10295,7 @@ bool is_downloading_state(int const st) // to avoid race condition, if we're already in a downloading state, // trigger the stop-when-ready logic immediately. - if (m_stop_when_ready - && is_downloading_state(m_state)) + if (m_stop_when_ready && is_downloading_state(m_state)) { #ifndef TORRENT_DISABLE_LOGGING debug_log("stop_when_ready triggered"); @@ -10681,7 +10522,7 @@ bool is_downloading_state(int const st) st->super_seeding = m_super_seeding; #endif st->has_metadata = valid_metadata(); - bytes_done(*st, bool(flags & torrent_handle::query_accurate_download_counters)); + bytes_done(*st, flags); TORRENT_ASSERT(st->total_wanted_done >= 0); TORRENT_ASSERT(st->total_done >= st->total_wanted_done); diff --git a/test/test_piece_picker.cpp b/test/test_piece_picker.cpp index 186b24331..d36a7a7b5 100644 --- a/test/test_piece_picker.cpp +++ b/test/test_piece_picker.cpp @@ -612,26 +612,26 @@ TORRENT_TEST(resize) // make sure init preserves priorities auto p = setup_picker("1111111", " ", "1111111", ""); - TEST_CHECK(p->num_filtered() == 0); - TEST_CHECK(p->num_have_filtered() == 0); - TEST_CHECK(p->num_have() == 0); + TEST_EQUAL(p->want().num_pieces, 7); + TEST_EQUAL(p->have_want().num_pieces, 0); + TEST_EQUAL(p->have().num_pieces, 0); p->set_piece_priority(piece_index_t(0), dont_download); - TEST_CHECK(p->num_filtered() == 1); - TEST_CHECK(p->num_have_filtered() == 0); - TEST_CHECK(p->num_have() == 0); + TEST_EQUAL(p->want().num_pieces, 6); + TEST_EQUAL(p->have_want().num_pieces, 0); + TEST_EQUAL(p->have().num_pieces, 0); p->we_have(piece_index_t(0)); - TEST_CHECK(p->num_filtered() == 0); - TEST_CHECK(p->num_have_filtered() == 1); - TEST_CHECK(p->num_have() == 1); + TEST_EQUAL(p->want().num_pieces, 6); + TEST_EQUAL(p->have_want().num_pieces, 0); + TEST_EQUAL(p->have().num_pieces, 1); p->resize(blocks_per_piece, blocks_per_piece, blocks_per_piece * 7); - TEST_CHECK(p->piece_priority(piece_index_t(0)) == dont_download); - TEST_CHECK(p->num_filtered() == 1); - TEST_CHECK(p->num_have_filtered() == 0); - TEST_CHECK(p->num_have() == 0); + TEST_EQUAL(p->piece_priority(piece_index_t(0)), dont_download); + TEST_EQUAL(p->want().num_pieces, blocks_per_piece * 7 - 1); + TEST_EQUAL(p->have_want().num_pieces, 0); + TEST_EQUAL(p->have().num_pieces, 0); } TORRENT_TEST(dont_pick_requested_blocks) @@ -992,15 +992,16 @@ TORRENT_TEST(piece_priorities) { // test piece priorities auto p = setup_picker("5555555", " ", "7654321", ""); - TEST_CHECK(p->num_filtered() == 0); - TEST_CHECK(p->num_have_filtered() == 0); + TEST_EQUAL(p->want().num_pieces, 7); + TEST_EQUAL(p->have_want().num_pieces, 0); p->set_piece_priority(piece_index_t(0), dont_download); - TEST_CHECK(p->num_filtered() == 1); - TEST_CHECK(p->num_have_filtered() == 0); + TEST_EQUAL(p->want().num_pieces, 6); + TEST_EQUAL(p->have_want().num_pieces, 0); p->mark_as_finished({piece_index_t(0), 0}, nullptr); p->we_have(piece_index_t(0)); - TEST_CHECK(p->num_filtered() == 0); - TEST_CHECK(p->num_have_filtered() == 1); + TEST_EQUAL(p->want().num_pieces, 6); + TEST_EQUAL(p->have_want().num_pieces, 0); + TEST_EQUAL(p->have().num_pieces, 1); p->we_dont_have(piece_index_t(0)); p->set_piece_priority(piece_index_t(0), top_priority); @@ -1537,28 +1538,28 @@ TORRENT_TEST(piece_passed) TEST_EQUAL(p->has_piece_passed(piece_index_t(0)), true); TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->num_passed(), 1); - TEST_EQUAL(p->num_have(), 1); + TEST_EQUAL(p->have().num_pieces, 1); p->piece_passed(piece_index_t(1)); TEST_EQUAL(p->num_passed(), 2); - TEST_EQUAL(p->num_have(), 1); + TEST_EQUAL(p->have().num_pieces, 1); p->we_have(piece_index_t(1)); - TEST_EQUAL(p->num_have(), 2); + TEST_EQUAL(p->have().num_pieces, 2); p->mark_as_finished({piece_index_t(2), 0}, &tmp1); p->piece_passed(piece_index_t(2)); TEST_EQUAL(p->num_passed(), 3); // just because the hash check passed doesn't mean // we "have" the piece. We need to write it to disk first - TEST_EQUAL(p->num_have(), 2); + TEST_EQUAL(p->have().num_pieces, 2); // piece 2 already passed the hash check, as soon as we've // written all the blocks to disk, we should have that piece too p->mark_as_finished({piece_index_t(2), 1}, &tmp1); p->mark_as_finished({piece_index_t(2), 2}, &tmp1); p->mark_as_finished({piece_index_t(2), 3}, &tmp1); - TEST_EQUAL(p->num_have(), 3); + TEST_EQUAL(p->have().num_pieces, 3); TEST_EQUAL(p->have_piece(piece_index_t(2)), true); } @@ -1569,15 +1570,15 @@ TORRENT_TEST(piece_passed_causing_we_have) TEST_EQUAL(p->has_piece_passed(piece_index_t(0)), true); TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->num_passed(), 1); - TEST_EQUAL(p->num_have(), 1); + TEST_EQUAL(p->have().num_pieces, 1); p->mark_as_finished({piece_index_t(1), 3}, &tmp1); TEST_EQUAL(p->num_passed(), 1); - TEST_EQUAL(p->num_have(), 1); + TEST_EQUAL(p->have().num_pieces, 1); p->piece_passed(piece_index_t(1)); TEST_EQUAL(p->num_passed(), 2); - TEST_EQUAL(p->num_have(), 2); + TEST_EQUAL(p->have().num_pieces, 2); } TORRENT_TEST(break_one_seed) @@ -1604,9 +1605,9 @@ TORRENT_TEST(we_dont_have2) TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->has_piece_passed(piece_index_t(2)), true); TEST_EQUAL(p->num_passed(), 2); - TEST_EQUAL(p->num_have(), 2); - TEST_EQUAL(p->num_have_filtered(), 1); - TEST_EQUAL(p->num_filtered(), 0); + TEST_EQUAL(p->have().num_pieces, 2); + TEST_EQUAL(p->have_want().num_pieces, 1); + TEST_EQUAL(p->want().num_pieces, 6); p->we_dont_have(piece_index_t(0)); @@ -1614,17 +1615,17 @@ TORRENT_TEST(we_dont_have2) TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->has_piece_passed(piece_index_t(2)), true); TEST_EQUAL(p->num_passed(), 1); - TEST_EQUAL(p->num_have(), 1); - TEST_EQUAL(p->num_have_filtered(), 1); + TEST_EQUAL(p->have().num_pieces, 1); + TEST_EQUAL(p->have_want().num_pieces, 0); p = setup_picker("1111111", "* * ", "1101111", ""); TEST_EQUAL(p->has_piece_passed(piece_index_t(0)), true); TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->has_piece_passed(piece_index_t(2)), true); TEST_EQUAL(p->num_passed(), 2); - TEST_EQUAL(p->num_have(), 2); - TEST_EQUAL(p->num_have_filtered(), 1); - TEST_EQUAL(p->num_filtered(), 0); + TEST_EQUAL(p->have().num_pieces, 2); + TEST_EQUAL(p->have_want().num_pieces, 1); + TEST_EQUAL(p->want().num_pieces, 6); p->we_dont_have(piece_index_t(2)); @@ -1632,8 +1633,9 @@ TORRENT_TEST(we_dont_have2) TEST_EQUAL(p->has_piece_passed(piece_index_t(1)), false); TEST_EQUAL(p->has_piece_passed(piece_index_t(2)), false); TEST_EQUAL(p->num_passed(), 1); - TEST_EQUAL(p->num_have(), 1); - TEST_EQUAL(p->num_have_filtered(), 0); + TEST_EQUAL(p->have().num_pieces, 1); + TEST_EQUAL(p->have_want().num_pieces, 1); + TEST_EQUAL(p->want().num_pieces, 6); } TORRENT_TEST(dont_have_but_passed_hash_check) @@ -2023,6 +2025,7 @@ TORRENT_TEST(mark_as_pad_whole_piece_seeding) p->mark_as_pad({piece_index_t{0}, 1}); p->mark_as_pad({piece_index_t{0}, 2}); p->mark_as_pad({piece_index_t{0}, 3}); + TEST_CHECK(p->have_piece(piece_index_t{0})); TEST_CHECK(!p->is_seeding()); @@ -2036,5 +2039,27 @@ TORRENT_TEST(mark_as_pad_whole_piece_seeding) TEST_CHECK(p->is_seeding()); } +TORRENT_TEST(pad_blocks_in_piece) +{ + auto p = setup_picker("11", " ", "44", ""); + p->mark_as_pad({piece_index_t{0}, 0}); + p->mark_as_pad({piece_index_t{0}, 1}); + p->mark_as_pad({piece_index_t{0}, 2}); + + TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{0}), 3); + TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{1}), 0); +} + +TORRENT_TEST(pad_blocks_in_last_piece) +{ + auto p = setup_picker("11", " ", "44", ""); + p->mark_as_pad({piece_index_t{1}, 0}); + p->mark_as_pad({piece_index_t{1}, 1}); + p->mark_as_pad({piece_index_t{1}, 2}); + + TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{1}), 3); + TEST_EQUAL(p->pad_blocks_in_piece(piece_index_t{0}), 0); +} + //TODO: 2 test picking with partial pieces and other peers present so that both // backup_pieces and backup_pieces2 are used diff --git a/test/test_torrent.cpp b/test/test_torrent.cpp index 9deaf0b64..fe2d263c0 100644 --- a/test/test_torrent.cpp +++ b/test/test_torrent.cpp @@ -692,3 +692,52 @@ TORRENT_TEST(test_read_piece_out_of_range) , lt::libtorrent_category())); } } + +namespace { +int const piece_size = 0x4000 * 2; + +file_storage test_fs() +{ + file_storage fs; + fs.set_piece_length(piece_size); + fs.add_file("temp", 999999); + fs.set_num_pieces(int((fs.total_size() + piece_size - 1) / piece_size)); + return fs; +} +} + +TORRENT_TEST(test_calc_bytes_pieces) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{2, 0, false}), 2 * piece_size); +} + +TORRENT_TEST(test_calc_bytes_pieces_last) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{2, 0, true}), piece_size + fs.total_size() % piece_size); +} + +TORRENT_TEST(test_calc_bytes_no_pieces) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{0, 0, false}), 0); +} + +TORRENT_TEST(test_calc_bytes_all_pieces) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{fs.num_pieces(), 0, true}), fs.total_size()); +} + +TORRENT_TEST(test_calc_bytes_all_pieces_one_pad) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{fs.num_pieces(), 1, true}), fs.total_size() - 0x4000); +} + +TORRENT_TEST(test_calc_bytes_all_pieces_two_pad) +{ + auto const fs = test_fs(); + TEST_EQUAL(calc_bytes(fs, piece_count{fs.num_pieces(), 2, true}), fs.total_size() - 2 * 0x4000); +}