fix: increase priority of first and last piece of each file (#5167)

The first and last pieces of a file generally include information needed
by preview generators, so prioritizing those pieces makes life easier
for users or tools that look at files while they're being downloaded.

This worked in 3.00 but isn't present in 4.0.0.
This commit is contained in:
Charles Kerr 2023-03-06 15:50:19 -06:00 committed by GitHub
parent 2dd39596d1
commit 39e3e1d87b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 16 deletions

View File

@ -60,11 +60,6 @@ void tr_file_piece_map::reset(tr_torrent_metainfo const& tm)
reset({ tm.totalSize(), tm.pieceSize() }, std::data(file_sizes), std::size(file_sizes));
}
tr_file_piece_map::piece_span_t tr_file_piece_map::pieceSpan(tr_file_index_t file) const
{
return file_pieces_[file];
}
tr_file_piece_map::file_span_t tr_file_piece_map::fileSpan(tr_piece_index_t piece) const
{
auto compare = CompareToSpan<tr_piece_index_t>{};
@ -129,20 +124,34 @@ tr_priority_t tr_file_priorities::filePriority(tr_file_index_t file) const
tr_priority_t tr_file_priorities::piecePriority(tr_piece_index_t piece) const
{
if (std::empty(priorities_))
if (std::empty(*fpm_)) // not initialized yet
{
return TR_PRI_NORMAL;
}
auto const [begin_idx, end_idx] = fpm_->fileSpan(piece);
auto const begin = std::begin(priorities_) + begin_idx;
auto const end = std::begin(priorities_) + end_idx;
auto const it = std::max_element(begin, end);
if (it == end)
// increase priority if a file begins or ends in this piece
// because that makes life easier for code/users using at incomplete files.
// Xrefs: f2daeb242, https://forum.transmissionbt.com/viewtopic.php?t=10473
auto const [begin_file, end_file] = fpm_->fileSpan(piece);
if ((begin_file + 1 != end_file) // at least one file ends in this piece
|| (piece == fpm_->pieceSpan(begin_file).begin) // piece's first file starts in this piece
|| (end_file > begin_file && piece + 1 == fpm_->pieceSpan(end_file - 1).end)) // piece's last file ends in this piece
{
return TR_PRI_NORMAL;
return TR_PRI_HIGH;
}
return *it;
// check the priorities of the files that touch this piece
if (end_file <= std::size(priorities_))
{
auto const begin = std::begin(priorities_) + begin_file;
auto const end = std::begin(priorities_) + end_file;
if (auto const it = std::max_element(begin, end); it != end)
{
return *it;
}
}
return TR_PRI_NORMAL;
}
// ---

View File

@ -52,7 +52,11 @@ public:
void reset(tr_block_info const& block_info, uint64_t const* file_sizes, size_t n_files);
void reset(tr_torrent_metainfo const& tm);
[[nodiscard]] piece_span_t pieceSpan(tr_file_index_t file) const;
[[nodiscard]] TR_CONSTEXPR20 piece_span_t pieceSpan(tr_file_index_t file) const noexcept
{
return file_pieces_[file];
}
[[nodiscard]] file_span_t fileSpan(tr_piece_index_t piece) const;
[[nodiscard]] file_offset_t fileOffset(uint64_t offset) const;
@ -62,6 +66,11 @@ public:
return std::size(file_pieces_);
}
[[nodiscard]] TR_CONSTEXPR20 bool empty() const noexcept
{
return std::empty(file_pieces_);
}
// TODO(ckerr) minor wart here, two identical span types
[[nodiscard]] tr_byte_span_t byteSpan(tr_file_index_t file) const
{

View File

@ -139,15 +139,33 @@ TEST_F(FilePieceMapTest, priorities)
{
for (tr_file_index_t i = 0; i < n_files; ++i)
{
EXPECT_EQ(int(expected_file_priorities[i]), int(file_priorities.filePriority(i)));
auto const expected = int{ expected_file_priorities[i] };
auto const actual = int{ file_priorities.filePriority(i) };
EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']';
}
for (tr_piece_index_t i = 0; i < block_info_.pieceCount(); ++i)
{
EXPECT_EQ(int(expected_piece_priorities[i]), int(file_priorities.piecePriority(i)));
auto const expected = int{ expected_piece_priorities[i] };
auto const actual = int{ file_priorities.piecePriority(i) };
EXPECT_EQ(expected, actual) << "idx[" << i << "] expected [" << expected << "] actual [" << actual << ']';
}
};
auto const mark_file_endpoints_as_high_priority = [&]()
{
for (tr_file_index_t i = 0; i < n_files; ++i)
{
auto const [begin_piece, end_piece] = fpm.pieceSpan(i);
expected_piece_priorities[begin_piece] = TR_PRI_HIGH;
if (end_piece > begin_piece)
{
expected_piece_priorities[end_piece - 1] = TR_PRI_HIGH;
}
}
};
// check default priority is normal
mark_file_endpoints_as_high_priority();
compare_to_expected();
// set the first file as high priority.
@ -160,6 +178,7 @@ TEST_F(FilePieceMapTest, priorities)
{
expected_piece_priorities[i] = pri;
}
mark_file_endpoints_as_high_priority();
compare_to_expected();
// This file shares a piece with another file.
@ -172,17 +191,20 @@ TEST_F(FilePieceMapTest, priorities)
file_priorities.set(5, pri);
expected_file_priorities[5] = pri;
expected_piece_priorities[5] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// ...and that shared piece should still be the same when both are high...
file_priorities.set(6, pri);
expected_file_priorities[6] = pri;
expected_piece_priorities[5] = pri;
expected_piece_priorities[6] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// ...and that shared piece should still be the same when only 6 is high...
pri = TR_PRI_NORMAL;
file_priorities.set(5, pri);
expected_file_priorities[5] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// setup for the next test: set all files to low priority
@ -193,6 +215,7 @@ TEST_F(FilePieceMapTest, priorities)
}
std::fill(std::begin(expected_file_priorities), std::end(expected_file_priorities), pri);
std::fill(std::begin(expected_piece_priorities), std::end(expected_piece_priorities), pri);
mark_file_endpoints_as_high_priority();
compare_to_expected();
// Raise the priority of a small 1-piece file.
@ -202,6 +225,7 @@ TEST_F(FilePieceMapTest, priorities)
file_priorities.set(8, pri);
expected_file_priorities[8] = pri;
expected_piece_priorities[6] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// Raise the priority of another small 1-piece file in the same piece.
// Since _it_ now has the highest priority in the piece, piecePriority should return _its_ value.
@ -210,6 +234,7 @@ TEST_F(FilePieceMapTest, priorities)
file_priorities.set(9, pri);
expected_file_priorities[9] = pri;
expected_piece_priorities[6] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// Prep for the next test: set all files to normal priority
@ -220,6 +245,7 @@ TEST_F(FilePieceMapTest, priorities)
}
std::fill(std::begin(expected_file_priorities), std::end(expected_file_priorities), pri);
std::fill(std::begin(expected_piece_priorities), std::end(expected_piece_priorities), pri);
mark_file_endpoints_as_high_priority();
compare_to_expected();
// *Sigh* OK what happens to piece priorities if you set the priority
@ -234,12 +260,14 @@ TEST_F(FilePieceMapTest, priorities)
file_priorities.set(1, pri);
expected_file_priorities[1] = pri;
expected_piece_priorities[5] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// Check that zero-sized files at the end of a torrent change the last piece's priority.
// file #16 byte [1001, 1001) piece [10, 11)
file_priorities.set(16, pri);
expected_file_priorities[16] = pri;
expected_piece_priorities[10] = pri;
mark_file_endpoints_as_high_priority();
compare_to_expected();
// test the batch API
@ -249,11 +277,13 @@ TEST_F(FilePieceMapTest, priorities)
file_priorities.set(std::data(file_indices), std::size(file_indices), pri);
std::fill(std::begin(expected_file_priorities), std::end(expected_file_priorities), pri);
std::fill(std::begin(expected_piece_priorities), std::end(expected_piece_priorities), pri);
mark_file_endpoints_as_high_priority();
compare_to_expected();
pri = TR_PRI_LOW;
file_priorities.set(std::data(file_indices), std::size(file_indices), pri);
std::fill(std::begin(expected_file_priorities), std::end(expected_file_priorities), pri);
std::fill(std::begin(expected_piece_priorities), std::end(expected_piece_priorities), pri);
mark_file_endpoints_as_high_priority();
compare_to_expected();
}