From 19df6450127080da19a271a7270acb74bae69226 Mon Sep 17 00:00:00 2001 From: arvidn Date: Sun, 29 Jul 2018 10:33:09 +0200 Subject: [PATCH] apply piece priorities immediately, even though file priority updares are async. save both file- and piece priorities in fast resume. when loading, apply file prios first, then piece prios --- ChangeLog | 1 + include/libtorrent/torrent.hpp | 2 +- include/libtorrent/torrent_handle.hpp | 19 +++++-- simulation/test_tracker.cpp | 3 +- src/torrent.cpp | 71 ++++++++++++++------------- test/setup_transfer.cpp | 25 ++++++++++ test/setup_transfer.hpp | 2 + test/test_priority.cpp | 45 +++++++++++++++++ test/test_resume.cpp | 71 +++++++++++++-------------- test/test_torrent.cpp | 6 ++- 10 files changed, 167 insertions(+), 78 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6ab191620..d85db05d5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,6 @@ 1.1.9 release + * save both file and piece priorities in resume file * added missing stats_metric python binding * uTP connections are no longer exempt from rate limits by default * fix exporting files from partfile while seeding diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 0924213ab..9ee9668e9 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -552,7 +552,7 @@ namespace libtorrent void set_piece_deadline(int piece, int t, int flags); void reset_piece_deadline(int piece); void clear_time_critical(); - void update_piece_priorities(); + void update_piece_priorities(std::vector const& file_prio); void status(torrent_status* st, boost::uint32_t flags); diff --git a/include/libtorrent/torrent_handle.hpp b/include/libtorrent/torrent_handle.hpp index 1af6e8327..ab07dd856 100644 --- a/include/libtorrent/torrent_handle.hpp +++ b/include/libtorrent/torrent_handle.hpp @@ -1022,9 +1022,8 @@ namespace libtorrent // The default priority of pieces is 4. // // Piece priorities can not be changed for torrents that have not - // downloaded the metadata yet. For instance, magnet links and torrents - // added by URL won't have metadata immediately. see the - // metadata_received_alert. + // downloaded the metadata yet. Magnet links won't have metadata + // immediately. see the metadata_received_alert. // // ``piece_priority`` sets or gets the priority for an individual piece, // specified by ``index``. @@ -1041,6 +1040,10 @@ namespace libtorrent // // ``piece_priorities`` returns a vector with one element for each piece // in the torrent. Each element is the current priority of that piece. + // + // It's possible to cancel the effect of *file* priorities by setting the + // priorities for the affected pieces. Care has to be taken when mixing + // usage of file- and piece priorities. void piece_priority(int index, int priority) const; int piece_priority(int index) const; void prioritize_pieces(std::vector const& pieces) const; @@ -1069,6 +1072,16 @@ namespace libtorrent // You cannot set the file priorities on a torrent that does not yet have // metadata or a torrent that is a seed. ``file_priority(int, int)`` and // prioritize_files() are both no-ops for such torrents. + // + // Since changing file priorities may involve disk operations (of moving + // files in- and out of the part file), the internal accounting of file + // priorities happen asynchronously. i.e. setting file priorities and then + // immediately querying them may not yield the same priorities just set. + // However, the *piece* priorities are updated immediately. + // + // when combining file- and piece priorities, the resume file will record + // both. When loading the resume data, the file priorities will be applied + // first, then the piece priorities. void file_priority(int index, int priority) const; int file_priority(int index) const; void prioritize_files(std::vector const& files) const; diff --git a/simulation/test_tracker.cpp b/simulation/test_tracker.cpp index c793fa851..6988c3b58 100644 --- a/simulation/test_tracker.cpp +++ b/simulation/test_tracker.cpp @@ -67,7 +67,6 @@ void test_interval(int interval) sim::default_config network_cfg; sim::simulation sim{network_cfg}; - lt::time_point start = lt::clock_type::now(); bool ran_to_completion = false; sim::asio::io_service web_server(sim, address_v4::from_string("2.2.2.2")); @@ -78,7 +77,7 @@ void test_interval(int interval) std::vector announces; http.register_handler("/announce" - , [&announces,interval,start,&ran_to_completion](std::string method, std::string req + , [&announces,interval,&ran_to_completion](std::string method, std::string req , std::map&) { // don't collect events once we're done. We're not interested in the diff --git a/src/torrent.cpp b/src/torrent.cpp index 3fe1e05f0..b72939c3b 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -1993,9 +1993,12 @@ namespace libtorrent } } - // if we've already loaded file priorities, don't load piece priorities, - // they will interfere. - if (!m_seed_mode && m_resume_data && m_file_priority.empty()) + // in case file priorities were passed in via the add_torrent_params + // and also in the case of share mode, we need to update the priorities + // this has to be applied before piece priority + if (!m_file_priority.empty()) update_piece_priorities(m_file_priority); + + if (!m_seed_mode && m_resume_data) { bdecode_node piece_priority = m_resume_data->node .dict_find_string("piece_priority"); @@ -2015,14 +2018,6 @@ namespace libtorrent } } - // in case file priorities were passed in via the add_torrent_params - // and also in the case of share mode, we need to update the priorities - if (!m_file_priority.empty() && std::find(m_file_priority.begin() - , m_file_priority.end(), 0) != m_file_priority.end()) - { - update_piece_priorities(); - } - #if TORRENT_USE_ASSERTS m_resume_data_loaded = true; #endif @@ -5683,7 +5678,6 @@ namespace { if (m_file_priority != *p) { m_file_priority = *p; - update_piece_priorities(); if (m_share_mode) recalc_share_mode(); } @@ -5710,6 +5704,13 @@ namespace { // storage may be NULL during shutdown if (m_storage) { + // the update of m_file_priority is deferred until the disk job comes + // back, but to preserve sanity and consistency, the piece priorities are + // updated immediately. If, on the off-chance, there's a disk failure, the + // piece priorities still stay the same, but the file priorities are + // possibly not fully updated. + update_piece_priorities(new_priority); + inc_refcount("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage.get() , new_priority, boost::bind(&torrent::on_file_priority, shared_from_this(), _1)); @@ -5742,6 +5743,13 @@ namespace { // storage may be NULL during shutdown if (m_storage) { + // the update of m_file_priority is deferred until the disk job comes + // back, but to preserve sanity and consistency, the piece priorities are + // updated immediately. If, on the off-chance, there's a disk failure, the + // piece priorities still stay the same, but the file priorities are + // possibly not fully updated. + update_piece_priorities(new_priority); + inc_refcount("file_priority"); m_ses.disk_thread().async_set_file_priority(m_storage.get() , new_priority, boost::bind(&torrent::on_file_priority, shared_from_this(), _1)); @@ -5787,7 +5795,7 @@ namespace { files->resize(m_torrent_file->num_files(), 4); } - void torrent::update_piece_priorities() + void torrent::update_piece_priorities(std::vector const& file_prios) { INVARIANT_CHECK; @@ -5795,7 +5803,7 @@ namespace { bool need_update = false; boost::int64_t position = 0; - int piece_length = m_torrent_file->piece_length(); + int const piece_length = m_torrent_file->piece_length(); // initialize the piece priorities to 0, then only allow // setting higher priorities std::vector pieces(m_torrent_file->num_pieces(), 0); @@ -5804,8 +5812,8 @@ namespace { { if (i >= fs.num_files()) break; - boost::int64_t start = position; - boost::int64_t size = m_torrent_file->files().file_size(i); + boost::int64_t const start = position; + boost::int64_t const size = m_torrent_file->files().file_size(i); if (size == 0) continue; position += size; int file_prio; @@ -5813,10 +5821,10 @@ namespace { // pad files always have priority 0 if (fs.pad_file_at(i)) file_prio = 0; - else if (m_file_priority.size() <= i) + else if (file_prios.size() <= i) file_prio = 4; else - file_prio = m_file_priority[i]; + file_prio = file_prios[i]; if (file_prio == 0) { @@ -5829,8 +5837,8 @@ namespace { // mark all pieces of the file with this file's priority // but only if the priority is higher than the pieces // already set (to avoid problems with overlapping pieces) - int start_piece = int(start / piece_length); - int last_piece = int((position - 1) / piece_length); + int const start_piece = int(start / piece_length); + int const last_piece = int((position - 1) / piece_length); TORRENT_ASSERT(last_piece < int(pieces.size())); // if one piece spans several files, we might // come here several times with the same start_piece, end_piece @@ -7620,21 +7628,18 @@ namespace { // piece priorities and file priorities are mutually exclusive. If there // are file priorities set, don't save piece priorities. - if (!m_file_priority.empty()) + // when in seed mode (i.e. the client promises that we have all files) + // it does not make sense to save file priorities. + if (!m_file_priority.empty() && !m_seed_mode) { - - // when in seed mode (i.e. the client promises that we have all files) - // it does not make sense to save file priorities. - if (!m_seed_mode) - { - // write file priorities - entry::list_type& file_priority = ret["file_priority"].list(); - file_priority.clear(); - for (int i = 0, end(m_file_priority.size()); i < end; ++i) - file_priority.push_back(m_file_priority[i]); - } + // write file priorities + entry::list_type& file_priority = ret["file_priority"].list(); + file_priority.clear(); + for (int i = 0, end(m_file_priority.size()); i < end; ++i) + file_priority.push_back(m_file_priority[i]); } - else if (has_picker()) + + if (has_picker()) { // write piece priorities // but only if they are not set to the default diff --git a/test/setup_transfer.cpp b/test/setup_transfer.cpp index 383507a4f..f1b473d88 100644 --- a/test/setup_transfer.cpp +++ b/test/setup_transfer.cpp @@ -74,6 +74,31 @@ namespace lt = libtorrent; #include #endif +boost::shared_ptr generate_torrent() +{ + file_storage fs; + fs.add_file("test/tmp1", 128 * 1024 * 8); + fs.add_file("test/tmp2", 128 * 1024); + fs.add_file("test/tmp3", 128 * 1024); + lt::create_torrent t(fs, 128 * 1024, 6); + + t.add_tracker("http://torrent_file_tracker.com/announce"); + t.add_url_seed("http://torrent_file_url_seed.com/"); + + int num = t.num_pieces(); + TEST_CHECK(num > 0); + for (int i = 0; i < num; ++i) + { + sha1_hash ph; + for (int k = 0; k < 20; ++k) ph[k] = lt::random(); + t.set_hash(i, ph); + } + + std::vector buf; + bencode(std::back_inserter(buf), t.generate()); + return boost::make_shared(&buf[0], buf.size()); +} + boost::uint32_t g_addr = 0x92343023; void init_rand_address() diff --git a/test/setup_transfer.hpp b/test/setup_transfer.hpp index 33b4ce69e..a31733983 100644 --- a/test/setup_transfer.hpp +++ b/test/setup_transfer.hpp @@ -44,6 +44,8 @@ namespace libtorrent class file_storage; } +EXPORT boost::shared_ptr generate_torrent(); + EXPORT int print_failures(); EXPORT unsigned char random_byte(); diff --git a/test/test_priority.cpp b/test/test_priority.cpp index 1c9943645..bf9a6ed52 100644 --- a/test/test_priority.cpp +++ b/test/test_priority.cpp @@ -502,3 +502,48 @@ TORRENT_TEST(export_file_while_seed) TEST_CHECK(exists("temporary")); } +TORRENT_TEST(test_piece_priority_after_resume) +{ + int const new_prio = 1; + + std::vector fast_resume_buf; + + boost::shared_ptr ti = generate_torrent(); + { + int const prio = 6; + + add_torrent_params p; + p.save_path = "."; + p.ti = ti; + p.file_priorities.resize(1, prio); + + lt::session ses(settings()); + torrent_handle h = ses.add_torrent(p); + + TEST_EQUAL(h.piece_priority(0), prio); + + std::vector > piece_prios; + piece_prios.push_back(std::make_pair(0, new_prio)); + h.prioritize_pieces(piece_prios); + TEST_EQUAL(h.piece_priority(0), new_prio); + + ses.pause(); + h.save_resume_data(); + + alert const* a = wait_for_alert(ses, save_resume_data_alert::alert_type); + save_resume_data_alert const* rd = alert_cast(a); + + bencode(std::back_inserter(fast_resume_buf), *(rd->resume_data)); + } + { + add_torrent_params p; + p.save_path = "."; + p.ti = ti; + p.resume_data = fast_resume_buf; + + lt::session ses(settings()); + torrent_handle h = ses.add_torrent(p); + + TEST_EQUAL(h.piece_priority(0), new_prio); + } +} diff --git a/test/test_resume.cpp b/test/test_resume.cpp index 639a9ed8b..4d3ad614c 100644 --- a/test/test_resume.cpp +++ b/test/test_resume.cpp @@ -49,31 +49,6 @@ POSSIBILITY OF SUCH DAMAGE. using namespace libtorrent; namespace lt = libtorrent; -boost::shared_ptr generate_torrent() -{ - file_storage fs; - fs.add_file("test_resume/tmp1", 128 * 1024 * 8); - fs.add_file("test_resume/tmp2", 128 * 1024); - fs.add_file("test_resume/tmp3", 128 * 1024); - lt::create_torrent t(fs, 128 * 1024, 6); - - t.add_tracker("http://torrent_file_tracker.com/announce"); - t.add_url_seed("http://torrent_file_url_seed.com/"); - - int num = t.num_pieces(); - TEST_CHECK(num > 0); - for (int i = 0; i < num; ++i) - { - sha1_hash ph; - for (int k = 0; k < 20; ++k) ph[k] = lt::random(); - t.set_hash(i, ph); - } - - std::vector buf; - bencode(std::back_inserter(buf), t.generate()); - return boost::make_shared(&buf[0], buf.size()); -} - std::vector generate_resume_data(torrent_info* ti , char const* file_priorities = "") { @@ -382,8 +357,6 @@ TORRENT_TEST(file_priorities_seed_mode) TORRENT_TEST(zero_file_prio) { - fprintf(stderr, "test_file_prio\n"); - lt::session ses(settings()); boost::shared_ptr ti = generate_torrent(); add_torrent_params p; @@ -397,17 +370,10 @@ TORRENT_TEST(zero_file_prio) rd["info-hash"] = ti->info_hash().to_string(); rd["blocks per piece"] = (std::max)(1, ti->piece_length() / 0x4000); - entry::list_type& file_prio = rd["file_priority"].list(); - for (int i = 0; i < 100; ++i) - { - file_prio.push_back(entry(0)); - } + // set file priorities to 0 + rd["file_priority"] = entry::list_type(100, entry(0)); - std::string pieces(ti->num_pieces(), '\x01'); - rd["pieces"] = pieces; - - std::string pieces_prio(ti->num_pieces(), '\x01'); - rd["piece_priority"] = pieces_prio; + rd["pieces"] = std::string(ti->num_pieces(), '\x01'); bencode(back_inserter(p.resume_data), rd); @@ -417,6 +383,37 @@ TORRENT_TEST(zero_file_prio) TEST_EQUAL(s.total_wanted, 0); } +TORRENT_TEST(mixing_file_and_piece_prio) +{ + lt::session ses(settings()); + boost::shared_ptr ti = generate_torrent(); + add_torrent_params p; + p.ti = ti; + p.save_path = "."; + + entry rd; + + rd["file-format"] = "libtorrent resume file"; + rd["file-version"] = 1; + rd["info-hash"] = ti->info_hash().to_string(); + rd["blocks per piece"] = (std::max)(1, ti->piece_length() / 0x4000); + + // set file priorities to 0 + rd["file_priority"] = entry::list_type(100, entry(0)); + + rd["pieces"] = std::string(ti->num_pieces(), '\x01'); + + // but set the piece priorities to 1. these take precedence + rd["piece_priority"] = std::string(ti->num_pieces(), '\x01'); + + bencode(back_inserter(p.resume_data), rd); + + torrent_handle h = ses.add_torrent(p); + + torrent_status s = h.status(); + TEST_EQUAL(s.total_wanted, ti->total_size()); +} + void test_seed_mode(bool file_prio, bool pieces_have, bool piece_prio , bool all_files_zero = false) { diff --git a/test/test_torrent.cpp b/test/test_torrent.cpp index f7f1eba89..49bf5ce50 100644 --- a/test/test_torrent.cpp +++ b/test/test_torrent.cpp @@ -220,13 +220,15 @@ TORRENT_TEST(total_wanted) torrent_handle h = ses.add_torrent(p); torrent_status st = h.status(); - std::cout << "total_wanted: " << st.total_wanted << " : " << 1024 << std::endl; TEST_EQUAL(st.total_wanted, 1024); - std::cout << "total_wanted_done: " << st.total_wanted_done << " : 0" << std::endl; TEST_EQUAL(st.total_wanted_done, 0); + // make sure that selecting and unseleting a file quickly still end up with + // the last set priority h.file_priority(1, 4); h.file_priority(1, 0); + + TEST_EQUAL(h.status(0).total_wanted, 0); TEST_CHECK(wait_priority(h, std::vector(fs.num_files()))); TEST_EQUAL(h.status(0).total_wanted, 0); }