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

@@ -69,6 +69,17 @@ namespace libtorrent {
 	using picker_options_t = flags::bitfield_flag<std::uint16_t, struct picker_options_tag>;
 	using download_queue_t = aux::strong_typedef<std::uint8_t, struct dl_queue_tag>;
 
+	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
+		// piece states
-		int num_have_filtered() const { return m_num_have_filtered; }
+		//
+		//       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
+		// number of pieces not filtered, as well as the number of
-		// been successfully flushed to disk. Including pieces we have
+		// blocks out of those pieces that are pad blocks.
-		// also filtered with priority 0 but have anyway.
+		// ``last_piece`` is set if the last piece is one of the
-		int num_have() const { return m_num_have; }
+		// 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<block_info> mutable_blocks_for_piece(downloading_piece const& dp);
 
 		std::tuple<bool, bool, int, int> requested_from(
@@ -695,6 +730,15 @@ namespace libtorrent {
 		// TODO: this could be a much more efficient data structure
 		std::set<piece_block> 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

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<std::int64_t> 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
+		// the number of (16kiB) blocks that fall entirely in pad files
-		std::uint32_t m_padding:24;
+		// 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

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.

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).

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<piece_block>::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 (pad_blocks_in_piece(block.piece_index) == blocks)
-		if (std::distance(begin, end) == 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<torrent_peer*>& 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};
+	}
+
 }

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<std::uint8_t>(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
+	// we assume the last block is never a pad block. Should be a fairly
-	// in for the given block. This returns block_size()
+	// safe assumption, and you just get a few kiB off if it is
-	// for all blocks except the last one (if it's smaller
+	std::int64_t calc_bytes(file_storage const& fs, piece_count const& pc)
-	// than block_size()) and blocks that overlap a padding
-	// file
-	int torrent::block_bytes_wanted(piece_block const& p) const
 	{
-		file_storage const& fs = m_torrent_file->files();
+		// it's an impossible combination to have 0 pieces, but still have one of them be the last piece
-		int const piece_size = m_torrent_file->piece_size(p.piece_index);
+		TORRENT_ASSERT(!(pc.num_pieces == 0 && pc.last_piece == true));
-		int const offset = p.block_index * block_size();
-		if (m_padding == 0) return std::min(piece_size - offset, block_size());
 
-		std::vector<file_slice> const files = fs.map_block(
+		// if we have 0 pieces, we can't have any pad blocks either
-			p.piece_index, offset, std::min(piece_size - offset, block_size()));
+		TORRENT_ASSERT(!(pc.num_pieces == 0 && pc.pad_blocks > 0));
-		std::int64_t ret = 0;
+
-		for (auto const& i : files)
+		// if we have all pieces, we must also have the last one
-		{
+		TORRENT_ASSERT(!(pc.num_pieces == fs.num_pieces() && pc.last_piece == false));
-			if (fs.pad_file_at(i.file_index)) continue;
+		int const block_size = std::min(default_block_size, fs.piece_length());
-			ret += i.size;
+
-		}
+		// every block should not be a pad block
-		TORRENT_ASSERT(ret <= std::min(piece_size - offset, block_size()));
+		TORRENT_ASSERT(pc.pad_blocks <= pc.num_pieces * fs.piece_length() / block_size);
-		return aux::numeric_cast<int>(ret);
+
+		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)
+		TORRENT_ASSERT(!valid_metadata() || m_torrent_file->num_pieces() > 0);
-			return;
+		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());
+		TORRENT_ASSERT(has_picker());
-		st.total_wanted_done = std::int64_t(num_have() - m_picker->num_have_filtered())
+
-			* piece_size;
+		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);
-
+		TORRENT_ASSERT(st.total_done >= 0);
-		// 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 >= 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<piece_picker::downloading_piece> 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;
+			if (m_picker->have_piece(index)) continue;
-			TORRENT_ASSERT(i->finished <= m_picker->blocks_in_piece(index));
 
-#if TORRENT_USE_ASSERTS
+			TORRENT_ASSERT(i->finished + i->writing <= m_picker->blocks_in_piece(index));
-			TORRENT_ASSERT(std::count_if(std::next(i), dl_queue.end()
+			TORRENT_ASSERT(i->finished + i->writing >= m_picker->pad_blocks_in_piece(index));
-				, [index](piece_picker::downloading_piece const& p) { return p.index == index; }) == 0);
-#endif
 
-			int idx = -1;
+			int const blocks = i->finished + i->writing - m_picker->pad_blocks_in_piece(index);
-			for (auto const& info : m_picker->blocks_for_piece(*i))
+			TORRENT_ASSERT(blocks >= 0);
-			{
-				++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);
-			}
 
-			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<piece_block, int> 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<int>(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<int>(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);
+		bool const filter_updated = m_picker->set_piece_priority(index, priority);
-		TORRENT_ASSERT(num_have() >= m_picker->num_have_filtered());
 
 		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()
+		int const num_downloaded_pieces = std::max(m_picker->have().num_pieces
-			, pieces_in_torrent - m_picker->num_filtered());
+			, 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
+		if (m_stop_when_ready && is_downloading_state(m_state))
-			&& 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);
 

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_EQUAL(p->want().num_pieces, 7);
-	TEST_CHECK(p->num_have_filtered() == 0);
+	TEST_EQUAL(p->have_want().num_pieces, 0);
-	TEST_CHECK(p->num_have() == 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_EQUAL(p->want().num_pieces, 6);
-	TEST_CHECK(p->num_have_filtered() == 0);
+	TEST_EQUAL(p->have_want().num_pieces, 0);
-	TEST_CHECK(p->num_have() == 0);
+	TEST_EQUAL(p->have().num_pieces, 0);
 
 	p->we_have(piece_index_t(0));
 
-	TEST_CHECK(p->num_filtered() == 0);
+	TEST_EQUAL(p->want().num_pieces, 6);
-	TEST_CHECK(p->num_have_filtered() == 1);
+	TEST_EQUAL(p->have_want().num_pieces, 0);
-	TEST_CHECK(p->num_have() == 1);
+	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_EQUAL(p->piece_priority(piece_index_t(0)), dont_download);
-	TEST_CHECK(p->num_filtered() == 1);
+	TEST_EQUAL(p->want().num_pieces, blocks_per_piece * 7 - 1);
-	TEST_CHECK(p->num_have_filtered() == 0);
+	TEST_EQUAL(p->have_want().num_pieces, 0);
-	TEST_CHECK(p->num_have() == 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_EQUAL(p->want().num_pieces, 7);
-	TEST_CHECK(p->num_have_filtered() == 0);
+	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_EQUAL(p->want().num_pieces, 6);
-	TEST_CHECK(p->num_have_filtered() == 0);
+	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_EQUAL(p->want().num_pieces, 6);
-	TEST_CHECK(p->num_have_filtered() == 1);
+	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->have().num_pieces, 2);
-	TEST_EQUAL(p->num_have_filtered(), 1);
+	TEST_EQUAL(p->have_want().num_pieces, 1);
-	TEST_EQUAL(p->num_filtered(), 0);
+	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->have().num_pieces, 1);
-	TEST_EQUAL(p->num_have_filtered(), 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->have().num_pieces, 2);
-	TEST_EQUAL(p->num_have_filtered(), 1);
+	TEST_EQUAL(p->have_want().num_pieces, 1);
-	TEST_EQUAL(p->num_filtered(), 0);
+	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->have().num_pieces, 1);
-	TEST_EQUAL(p->num_have_filtered(), 0);
+	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

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);
+}