From 6321d77a3a72125c9985298013e7e00849e2535c Mon Sep 17 00:00:00 2001 From: kannibalox Date: Sun, 1 Dec 2024 13:34:35 -0500 Subject: [PATCH 1/3] Convert __sync* built-ins to std::atomic Also convert thread_base to use std::mutex instead of pthread_mutex --- src/thread_disk.cc | 2 +- src/torrent/poll_epoll.cc | 2 - src/torrent/poll_kqueue.cc | 2 - src/torrent/poll_select.cc | 2 - src/torrent/utils/signal_bitfield.cc | 4 +- src/torrent/utils/signal_bitfield.h | 14 ++-- src/torrent/utils/thread_base.cc | 13 +-- src/torrent/utils/thread_base.h | 51 +++--------- src/torrent/utils/thread_interrupt.cc | 6 +- src/torrent/utils/thread_interrupt.h | 3 +- src/utils/instrumentation.cc | 97 ++++++++-------------- src/utils/instrumentation.h | 5 +- test/helpers/test_thread.cc | 10 +-- test/helpers/test_thread.h | 10 +-- test/torrent/task_manager_test.cc | 14 ++-- test/torrent/utils/test_signal_bitfield.cc | 14 ++-- test/torrent/utils/test_thread_base.cc | 1 - 17 files changed, 101 insertions(+), 149 deletions(-) diff --git a/src/thread_disk.cc b/src/thread_disk.cc index ff5b3073f..5d0c64df4 100644 --- a/src/thread_disk.cc +++ b/src/thread_disk.cc @@ -67,7 +67,7 @@ thread_disk::call_events() { if ((m_flags & flag_did_shutdown)) throw internal_error("Already trigged shutdown."); - __sync_or_and_fetch(&m_flags, flag_did_shutdown); + m_flags |= flag_did_shutdown; throw shutdown_exception(); } diff --git a/src/torrent/poll_epoll.cc b/src/torrent/poll_epoll.cc index 345a7365e..0b5b9a42f 100644 --- a/src/torrent/poll_epoll.cc +++ b/src/torrent/poll_epoll.cc @@ -203,13 +203,11 @@ PollEPoll::do_poll(int64_t timeout_usec, int flags) { if (!(flags & poll_worker_thread)) { thread_base::release_global_lock(); - thread_base::entering_main_polling(); } int status = poll((timeout.usec() + 999) / 1000); if (!(flags & poll_worker_thread)) { - thread_base::leaving_main_polling(); thread_base::acquire_global_lock(); } diff --git a/src/torrent/poll_kqueue.cc b/src/torrent/poll_kqueue.cc index 6bd4d02da..0e90e1fe2 100644 --- a/src/torrent/poll_kqueue.cc +++ b/src/torrent/poll_kqueue.cc @@ -262,13 +262,11 @@ PollKQueue::do_poll(int64_t timeout_usec, int flags) { if (!(flags & poll_worker_thread)) { thread_base::release_global_lock(); - thread_base::entering_main_polling(); } int status = poll((timeout.usec() + 999) / 1000); if (!(flags & poll_worker_thread)) { - thread_base::leaving_main_polling(); thread_base::acquire_global_lock(); } diff --git a/src/torrent/poll_select.cc b/src/torrent/poll_select.cc index dc802c3cd..def737a1f 100644 --- a/src/torrent/poll_select.cc +++ b/src/torrent/poll_select.cc @@ -243,14 +243,12 @@ PollSelect::do_poll(int64_t timeout_usec, int flags) { timeval t = timeout.tval(); if (!(flags & poll_worker_thread)) { - thread_base::entering_main_polling(); thread_base::release_global_lock(); } int status = select(maxFd + 1, read_set, write_set, error_set, &t); if (!(flags & poll_worker_thread)) { - thread_base::leaving_main_polling(); thread_base::acquire_global_lock(); } diff --git a/src/torrent/utils/signal_bitfield.cc b/src/torrent/utils/signal_bitfield.cc index dfc3d1feb..c1283d8ef 100644 --- a/src/torrent/utils/signal_bitfield.cc +++ b/src/torrent/utils/signal_bitfield.cc @@ -18,7 +18,7 @@ signal_bitfield::add_signal(slot_type slot) { throw internal_error("signal_bitfield::add_signal(...): Cannot add empty slot."); unsigned int index = m_size; - __sync_add_and_fetch(&m_size, 1); + ++m_size; m_slots[index] = slot; return index; @@ -28,7 +28,7 @@ void signal_bitfield::work() { bitfield_type bitfield; - while (!__sync_bool_compare_and_swap(&m_bitfield, (bitfield = m_bitfield), 0)) + while (!m_bitfield.compare_exchange_weak(bitfield, 0)) ; // Do nothing. unsigned int i = 0; diff --git a/src/torrent/utils/signal_bitfield.h b/src/torrent/utils/signal_bitfield.h index ffa336d2c..94499a582 100644 --- a/src/torrent/utils/signal_bitfield.h +++ b/src/torrent/utils/signal_bitfield.h @@ -37,7 +37,10 @@ #ifndef LIBTORRENT_UTILS_SIGNAL_BITFIELD_H #define LIBTORRENT_UTILS_SIGNAL_BITFIELD_H +#include "config.h" + #include +#include #include @@ -48,22 +51,21 @@ class LIBTORRENT_EXPORT lt_cacheline_aligned signal_bitfield { typedef uint32_t bitfield_type; typedef std::function slot_type; - static const unsigned int max_size = 32; + static constexpr unsigned int max_size = 32; - signal_bitfield() : m_bitfield(0), m_size(0) {} + signal_bitfield() = default; bool has_signal(unsigned int index) const { return m_bitfield & (1 << index); } // Do the interrupt from the thread? - void signal(unsigned int index) { __sync_or_and_fetch(&m_bitfield, 1 << index); } + void signal(unsigned int index) { m_bitfield |= 1 << index; } void work(); unsigned int add_signal(slot_type slot); private: - - bitfield_type m_bitfield; - unsigned int m_size; + std::atomic m_bitfield{0}; + std::atomic m_size{0}; slot_type m_slots[max_size] lt_cacheline_aligned; }; diff --git a/src/torrent/utils/thread_base.cc b/src/torrent/utils/thread_base.cc index ec0619f37..1462f6a22 100644 --- a/src/torrent/utils/thread_base.cc +++ b/src/torrent/utils/thread_base.cc @@ -13,7 +13,8 @@ namespace torrent { -thread_base::global_lock_type lt_cacheline_aligned thread_base::m_global = { 0, 0, PTHREAD_MUTEX_INITIALIZER }; +std::mutex thread_base::m_globalLock; +std::atomic thread_base::m_waiting; thread_base::thread_base() : m_state(STATE_UNKNOWN), @@ -56,7 +57,7 @@ thread_base::start_thread() { void thread_base::stop_thread() { - __sync_fetch_and_or(&m_flags, flag_do_shutdown); + m_flags |= flag_do_shutdown; interrupt(); } @@ -94,7 +95,7 @@ thread_base::event_loop(thread_base* thread) { if (!thread->is_initialized()) throw internal_error("thread_base::event_loop call on an uninitialized object"); - __sync_lock_test_and_set(&thread->m_state, STATE_ACTIVE); + thread->m_state = STATE_ACTIVE; #if defined(HAS_PTHREAD_SETNAME_NP_DARWIN) pthread_setname_np(thread->name()); @@ -117,7 +118,7 @@ thread_base::event_loop(thread_base* thread) { thread->call_events(); thread->signal_bitfield()->work(); - __sync_fetch_and_or(&thread->m_flags, flag_polling); + thread->m_flags |= flag_polling; // Call again after setting flag_polling to ensure we process // any events set while it was working. @@ -152,7 +153,7 @@ thread_base::event_loop(thread_base* thread) { instrumentation_update(INSTRUMENTATION_POLLING_EVENTS, event_count); instrumentation_update(instrumentation_enum(INSTRUMENTATION_POLLING_EVENTS + thread->m_instrumentation_index), event_count); - __sync_fetch_and_and(&thread->m_flags, ~(flag_polling | flag_no_timeout)); + thread->m_flags &= ~(flag_polling | flag_no_timeout); } // #ifdef USE_INTERRUPT_SOCKET @@ -163,7 +164,7 @@ thread_base::event_loop(thread_base* thread) { lt_log_print(torrent::LOG_THREAD_NOTICE, "%s: Shutting down thread.", thread->name()); } - __sync_lock_test_and_set(&thread->m_state, STATE_INACTIVE); + thread->m_state = STATE_INACTIVE; return NULL; } diff --git a/src/torrent/utils/thread_base.h b/src/torrent/utils/thread_base.h index bead96592..a7dd05521 100644 --- a/src/torrent/utils/thread_base.h +++ b/src/torrent/utils/thread_base.h @@ -2,6 +2,7 @@ #define LIBTORRENT_UTILS_THREAD_BASE_H #import +#import #import #import @@ -69,36 +70,29 @@ class LIBTORRENT_EXPORT lt_cacheline_aligned thread_base { slot_void& slot_do_work() { return m_slot_do_work; } slot_timer& slot_next_timeout() { return m_slot_next_timeout; } - static inline int global_queue_size() { return m_global.waiting; } + static inline int global_queue_size() { return m_waiting; } static inline void acquire_global_lock(); static inline bool trylock_global_lock(); static inline void release_global_lock(); static inline void waive_global_lock(); - static inline bool is_main_polling() { return m_global.main_polling; } - static inline void entering_main_polling(); - static inline void leaving_main_polling(); - static bool should_handle_sigusr1(); static void* event_loop(thread_base* thread); protected: - struct lt_cacheline_aligned global_lock_type { - int waiting; - int main_polling; - pthread_mutex_t lock; - }; + static std::mutex lt_cacheline_aligned m_globalLock; + static std::atomic lt_cacheline_aligned m_waiting; virtual void call_events() = 0; virtual int64_t next_timeout_usec() = 0; - static global_lock_type m_global; pthread_t m_thread; - state_type m_state lt_cacheline_aligned; - int m_flags lt_cacheline_aligned; + + std::atomic lt_cacheline_aligned m_state{STATE_UNKNOWN}; + std::atomic lt_cacheline_aligned m_flags{0}; int m_instrumentation_index; @@ -124,13 +118,11 @@ thread_base::is_current() const { inline int thread_base::flags() const { - __sync_synchronize(); return m_flags; } inline thread_base::state_type thread_base::state() const { - __sync_synchronize(); return m_state; } @@ -144,46 +136,29 @@ thread_base::send_event_signal(unsigned int index, bool do_interrupt) { inline void thread_base::acquire_global_lock() { - __sync_add_and_fetch(&thread_base::m_global.waiting, 1); - pthread_mutex_lock(&thread_base::m_global.lock); - __sync_sub_and_fetch(&thread_base::m_global.waiting, 1); + ++m_waiting; + m_globalLock.lock(); + --m_waiting; } inline bool thread_base::trylock_global_lock() { - return pthread_mutex_trylock(&thread_base::m_global.lock) == 0; + return thread_base::m_globalLock.try_lock(); } inline void thread_base::release_global_lock() { - pthread_mutex_unlock(&thread_base::m_global.lock); + return thread_base::m_globalLock.unlock(); } inline void thread_base::waive_global_lock() { - pthread_mutex_unlock(&thread_base::m_global.lock); + thread_base::m_globalLock.unlock(); // Do we need to sleep here? Make a CppUnit test for this. acquire_global_lock(); } -// 'entering/leaving_main_polling' is used by the main polling thread -// to indicate to other threads when it is safe to change the main -// thread's event entries. -// -// A thread should first aquire global lock, then if it needs to -// change poll'ed sockets on the main thread it should call -// 'interrupt_main_polling' unless 'is_main_polling() == false'. -inline void -thread_base::entering_main_polling() { - __sync_lock_test_and_set(&thread_base::m_global.main_polling, 1); -} - -inline void -thread_base::leaving_main_polling() { - __sync_lock_test_and_set(&thread_base::m_global.main_polling, 0); -} - } #endif diff --git a/src/torrent/utils/thread_interrupt.cc b/src/torrent/utils/thread_interrupt.cc index 27426d9c8..d3fdbda58 100644 --- a/src/torrent/utils/thread_interrupt.cc +++ b/src/torrent/utils/thread_interrupt.cc @@ -63,7 +63,8 @@ thread_interrupt::~thread_interrupt() { bool thread_interrupt::poke() { - if (!__sync_bool_compare_and_swap(&m_other->m_poking, false, true)) + bool flag = false; + if (!m_other->m_poking.compare_exchange_strong(flag, true)) return true; int result = ::send(m_fileDesc, "a", 1, 0); @@ -104,7 +105,8 @@ thread_interrupt::event_read() { instrumentation_update(INSTRUMENTATION_POLLING_INTERRUPT_READ_EVENT, 1); - __sync_bool_compare_and_swap(&m_poking, true, false); + m_poking = false; } } + diff --git a/src/torrent/utils/thread_interrupt.h b/src/torrent/utils/thread_interrupt.h index 0d388027b..f5ae7e4d9 100644 --- a/src/torrent/utils/thread_interrupt.h +++ b/src/torrent/utils/thread_interrupt.h @@ -38,6 +38,7 @@ #define LIBTORRENT_UTILS_THREAD_INTERRUPT_H #include +#include #include namespace torrent { @@ -66,7 +67,7 @@ class LIBTORRENT_EXPORT lt_cacheline_aligned thread_interrupt : public Event { SocketFd& get_fd() { return *reinterpret_cast(&m_fileDesc); } thread_interrupt* m_other; - bool m_poking lt_cacheline_aligned; + std::atomic m_poking lt_cacheline_aligned; }; inline bool diff --git a/src/utils/instrumentation.cc b/src/utils/instrumentation.cc index 178d6a19a..5251d0043 100644 --- a/src/utils/instrumentation.cc +++ b/src/utils/instrumentation.cc @@ -1,70 +1,39 @@ -// libTorrent - BitTorrent library -// Copyright (C) 2005-2011, Jari Sundell -// -// This program is free software; you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation; either version 2 of the License, or -// (at your option) any later version. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program; if not, write to the Free Software -// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA -// -// In addition, as a special exception, the copyright holders give -// permission to link the code of portions of this program with the -// OpenSSL library under certain conditions as described in each -// individual source file, and distribute linked combinations -// including the two. -// -// You must obey the GNU General Public License in all respects for -// all of the code used other than OpenSSL. If you modify file(s) -// with this exception, you may extend this exception to your version -// of the file(s), but you are not obligated to do so. If you do not -// wish to do so, delete this exception statement from your version. -// If you delete this exception statement from all source files in the -// program, then also delete it here. -// -// Contact: Jari Sundell -// -// Skomakerveien 33 -// 3185 Skoppum, NORWAY - #include "config.h" +#include + #include "instrumentation.h" namespace torrent { -std::array instrumentation_values lt_cacheline_aligned; +std::array, INSTRUMENTATION_MAX_SIZE> instrumentation_values lt_cacheline_aligned; inline int64_t instrumentation_fetch_and_clear(instrumentation_enum type) { #ifdef LT_INSTRUMENTATION - return __sync_fetch_and_and(&instrumentation_values[type], int64_t()); + // Unlike most assignments, atomics don't return the original value, + // but the stored value, so we have to copy it first + auto value = instrumentation_values[type].load(); + instrumentation_values[type] = 0; + return value; #else - return instrumentation_values[type] = 0; + return 0; #endif } void instrumentation_tick() { - // Since the values are updated with __sync_add, they can be read - // without any memory barriers. +#ifdef LT_INSTRUMENTATION lt_log_print(LOG_INSTRUMENTATION_MEMORY, - "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, - instrumentation_values[INSTRUMENTATION_MEMORY_CHUNK_USAGE], - instrumentation_values[INSTRUMENTATION_MEMORY_CHUNK_COUNT], - instrumentation_values[INSTRUMENTATION_MEMORY_HASHING_CHUNK_USAGE], - instrumentation_values[INSTRUMENTATION_MEMORY_HASHING_CHUNK_COUNT], - instrumentation_values[INSTRUMENTATION_MEMORY_BITFIELDS]); + "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, + instrumentation_values[INSTRUMENTATION_MEMORY_CHUNK_USAGE].load(), + instrumentation_values[INSTRUMENTATION_MEMORY_CHUNK_COUNT].load(), + instrumentation_values[INSTRUMENTATION_MEMORY_HASHING_CHUNK_USAGE].load(), + instrumentation_values[INSTRUMENTATION_MEMORY_HASHING_CHUNK_COUNT].load(), + instrumentation_values[INSTRUMENTATION_MEMORY_BITFIELDS].load()); lt_log_print(LOG_INSTRUMENTATION_MINCORE, - "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, instrumentation_fetch_and_clear(INSTRUMENTATION_MINCORE_INCORE_TOUCHED), @@ -83,9 +52,9 @@ instrumentation_tick() { instrumentation_fetch_and_clear(INSTRUMENTATION_MINCORE_DEALLOCATIONS)); lt_log_print(LOG_INSTRUMENTATION_POLLING, - "%" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, + "%" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, instrumentation_fetch_and_clear(INSTRUMENTATION_POLLING_INTERRUPT_POKE), instrumentation_fetch_and_clear(INSTRUMENTATION_POLLING_INTERRUPT_READ_EVENT), @@ -100,11 +69,11 @@ instrumentation_tick() { instrumentation_fetch_and_clear(INSTRUMENTATION_POLLING_EVENTS_OTHERS)); lt_log_print(LOG_INSTRUMENTATION_TRANSFERS, - "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 - " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + "%" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 + " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64 " %" PRIi64, instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_DELEGATED), @@ -117,24 +86,28 @@ instrumentation_tick() { instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_QUEUED_ADDED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_QUEUED_MOVED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_QUEUED_REMOVED), - instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_QUEUED_TOTAL], + instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_QUEUED_TOTAL].load(), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_UNORDERED_ADDED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_UNORDERED_MOVED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_UNORDERED_REMOVED), - instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_UNORDERED_TOTAL], + instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_UNORDERED_TOTAL].load(), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_STALLED_ADDED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_STALLED_MOVED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_STALLED_REMOVED), - instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_STALLED_TOTAL], + instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_STALLED_TOTAL].load(), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_ADDED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_MOVED), instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_REMOVED), - instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_TOTAL], + instrumentation_values[INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_TOTAL].load(), - instrumentation_values[INSTRUMENTATION_TRANSFER_PEER_INFO_UNACCOUNTED]); + instrumentation_values[INSTRUMENTATION_TRANSFER_PEER_INFO_UNACCOUNTED].load()); +#else + lt_log_print(LOG_INSTRUMENTATION_POLLING, "rTorrent not compiled with intrumentation support"); + lt_log_print(LOG_INSTRUMENTATION_TRANSFERS, "rTorrent not compiled with intrumentation support"); +#endif } void @@ -188,4 +161,4 @@ instrumentation_reset() { instrumentation_fetch_and_clear(INSTRUMENTATION_TRANSFER_REQUESTS_CHOKED_REMOVED); } -} +} // namespace torrent diff --git a/src/utils/instrumentation.h b/src/utils/instrumentation.h index 11e77f6d5..03f650885 100644 --- a/src/utils/instrumentation.h +++ b/src/utils/instrumentation.h @@ -39,6 +39,7 @@ #include #include +#include #include "torrent/common.h" #include "torrent/utils/log.h" @@ -106,7 +107,7 @@ enum instrumentation_enum { INSTRUMENTATION_MAX_SIZE }; -extern std::array instrumentation_values lt_cacheline_aligned; +extern std::array, INSTRUMENTATION_MAX_SIZE> instrumentation_values lt_cacheline_aligned; void instrumentation_initialize(); void instrumentation_update(instrumentation_enum type, int64_t change); @@ -125,7 +126,7 @@ instrumentation_initialize() { inline void instrumentation_update(instrumentation_enum type, int64_t change) { #ifdef LT_INSTRUMENTATION - __sync_add_and_fetch(&instrumentation_values[type], change); + instrumentation_values[type] += change; #endif } diff --git a/test/helpers/test_thread.cc b/test/helpers/test_thread.cc index 4b3d4c95a..82197bdd9 100644 --- a/test/helpers/test_thread.cc +++ b/test/helpers/test_thread.cc @@ -34,19 +34,19 @@ test_thread::init_thread() { void test_thread::call_events() { if ((m_test_flags & test_flag_pre_stop) && m_test_state == TEST_PRE_START && m_state == STATE_ACTIVE) - __sync_lock_test_and_set(&m_test_state, TEST_PRE_STOP); + m_test_state = TEST_PRE_STOP; if ((m_test_flags & test_flag_acquire_global)) { acquire_global_lock(); - __sync_and_and_fetch(&m_test_flags, ~test_flag_acquire_global); - __sync_or_and_fetch(&m_test_flags, test_flag_has_global); + m_test_flags &= ~test_flag_acquire_global; + m_test_flags |= test_flag_has_global; } if ((m_flags & flag_do_shutdown)) { if ((m_flags & flag_did_shutdown)) throw torrent::internal_error("Already trigged shutdown."); - __sync_or_and_fetch(&m_flags, flag_did_shutdown); + m_flags |= flag_did_shutdown; throw torrent::shutdown_exception(); } @@ -55,7 +55,7 @@ test_thread::call_events() { if ((m_test_flags & test_flag_do_work)) { usleep(10 * 1000); // TODO: Don't just sleep, as that give up core. - __sync_and_and_fetch(&m_test_flags, ~test_flag_do_work); + m_test_flags &= ~test_flag_do_work; } if ((m_test_flags & test_flag_post_poke)) { diff --git a/test/helpers/test_thread.h b/test/helpers/test_thread.h index 520370362..322bb4cc4 100644 --- a/test/helpers/test_thread.h +++ b/test/helpers/test_thread.h @@ -31,17 +31,17 @@ class test_thread : public torrent::thread_base { void init_thread(); - void set_pre_stop() { __sync_or_and_fetch(&m_test_flags, test_flag_pre_stop); } - void set_acquire_global() { __sync_or_and_fetch(&m_test_flags, test_flag_acquire_global); } + void set_pre_stop() { m_test_flags |= test_flag_pre_stop; } + void set_acquire_global() { m_test_flags |= test_flag_acquire_global; } - void set_test_flag(int flags) { __sync_or_and_fetch(&m_test_flags, flags); } + void set_test_flag(int flags) { m_test_flags |= flags; } private: void call_events(); int64_t next_timeout_usec() { return (m_test_flags & test_flag_long_timeout) ? (10000 * 1000) : (100 * 1000); } - int m_test_state lt_cacheline_aligned; - int m_test_flags lt_cacheline_aligned; + std::atomic m_test_state lt_cacheline_aligned; + std::atomic m_test_flags lt_cacheline_aligned; }; struct thread_management_type { diff --git a/test/torrent/task_manager_test.cc b/test/torrent/task_manager_test.cc index 73b1dfd9f..6bd32be58 100644 --- a/test/torrent/task_manager_test.cc +++ b/test/torrent/task_manager_test.cc @@ -1,5 +1,7 @@ #include "config.h" +#include + #include "task_manager_test.h" CPPUNIT_TEST_SUITE_REGISTRATION(TaskManagerTest); @@ -8,7 +10,7 @@ struct TMT_Data { TMT_Data(TaskManagerTest* m) : manager(m), counter(0) {} TaskManagerTest *manager; - unsigned int counter; + std::atomic counter; }; void @@ -22,33 +24,33 @@ TaskManagerTest::tearDown() { void* TMT_Func(TMT_Data* data) { - __sync_add_and_fetch(&data->counter, 1); + data->counter++; return NULL; } void* TMT_lock_func(TMT_Data* data) { - __sync_add_and_fetch(&data->counter, 1); + data->counter++; data->manager->m_manager.acquire_global_lock(); usleep(10000); data->manager->m_manager.release_global_lock(); - __sync_add_and_fetch(&data->counter, 1); + data->counter++; return NULL; } void* TMT_waive_lock_func(TMT_Data* data) { - __sync_add_and_fetch(&data->counter, 1); + data->counter++; data->manager->m_manager.acquire_global_lock(); usleep(20000); data->manager->m_manager.waive_global_lock(); - __sync_add_and_fetch(&data->counter, 1); + data->counter++; return NULL; } diff --git a/test/torrent/utils/test_signal_bitfield.cc b/test/torrent/utils/test_signal_bitfield.cc index 4ecd18c0c..8386bd793 100644 --- a/test/torrent/utils/test_signal_bitfield.cc +++ b/test/torrent/utils/test_signal_bitfield.cc @@ -1,5 +1,7 @@ #include "config.h" +#include + #include "test_signal_bitfield.h" #include "helpers/test_thread.h" @@ -12,12 +14,12 @@ CPPUNIT_TEST_SUITE_NAMED_REGISTRATION(test_signal_bitfield, "torrent/utils"); static void -mark_index(uint32_t* bitfield, unsigned int index) { - __sync_fetch_and_or(bitfield, 1 << index); +mark_index(std::atomic* bitfield, unsigned int index) { + bitfield->fetch_or(1 << index); } static bool -check_index(uint32_t* bitfield, unsigned int index) { +check_index(std::atomic* bitfield, unsigned int index) { return *bitfield & (1 << index); } @@ -42,7 +44,7 @@ verify_did_internal_error(std::function func, bool should_throw } #define SETUP_SIGNAL_BITFIELD() \ - uint32_t marked_bitfield = 0; \ + std::atomic marked_bitfield{0}; \ torrent::signal_bitfield signal_bitfield; @@ -106,7 +108,7 @@ test_signal_bitfield::test_multiple() { void test_signal_bitfield::test_threaded() { - uint32_t marked_bitfield = 0; + std::atomic marked_bitfield{0}; test_thread* thread = new test_thread; // thread->set_test_flag(test_thread::test_flag_long_timeout); @@ -126,7 +128,7 @@ test_signal_bitfield::test_threaded() { // thread->interrupt(); CPPUNIT_ASSERT(wait_for_true(std::bind(&check_index, &marked_bitfield, i % 20))); - __sync_fetch_and_and(&marked_bitfield, ~uint32_t()); + marked_bitfield &= ~uint32_t(); } thread->stop_thread(); diff --git a/test/torrent/utils/test_thread_base.cc b/test/torrent/utils/test_thread_base.cc index 33519b7c4..8c93a851f 100644 --- a/test/torrent/utils/test_thread_base.cc +++ b/test/torrent/utils/test_thread_base.cc @@ -33,7 +33,6 @@ test_thread_base::test_basic() { CPPUNIT_ASSERT(thread->flags() == 0); - CPPUNIT_ASSERT(!thread->is_main_polling()); CPPUNIT_ASSERT(!thread->is_active()); CPPUNIT_ASSERT(thread->global_queue_size() == 0); CPPUNIT_ASSERT(thread->poll() == NULL); From 62b5dac7f5cf51d640533785b27012705495a570 Mon Sep 17 00:00:00 2001 From: kannibalox Date: Sun, 1 Dec 2024 16:16:07 -0500 Subject: [PATCH 2/3] Apply clang-tidy fixes Also remove unused TaskManager test files --- src/torrent/utils/thread_interrupt.h | 2 + src/utils/instrumentation.h | 2 + test/helpers/test_thread.cc | 24 +++--- test/helpers/test_thread.h | 20 ++--- test/torrent/task_manager_test.cc | 111 --------------------------- test/torrent/task_manager_test.h | 28 ------- 6 files changed, 26 insertions(+), 161 deletions(-) delete mode 100644 test/torrent/task_manager_test.cc delete mode 100644 test/torrent/task_manager_test.h diff --git a/src/torrent/utils/thread_interrupt.h b/src/torrent/utils/thread_interrupt.h index f5ae7e4d9..fe148c925 100644 --- a/src/torrent/utils/thread_interrupt.h +++ b/src/torrent/utils/thread_interrupt.h @@ -37,6 +37,8 @@ #ifndef LIBTORRENT_UTILS_THREAD_INTERRUPT_H #define LIBTORRENT_UTILS_THREAD_INTERRUPT_H +#include "config.h" + #include #include #include diff --git a/src/utils/instrumentation.h b/src/utils/instrumentation.h index 03f650885..e9fb519f3 100644 --- a/src/utils/instrumentation.h +++ b/src/utils/instrumentation.h @@ -37,6 +37,8 @@ #ifndef LIBTORRENT_UTILS_INSTRUMENTATION_H #define LIBTORRENT_UTILS_INSTRUMENTATION_H +#include "config.h" + #include #include #include diff --git a/test/helpers/test_thread.cc b/test/helpers/test_thread.cc index 82197bdd9..137f24954 100644 --- a/test/helpers/test_thread.cc +++ b/test/helpers/test_thread.cc @@ -20,26 +20,26 @@ const int test_thread::test_flag_pre_poke; const int test_thread::test_flag_post_poke; test_thread::test_thread() : - m_test_state(TEST_NONE), - m_test_flags(0) { + m_testState(TEST_NONE), + m_testFlags(0) { } void test_thread::init_thread() { m_state = STATE_INITIALIZED; - m_test_state = TEST_PRE_START; + m_testState = TEST_PRE_START; m_poll = torrent::PollSelect::create(256); } void test_thread::call_events() { - if ((m_test_flags & test_flag_pre_stop) && m_test_state == TEST_PRE_START && m_state == STATE_ACTIVE) - m_test_state = TEST_PRE_STOP; + if ((m_testFlags & test_flag_pre_stop) && m_testState == TEST_PRE_START && m_state == STATE_ACTIVE) + m_testState = TEST_PRE_STOP; - if ((m_test_flags & test_flag_acquire_global)) { + if ((m_testFlags & test_flag_acquire_global)) { acquire_global_lock(); - m_test_flags &= ~test_flag_acquire_global; - m_test_flags |= test_flag_has_global; + m_testFlags &= ~test_flag_acquire_global; + m_testFlags |= test_flag_has_global; } if ((m_flags & flag_do_shutdown)) { @@ -50,15 +50,15 @@ test_thread::call_events() { throw torrent::shutdown_exception(); } - if ((m_test_flags & test_flag_pre_poke)) { + if ((m_testFlags & test_flag_pre_poke)) { } - if ((m_test_flags & test_flag_do_work)) { + if ((m_testFlags & test_flag_do_work)) { usleep(10 * 1000); // TODO: Don't just sleep, as that give up core. - m_test_flags &= ~test_flag_do_work; + m_testFlags &= ~test_flag_do_work; } - if ((m_test_flags & test_flag_post_poke)) { + if ((m_testFlags & test_flag_post_poke)) { } } diff --git a/test/helpers/test_thread.h b/test/helpers/test_thread.h index 322bb4cc4..10902c8a5 100644 --- a/test/helpers/test_thread.h +++ b/test/helpers/test_thread.h @@ -21,27 +21,27 @@ class test_thread : public torrent::thread_base { test_thread(); - int test_state() const { return m_test_state; } + int test_state() const { return m_testState; } bool is_state(int state) const { return m_state == state; } - bool is_test_state(int state) const { return m_test_state == state; } - bool is_test_flags(int flags) const { return (m_test_flags & flags) == flags; } - bool is_not_test_flags(int flags) const { return !(m_test_flags & flags); } + bool is_test_state(int state) const { return m_testState == state; } + bool is_test_flags(int flags) const { return (m_testFlags & flags) == flags; } + bool is_not_test_flags(int flags) const { return !(m_testFlags & flags); } auto name() const -> const char* { return "test_thread"; } void init_thread(); - void set_pre_stop() { m_test_flags |= test_flag_pre_stop; } - void set_acquire_global() { m_test_flags |= test_flag_acquire_global; } + void set_pre_stop() { m_testFlags |= test_flag_pre_stop; } + void set_acquire_global() { m_testFlags |= test_flag_acquire_global; } - void set_test_flag(int flags) { m_test_flags |= flags; } + void set_test_flag(int flags) { m_testFlags |= flags; } private: void call_events(); - int64_t next_timeout_usec() { return (m_test_flags & test_flag_long_timeout) ? (10000 * 1000) : (100 * 1000); } + int64_t next_timeout_usec() { return (m_testFlags & test_flag_long_timeout) ? (10000 * 1000) : (100 * 1000); } - std::atomic m_test_state lt_cacheline_aligned; - std::atomic m_test_flags lt_cacheline_aligned; + std::atomic m_testState lt_cacheline_aligned; + std::atomic m_testFlags lt_cacheline_aligned; }; struct thread_management_type { diff --git a/test/torrent/task_manager_test.cc b/test/torrent/task_manager_test.cc deleted file mode 100644 index 6bd32be58..000000000 --- a/test/torrent/task_manager_test.cc +++ /dev/null @@ -1,111 +0,0 @@ -#include "config.h" - -#include - -#include "task_manager_test.h" - -CPPUNIT_TEST_SUITE_REGISTRATION(TaskManagerTest); - -struct TMT_Data { - TMT_Data(TaskManagerTest* m) : manager(m), counter(0) {} - - TaskManagerTest *manager; - std::atomic counter; -}; - -void -TaskManagerTest::setUp() { - -} - -void -TaskManagerTest::tearDown() { -} - -void* -TMT_Func(TMT_Data* data) { - data->counter++; - - return NULL; -} - -void* -TMT_lock_func(TMT_Data* data) { - data->counter++; - - data->manager->m_manager.acquire_global_lock(); - usleep(10000); - data->manager->m_manager.release_global_lock(); - - data->counter++; - - return NULL; -} - -void* -TMT_waive_lock_func(TMT_Data* data) { - data->counter++; - - data->manager->m_manager.acquire_global_lock(); - usleep(20000); - data->manager->m_manager.waive_global_lock(); - - data->counter++; - - return NULL; -} - -inline TaskManagerTest::task_pair -TaskManagerTest::create_task(const char* name, torrent::Task::pt_func func) { - TMT_Data* data = new TMT_Data(this); - - return task_pair(m_manager.insert(name, func, data), data); -} - -void -TaskManagerTest::testInsert() { - task_pair task1 = create_task("Test 1", (torrent::Task::pt_func)&TMT_Func); - task_pair task2 = create_task("Test 2", (torrent::Task::pt_func)&TMT_Func); - - usleep(10000); - - CPPUNIT_ASSERT(task1.first != m_manager.end() && task2.first != m_manager.end()); - CPPUNIT_ASSERT(task1.second->counter == 1 && task2.second->counter == 1); - - // Clean up test, check that we've taken down the threads properly. - - // delete data1; - // delete data2; -} - -void -TaskManagerTest::testLock() { - task_pair task1 = create_task("Test Lock 1", (torrent::Task::pt_func)&TMT_lock_func); - task_pair task2 = create_task("Test Lock 2", (torrent::Task::pt_func)&TMT_lock_func); - task_pair task3 = create_task("Test Lock 3", (torrent::Task::pt_func)&TMT_lock_func); - - usleep(100000); - - CPPUNIT_ASSERT(task1.second->counter == 2 && - task2.second->counter == 2 && - task3.second->counter == 2); -} - -void -TaskManagerTest::testWaiveLock() { - task_pair task1 = create_task("Test Waive Lock 1", (torrent::Task::pt_func)&TMT_waive_lock_func); - usleep(5000); - task_pair task2 = create_task("Test Waive Lock 2", (torrent::Task::pt_func)&TMT_lock_func); - task_pair task3 = create_task("Test Waive Lock 3", (torrent::Task::pt_func)&TMT_lock_func); - - usleep(100000); - - CPPUNIT_ASSERT(task1.second->counter == 2 && - task2.second->counter == 2 && - task3.second->counter == 2); - - // The global lock needs to be released as 'Test Lock 1' doesn't - // release it. - m_manager.release_global_lock(); -} - diff --git a/test/torrent/task_manager_test.h b/test/torrent/task_manager_test.h deleted file mode 100644 index 447d4c3b5..000000000 --- a/test/torrent/task_manager_test.h +++ /dev/null @@ -1,28 +0,0 @@ -#include - -#include "torrent/task_manager.h" - -struct TMT_Data; - -class TaskManagerTest : public CppUnit::TestFixture { - CPPUNIT_TEST_SUITE(TaskManagerTest); - CPPUNIT_TEST(testInsert); - CPPUNIT_TEST(testLock); - CPPUNIT_TEST(testWaiveLock); - CPPUNIT_TEST_SUITE_END(); - -public: - typedef std::pair task_pair; - - void setUp(); - void tearDown(); - - void testInsert(); - void testLock(); - void testWaiveLock(); - - inline task_pair - create_task(const char* name, torrent::Task::pt_func func); - - torrent::TaskManager m_manager; -}; From 64bbd83589ad82b0329c335205c54934cb47b863 Mon Sep 17 00:00:00 2001 From: kannibalox Date: Sun, 1 Dec 2024 17:47:39 -0500 Subject: [PATCH 3/3] Remove config.h from headers --- src/torrent/utils/signal_bitfield.h | 2 -- src/torrent/utils/thread_interrupt.h | 2 -- src/utils/instrumentation.h | 2 -- 3 files changed, 6 deletions(-) diff --git a/src/torrent/utils/signal_bitfield.h b/src/torrent/utils/signal_bitfield.h index 94499a582..64640e2b2 100644 --- a/src/torrent/utils/signal_bitfield.h +++ b/src/torrent/utils/signal_bitfield.h @@ -37,8 +37,6 @@ #ifndef LIBTORRENT_UTILS_SIGNAL_BITFIELD_H #define LIBTORRENT_UTILS_SIGNAL_BITFIELD_H -#include "config.h" - #include #include diff --git a/src/torrent/utils/thread_interrupt.h b/src/torrent/utils/thread_interrupt.h index fe148c925..f5ae7e4d9 100644 --- a/src/torrent/utils/thread_interrupt.h +++ b/src/torrent/utils/thread_interrupt.h @@ -37,8 +37,6 @@ #ifndef LIBTORRENT_UTILS_THREAD_INTERRUPT_H #define LIBTORRENT_UTILS_THREAD_INTERRUPT_H -#include "config.h" - #include #include #include diff --git a/src/utils/instrumentation.h b/src/utils/instrumentation.h index e9fb519f3..03f650885 100644 --- a/src/utils/instrumentation.h +++ b/src/utils/instrumentation.h @@ -37,8 +37,6 @@ #ifndef LIBTORRENT_UTILS_INSTRUMENTATION_H #define LIBTORRENT_UTILS_INSTRUMENTATION_H -#include "config.h" - #include #include #include