From 96843fbae477e3086ea372f9cefca26ce9cc2d06 Mon Sep 17 00:00:00 2001 From: Rodrigo Martins Date: Tue, 22 Dec 2020 21:47:52 -0300 Subject: [PATCH] Take advantage of fast networks for file transfers - Make sender send more data per iteration. - Make receiver iterate more often while receiving. Before this commit tox would send at maximum around 4MiB/s. With this patch sustained speeds of up to 100MiB/s were observed on a low-latency, high-bandwidth network. As a consequence of iterating more frequently the receiver's CPU usage is increased for the duration of the transfer. The data structures used to represent friends and file transfers cause the sender code use costly loops that do little real work. This patch makes this problem more visible: the sender uses more CPU while sending. Poor network conditions were simulated using the netem kernel facility: $ tc qdisc add dev lo root netem delay 100ms 50ms \ loss 1% duplicate 1% corrupt 1% reorder 25% 50% and no adverse behavior was encountered. Tests were conducted using toxic using both UDP and TCP. --- auto_tests/set_name_test.c | 4 +++- toxcore/Messenger.c | 27 ++++++++++++++++++++++++++- toxcore/Messenger.h | 4 ++++ toxcore/tox.c | 5 +++++ 4 files changed, 38 insertions(+), 2 deletions(-) diff --git a/auto_tests/set_name_test.c b/auto_tests/set_name_test.c index 384e22a82..1dba7d103 100644 --- a/auto_tests/set_name_test.c +++ b/auto_tests/set_name_test.c @@ -16,7 +16,9 @@ static void nickchange_callback(Tox *tox, uint32_t friendnumber, const uint8_t *string, size_t length, void *userdata) { - ck_assert_msg(length == sizeof(NICKNAME) && memcmp(string, NICKNAME, sizeof(NICKNAME)) == 0, "Name not correct"); + ck_assert_msg(length == sizeof(NICKNAME), "Name length not correct: %d != %d", (uint16_t)length, + (uint16_t)sizeof(NICKNAME)); + ck_assert_msg(memcmp(string, NICKNAME, sizeof(NICKNAME)) == 0, "Name not correct: %s", (const char *)string); bool *nickname_updated = (bool *)userdata; *nickname_updated = true; } diff --git a/toxcore/Messenger.c b/toxcore/Messenger.c index 0a716c7da..acd482971 100644 --- a/toxcore/Messenger.c +++ b/toxcore/Messenger.c @@ -1596,7 +1596,7 @@ static void do_reqchunk_filecb(Messenger *m, int32_t friendnumber, void *userdat // that the file transfer has finished and may end up in an infinite loop. // // Request up to that number of chunks per file from the client - const uint32_t max_ft_loops = 16; + const uint32_t max_ft_loops = 4096; for (uint32_t i = 0; i < max_ft_loops; ++i) { if (!do_all_filetransfers(m, friendnumber, userdata, &free_slots)) { @@ -3296,6 +3296,7 @@ Messenger *new_messenger(Mono_Time *mono_time, Messenger_Options *options, unsig set_filter_function(m->fr, &friend_already_added, m); m->lastdump = 0; + m->is_receiving_file = 0; m_register_default_plugins(m); @@ -3338,3 +3339,27 @@ void kill_messenger(Messenger *m) free(m->options.state_plugins); free(m); } + +bool is_receiving_file(Messenger *m) +{ + // Only run the expensive loop below once every 64 tox_iterate calls. + const uint8_t skip_count = 64; + + if (m->is_receiving_file != 0) { + --m->is_receiving_file; + return true; + } + + // TODO(iphydf): This is a very expensive loop. Consider keeping track of + // the number of live file transfers. + for (size_t friend_number = 0; friend_number < m->numfriends; ++friend_number) { + for (size_t i = 0; i < MAX_CONCURRENT_FILE_PIPES; ++i) { + if (m->friendlist[friend_number].file_receiving[i].status == FILESTATUS_TRANSFERRING) { + m->is_receiving_file = skip_count; + return true; + } + } + } + + return false; +} diff --git a/toxcore/Messenger.h b/toxcore/Messenger.h index a7e7a6cd8..c768d2b2f 100644 --- a/toxcore/Messenger.h +++ b/toxcore/Messenger.h @@ -259,6 +259,7 @@ struct Messenger { uint32_t numfriends; time_t lastdump; + uint8_t is_receiving_file; bool has_added_relays; // If the first connection has occurred in do_messenger @@ -818,4 +819,7 @@ uint32_t count_friendlist(const Messenger *m); non_null() uint32_t copy_friendlist(const Messenger *m, uint32_t *out_list, uint32_t list_size); +non_null() +bool is_receiving_file(Messenger *m); + #endif diff --git a/toxcore/tox.c b/toxcore/tox.c index 961fe57b7..9d9df70db 100644 --- a/toxcore/tox.c +++ b/toxcore/tox.c @@ -852,6 +852,11 @@ uint32_t tox_iteration_interval(const Tox *tox) assert(tox != nullptr); lock(tox); uint32_t ret = messenger_run_interval(tox->m); + + if (is_receiving_file(tox->m)) { + ret = 1; + } + unlock(tox); return ret; }