From 9d3cf5e68beff7b78248a2bce87ab9b939055d92 Mon Sep 17 00:00:00 2001 From: Alden Torres Date: Mon, 6 Jun 2016 18:22:33 -0400 Subject: [PATCH] changed min_distance_exp for correctness and clarity (#789) changed min_distance_exp for correctness and clarity --- include/libtorrent/alert_types.hpp | 4 +- include/libtorrent/kademlia/dht_storage.hpp | 2 + simulation/test_dht_storage.cpp | 40 +++++++----- src/kademlia/node_id.cpp | 16 ++--- test/test_dht.cpp | 2 - test/test_dht_storage.cpp | 71 +++++++++++---------- 6 files changed, 69 insertions(+), 66 deletions(-) diff --git a/include/libtorrent/alert_types.hpp b/include/libtorrent/alert_types.hpp index 755fe1e49..039791f46 100644 --- a/include/libtorrent/alert_types.hpp +++ b/include/libtorrent/alert_types.hpp @@ -1334,8 +1334,8 @@ namespace libtorrent }; // This alert is posted when the listen port succeeds to be opened on a - // particular interface. ``endpoint`` is the endpoint that successfully - // was opened for listening. + // particular interface. ``address`` and ``port`` is the endpoint that + // successfully was opened for listening. struct TORRENT_EXPORT listen_succeeded_alert final : alert { enum socket_type_t { tcp, tcp_ssl, udp, i2p, socks5, utp_ssl }; diff --git a/include/libtorrent/kademlia/dht_storage.hpp b/include/libtorrent/kademlia/dht_storage.hpp index 5133431e9..be9b181f1 100644 --- a/include/libtorrent/kademlia/dht_storage.hpp +++ b/include/libtorrent/kademlia/dht_storage.hpp @@ -76,6 +76,8 @@ namespace dht // libtorrent comes with one built-in storage implementation: // ``dht_default_storage`` (private non-accessible class). Its // constructor function is called dht_default_storage_constructor(). + // You should know that if this storage becomes full of DHT items, + // the current implementation could degrade in performance. // struct TORRENT_EXPORT dht_storage_interface { diff --git a/simulation/test_dht_storage.cpp b/simulation/test_dht_storage.cpp index 14ce9e0f3..4691d047d 100644 --- a/simulation/test_dht_storage.cpp +++ b/simulation/test_dht_storage.cpp @@ -30,6 +30,8 @@ POSSIBILITY OF SUCH DAMAGE. */ +#ifndef TORRENT_DISABLE_DHT + #include "test.hpp" #include "settings.hpp" #include "setup_transfer.hpp" // for ep() @@ -56,15 +58,13 @@ using sim::default_config; using namespace std::placeholders; -#ifndef TORRENT_DISABLE_DHT - namespace { dht_settings test_settings() { dht_settings sett; sett.max_torrents = 2; sett.max_dht_items = 2; - sett.item_lifetime = seconds(120 * 60).count(); + sett.item_lifetime = int(seconds(120 * 60).count()); return sett; } @@ -75,9 +75,20 @@ namespace TORRENT_ASSERT(!hash.fail()); return ret; } + + std::unique_ptr create_default_dht_storage( + dht_settings const& sett) + { + std::unique_ptr s(dht_default_storage_constructor(sett)); + TEST_CHECK(s.get() != nullptr); + + s->update_node_ids({to_hash("0000000000000000000000000000000000000200")}); + + return s; + } } -void timer_tick(boost::shared_ptr s +void timer_tick(dht_storage_interface* s , dht_storage_counters const& c , boost::system::error_code const& ec) { @@ -91,7 +102,7 @@ void timer_tick(boost::shared_ptr s } void test_expiration(high_resolution_clock::duration const& expiry_time - , boost::shared_ptr s + , std::unique_ptr& s , dht_storage_counters const& c) { default_config cfg; @@ -100,26 +111,23 @@ void test_expiration(high_resolution_clock::duration const& expiry_time sim::asio::high_resolution_timer timer(ios); timer.expires_from_now(expiry_time); - timer.async_wait(std::bind(&timer_tick, s, c, _1)); + timer.async_wait(std::bind(&timer_tick, s.get(), c, _1)); boost::system::error_code ec; sim.run(ec); } -#endif // TORRENT_DISABLE_DHT - TORRENT_TEST(dht_storage_counters) { -#ifndef TORRENT_DISABLE_DHT dht_settings sett = test_settings(); - boost::shared_ptr s(dht_default_storage_constructor(sett)); + std::unique_ptr s(create_default_dht_storage(sett)); - TEST_CHECK(s.get() != NULL); + TEST_CHECK(s.get() != nullptr); - sha1_hash n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); - sha1_hash n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); - sha1_hash n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); - sha1_hash n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); + sha1_hash const n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); + sha1_hash const n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); + sha1_hash const n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); + sha1_hash const n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); tcp::endpoint const p1 = ep("124.31.75.21", 1); tcp::endpoint const p2 = ep("124.31.75.22", 1); @@ -162,6 +170,6 @@ TORRENT_TEST(dht_storage_counters) c.immutable_data = 0; c.mutable_data = 0; test_expiration(hours(1), s, c); // test expiration of everything after 3 hours -#endif // TORRENT_DISABLE_DHT } +#endif // TORRENT_DISABLE_DHT diff --git a/src/kademlia/node_id.cpp b/src/kademlia/node_id.cpp index e83e90464..0e1c9bfb6 100644 --- a/src/kademlia/node_id.cpp +++ b/src/kademlia/node_id.cpp @@ -31,11 +31,9 @@ POSSIBILITY OF SUCH DAMAGE. */ #include -#include #include "libtorrent/kademlia/node_id.hpp" #include "libtorrent/kademlia/node_entry.hpp" -#include "libtorrent/hasher.hpp" #include "libtorrent/assert.hpp" #include "libtorrent/broadcast_socket.hpp" // for is_local et.al #include "libtorrent/socket_io.hpp" // for hash_address @@ -73,18 +71,12 @@ int distance_exp(node_id const& n1, node_id const& n2) int min_distance_exp(node_id const& n1, std::vector const& ids) { - // specialized for cases of 0, 1 and 2 for performance reasons - if (ids.size() == 0) return 0; - if (ids.size() == 1) return distance_exp(n1, ids[0]); - if (ids.size() == 2) - return std::min(distance_exp(n1, ids[0]), distance_exp(n1, ids[1])); + TORRENT_ASSERT(ids.size() > 0); - int min = std::numeric_limits::max(); - for (const auto &node_id : ids) + int min = 160; // see distance_exp for the why of this constant + for (auto const& node_id : ids) { - int d = distance_exp(n1, node_id); - if (d < min) - min = d; + min = std::min(min, distance_exp(n1, node_id)); } return min; diff --git a/test/test_dht.cpp b/test/test_dht.cpp index 5289b1645..8af344107 100644 --- a/test/test_dht.cpp +++ b/test/test_dht.cpp @@ -2712,8 +2712,6 @@ TORRENT_TEST(node_id_min_distance_exp) std::vector ids; - TEST_EQUAL(min_distance_exp(sha1_hash::min(), ids), 0); - ids.push_back(n1); TEST_EQUAL(min_distance_exp(sha1_hash::min(), ids), 1); diff --git a/test/test_dht_storage.cpp b/test/test_dht_storage.cpp index c7b0ab528..53f32b4c7 100644 --- a/test/test_dht_storage.cpp +++ b/test/test_dht_storage.cpp @@ -83,18 +83,28 @@ namespace g_storage_constructor_invoked = true; return dht_default_storage_constructor(settings); } + + std::unique_ptr create_default_dht_storage( + dht_settings const& sett) + { + std::unique_ptr s(dht_default_storage_constructor(sett)); + TEST_CHECK(s.get() != nullptr); + + s->update_node_ids({to_hash("0000000000000000000000000000000000000200")}); + + return s; + } } -const sha1_hash n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); -const sha1_hash n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); -const sha1_hash n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); -const sha1_hash n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); +sha1_hash const n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); +sha1_hash const n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); +sha1_hash const n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); +sha1_hash const n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); TORRENT_TEST(announce_peer) { dht_settings sett = test_settings(); - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); entry peers; s->get_peers(n1, false, false, peers); @@ -102,10 +112,10 @@ TORRENT_TEST(announce_peer) TEST_CHECK(peers["n"].string().empty()) TEST_CHECK(peers["values"].list().empty()); - tcp::endpoint p1 = ep("124.31.75.21", 1); - tcp::endpoint p2 = ep("124.31.75.22", 1); - tcp::endpoint p3 = ep("124.31.75.23", 1); - tcp::endpoint p4 = ep("124.31.75.24", 1); + tcp::endpoint const p1 = ep("124.31.75.21", 1); + tcp::endpoint const p2 = ep("124.31.75.22", 1); + tcp::endpoint const p3 = ep("124.31.75.23", 1); + tcp::endpoint const p4 = ep("124.31.75.24", 1); s->announce_peer(n1, p1, "torrent_name", false); s->get_peers(n1, false, false, peers); @@ -122,8 +132,7 @@ TORRENT_TEST(announce_peer) TORRENT_TEST(put_immutable_item) { dht_settings sett = test_settings(); - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); entry item; bool r = s->get_immutable_item(n4, item); @@ -152,22 +161,20 @@ TORRENT_TEST(put_immutable_item) TORRENT_TEST(counters) { dht_settings sett = test_settings(); - std::unique_ptr s(dht_default_storage_constructor(sett)); + std::unique_ptr s(create_default_dht_storage(sett)); - TEST_CHECK(s.get() != NULL); - - sha1_hash n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); - sha1_hash n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); - sha1_hash n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); - sha1_hash n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); + sha1_hash const n1 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee401"); + sha1_hash const n2 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee402"); + sha1_hash const n3 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee403"); + sha1_hash const n4 = to_hash("5fbfbff10c5d6a4ec8a88e4c6ab4c28b95eee404"); TEST_EQUAL(s->counters().peers, 0); TEST_EQUAL(s->counters().torrents, 0); - tcp::endpoint p1 = ep("124.31.75.21", 1); - tcp::endpoint p2 = ep("124.31.75.22", 1); - tcp::endpoint p3 = ep("124.31.75.23", 1); - tcp::endpoint p4 = ep("124.31.75.24", 1); + tcp::endpoint const p1 = ep("124.31.75.21", 1); + tcp::endpoint const p2 = ep("124.31.75.22", 1); + tcp::endpoint const p3 = ep("124.31.75.23", 1); + tcp::endpoint const p4 = ep("124.31.75.24", 1); s->announce_peer(n1, p1, "torrent_name", false); TEST_EQUAL(s->counters().peers, 1); @@ -241,8 +248,7 @@ TORRENT_TEST(peer_limit) { dht_settings sett = test_settings(); sett.max_peers = 42; - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); for (int i = 0; i < 200; ++i) { @@ -259,8 +265,7 @@ TORRENT_TEST(torrent_limit) { dht_settings sett = test_settings(); sett.max_torrents = 42; - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); for (int i = 0; i < 200; ++i) { @@ -277,8 +282,7 @@ TORRENT_TEST(immutable_item_limit) { dht_settings sett = test_settings(); sett.max_dht_items = 42; - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); for (int i = 0; i < 200; ++i) { @@ -294,8 +298,7 @@ TORRENT_TEST(mutable_item_limit) { dht_settings sett = test_settings(); sett.max_dht_items = 42; - std::unique_ptr s(dht_default_storage_constructor(sett)); - TEST_CHECK(s.get() != NULL); + std::unique_ptr s(create_default_dht_storage(sett)); char public_key[item_pk_len]; char signature[item_sig_len]; @@ -329,9 +332,9 @@ TORRENT_TEST(update_node_ids) dht_storage_counters cnt; bool r; - sha1_hash h1 = to_hash("0000000000000000000000000000000000010200"); - sha1_hash h2 = to_hash("0000000000000000000000000000000100000400"); - sha1_hash h3 = to_hash("0000000000000000000000010000000000000800"); + sha1_hash const h1 = to_hash("0000000000000000000000000000000000010200"); + sha1_hash const h2 = to_hash("0000000000000000000000000000000100000400"); + sha1_hash const h3 = to_hash("0000000000000000000000010000000000000800"); TEST_EQUAL(min_distance_exp(h1, node_ids), 16); TEST_EQUAL(min_distance_exp(h2, node_ids), 32);