fix tail padding in file_storage::optimize and improve file placement with some tests (#1105)
This commit is contained in:
parent
6c31ea3d44
commit
527772420a
|
@ -1,3 +1,4 @@
|
|||
* improve file layout optimization when creating torrents with padfiles
|
||||
* remove remote_dl_rate feature
|
||||
* source code migration from boost::shared_ptr to std::shared_ptr
|
||||
* storage_interface API changed to use span and references
|
||||
|
|
|
@ -96,18 +96,6 @@ namespace libtorrent
|
|||
|
||||
namespace
|
||||
{
|
||||
bool compare_string(char const* str, int len, std::string const& str2)
|
||||
{
|
||||
if (str2.size() != len) return false;
|
||||
return memcmp(str2.c_str(), str, len) == 0;
|
||||
}
|
||||
|
||||
bool compare_file_entry_size(internal_file_entry const& fe1
|
||||
, internal_file_entry const& fe2)
|
||||
{
|
||||
return fe1.size < fe2.size;
|
||||
}
|
||||
|
||||
bool compare_file_offset(internal_file_entry const& lhs
|
||||
, internal_file_entry const& rhs)
|
||||
{
|
||||
|
@ -156,7 +144,7 @@ namespace libtorrent
|
|||
&& std::memcmp(branch_path, m_name.c_str(), m_name.size()) == 0)
|
||||
{
|
||||
// the +1 is to skip the trailing '/' (or '\')
|
||||
int offset = int(m_name.size())
|
||||
int const offset = int(m_name.size())
|
||||
+ (m_name.size() == branch_len?0:1);
|
||||
branch_path += offset;
|
||||
branch_len -= offset;
|
||||
|
@ -168,9 +156,12 @@ namespace libtorrent
|
|||
}
|
||||
|
||||
// do we already have this path in the path list?
|
||||
std::vector<std::string>::reverse_iterator p
|
||||
= std::find_if(m_paths.rbegin(), m_paths.rend()
|
||||
, std::bind(&compare_string, branch_path, branch_len, _1));
|
||||
auto p = std::find_if(m_paths.rbegin(), m_paths.rend()
|
||||
, [&] (std::string const& str)
|
||||
{
|
||||
if (str.size() != branch_len) return false;
|
||||
return memcmp(str.c_str(), branch_path, branch_len) == 0;
|
||||
});
|
||||
|
||||
if (p == m_paths.rend())
|
||||
{
|
||||
|
@ -403,7 +394,7 @@ namespace libtorrent
|
|||
target.offset = offset;
|
||||
TORRENT_ASSERT(!compare_file_offset(target, m_files.front()));
|
||||
|
||||
std::vector<internal_file_entry>::const_iterator file_iter = std::upper_bound(
|
||||
auto file_iter = std::upper_bound(
|
||||
begin_deprecated(), end_deprecated(), target, compare_file_offset);
|
||||
|
||||
TORRENT_ASSERT(file_iter != begin_deprecated());
|
||||
|
@ -424,7 +415,7 @@ namespace libtorrent
|
|||
target.offset = offset;
|
||||
TORRENT_ASSERT(!compare_file_offset(target, m_files.front()));
|
||||
|
||||
std::vector<internal_file_entry>::const_iterator file_iter = std::upper_bound(
|
||||
auto file_iter = std::upper_bound(
|
||||
m_files.begin(), m_files.end(), target, compare_file_offset);
|
||||
|
||||
TORRENT_ASSERT(file_iter != m_files.begin());
|
||||
|
@ -463,7 +454,7 @@ namespace libtorrent
|
|||
if (std::int64_t(target.offset + size) > m_total_size)
|
||||
size = m_total_size - target.offset;
|
||||
|
||||
std::vector<internal_file_entry>::const_iterator file_iter = std::upper_bound(
|
||||
auto file_iter = std::upper_bound(
|
||||
m_files.begin(), m_files.end(), target, compare_file_offset);
|
||||
|
||||
TORRENT_ASSERT(file_iter != m_files.begin());
|
||||
|
@ -969,12 +960,14 @@ namespace libtorrent
|
|||
#endif // TORRENT_DEPRECATED
|
||||
}
|
||||
|
||||
void file_storage::optimize(int pad_file_limit, int alignment
|
||||
, bool tail_padding)
|
||||
void file_storage::optimize(int const pad_file_limit, int alignment
|
||||
, bool const tail_padding)
|
||||
{
|
||||
if (alignment == -1)
|
||||
alignment = m_piece_length;
|
||||
|
||||
// TODO: padfiles should be removed
|
||||
|
||||
std::int64_t off = 0;
|
||||
int padding_file = 0;
|
||||
for (std::vector<internal_file_entry>::iterator i = m_files.begin();
|
||||
|
@ -983,10 +976,24 @@ namespace libtorrent
|
|||
if ((off % alignment) == 0)
|
||||
{
|
||||
// this file position is aligned, pick the largest
|
||||
// available file to put here
|
||||
std::vector<internal_file_entry>::iterator best_match
|
||||
= std::max_element(i, m_files.end()
|
||||
, &compare_file_entry_size);
|
||||
// available file to put here. If we encounter a file whose size is
|
||||
// divisible by `alignment`, we pick that immediately, since that
|
||||
// will not affect whether we're at an aligned position and will
|
||||
// improve packing of files
|
||||
std::vector<internal_file_entry>::iterator best_match = i;
|
||||
for (auto k = i; k != m_files.end(); ++k)
|
||||
{
|
||||
// a file whose size fits the alignment always takes priority,
|
||||
// since it will let us keep placing aligned files
|
||||
if ((k->size % alignment) == 0)
|
||||
{
|
||||
best_match = k;
|
||||
break;
|
||||
}
|
||||
// otherwise, pick the largest file, to have as many bytes be
|
||||
// aligned.
|
||||
if (best_match->size < k->size) best_match = k;
|
||||
}
|
||||
|
||||
if (best_match != i)
|
||||
{
|
||||
|
@ -1062,14 +1069,15 @@ namespace libtorrent
|
|||
// skip the file we just put in place, so we put the pad
|
||||
// file after it
|
||||
++i;
|
||||
if (i == m_files.end()) break;
|
||||
|
||||
// tail-padding is enabled, and the offset after this file is not
|
||||
// aligned and it's not the last file. The last file must be padded
|
||||
// too, in order to match an equivalent tail-padded file.
|
||||
// aligned. The last file must be padded too, in order to match an
|
||||
// equivalent tail-padded file.
|
||||
add_pad_file(alignment - (off % alignment), i, off, padding_file);
|
||||
|
||||
TORRENT_ASSERT((off % alignment) == 0);
|
||||
|
||||
if (i == m_files.end()) break;
|
||||
}
|
||||
}
|
||||
m_total_size = off;
|
||||
|
@ -1080,8 +1088,8 @@ namespace libtorrent
|
|||
, std::int64_t& offset
|
||||
, int& pad_file_counter)
|
||||
{
|
||||
int cur_index = i - m_files.begin();
|
||||
int index = int(m_files.size());
|
||||
int const cur_index = i - m_files.begin();
|
||||
int const index = int(m_files.size());
|
||||
m_files.push_back(internal_file_entry());
|
||||
++m_num_files;
|
||||
internal_file_entry& e = m_files.back();
|
||||
|
@ -1104,7 +1112,7 @@ namespace libtorrent
|
|||
if (!m_file_base.empty()) m_file_base.resize(index + 1, 0);
|
||||
#endif
|
||||
|
||||
reorder_file(index, cur_index);
|
||||
if (index != cur_index) reorder_file(index, cur_index);
|
||||
}
|
||||
|
||||
void file_storage::unload()
|
||||
|
|
|
@ -211,7 +211,99 @@ TORRENT_TEST(file_path_hash)
|
|||
TEST_EQUAL(file_hash0, file_hash1);
|
||||
}
|
||||
|
||||
// TODO: test file_storage::optimize
|
||||
// make sure that files whose size is a multiple by the alignment are picked
|
||||
// first, since that lets us keep placing files aligned
|
||||
TORRENT_TEST(optimize_aligned_sizes)
|
||||
{
|
||||
file_storage fs;
|
||||
fs.set_piece_length(512);
|
||||
fs.add_file(combine_path("s", "1"), 1);
|
||||
fs.add_file(combine_path("s", "2"), 40000);
|
||||
fs.add_file(combine_path("s", "3"), 1024);
|
||||
|
||||
fs.optimize(512, 512, false);
|
||||
|
||||
// since the size of file 3 is a multiple of the alignment (512), it should
|
||||
// be prioritized, to minimize the amount of padding.
|
||||
// after that, we want to pick the largest file (2), and since file 1 is
|
||||
// smaller than the pad-file limit (512) we won't pad it. Since tail_padding
|
||||
// is false, we won't pad the tail of the torrent either
|
||||
|
||||
TEST_EQUAL(fs.num_files(), 3);
|
||||
|
||||
TEST_EQUAL(fs.file_size(0), 1024);
|
||||
TEST_EQUAL(fs.file_name(0), "3");
|
||||
TEST_EQUAL(fs.pad_file_at(0), false);
|
||||
|
||||
TEST_EQUAL(fs.file_size(1), 40000);
|
||||
TEST_EQUAL(fs.file_name(1), "2");
|
||||
TEST_EQUAL(fs.pad_file_at(1), false);
|
||||
|
||||
TEST_EQUAL(fs.file_size(2), 1);
|
||||
TEST_EQUAL(fs.file_name(2), "1");
|
||||
TEST_EQUAL(fs.pad_file_at(2), false);
|
||||
}
|
||||
|
||||
// make sure we pad the end of the torrent when tail_padding is specified
|
||||
TORRENT_TEST(optimize_tail_padding)
|
||||
{
|
||||
file_storage fs;
|
||||
fs.set_piece_length(512);
|
||||
fs.add_file(combine_path("s", "1"), 700);
|
||||
|
||||
fs.optimize(512, 512, true);
|
||||
|
||||
// since the size of file 3 is a multiple of the alignment (512), it should
|
||||
// be prioritized, to minimize the amount of padding.
|
||||
// after that, we want to pick the largest file (2), and since file 1 is
|
||||
// smaller than the pad-file limit (512) we won't pad it. Since tail_padding
|
||||
// is false, we won't pad the tail of the torrent either
|
||||
|
||||
TEST_EQUAL(fs.num_files(), 2);
|
||||
|
||||
TEST_EQUAL(fs.file_size(0), 700);
|
||||
TEST_EQUAL(fs.file_name(0), "1");
|
||||
TEST_EQUAL(fs.pad_file_at(0), false);
|
||||
|
||||
TEST_EQUAL(fs.file_size(1), 1024 - 700);
|
||||
TEST_EQUAL(fs.pad_file_at(1), true);
|
||||
}
|
||||
|
||||
|
||||
// make sure we fill in padding with small files
|
||||
TORRENT_TEST(optimize_pad_fillers)
|
||||
{
|
||||
file_storage fs;
|
||||
fs.set_piece_length(512);
|
||||
fs.add_file(combine_path("s", "1"), 1);
|
||||
fs.add_file(combine_path("s", "2"), 1000);
|
||||
fs.add_file(combine_path("s", "3"), 1001);
|
||||
|
||||
fs.optimize(512, 512, false);
|
||||
|
||||
// first we pick the largest file, then we need to add padding, since file 1
|
||||
// is smaller than the pad file limit, it won't be aligned anyway, so we
|
||||
// place that as part of the padding
|
||||
|
||||
TEST_EQUAL(fs.num_files(), 4);
|
||||
|
||||
TEST_EQUAL(fs.file_size(0), 1001);
|
||||
TEST_EQUAL(fs.file_name(0), "3");
|
||||
TEST_EQUAL(fs.pad_file_at(0), false);
|
||||
|
||||
TEST_EQUAL(fs.file_size(1), 1);
|
||||
TEST_EQUAL(fs.file_name(1), "1");
|
||||
TEST_EQUAL(fs.pad_file_at(1), false);
|
||||
|
||||
TEST_EQUAL(fs.file_size(2), 1024 - (1001 + 1));
|
||||
TEST_EQUAL(fs.pad_file_at(2), true);
|
||||
|
||||
TEST_EQUAL(fs.file_size(3), 1000);
|
||||
TEST_EQUAL(fs.file_name(3), "2");
|
||||
TEST_EQUAL(fs.pad_file_at(3), false);
|
||||
}
|
||||
|
||||
// TODO: add more optimize() tests
|
||||
// TODO: test map_block
|
||||
// TODO: test piece_size(int piece)
|
||||
// TODO: test file_index_at_offset
|
||||
|
|
Loading…
Reference in New Issue