From 1a9abed1dcdf38d2a97af9f362bd828d65950d1d Mon Sep 17 00:00:00 2001 From: TTTTTAAAAAKKKKEEEENNNN <48080812+TTTTTAAAAAKKKKEEEENNNN@users.noreply.github.com> Date: Wed, 17 Mar 2021 22:13:22 +0800 Subject: [PATCH 1/3] bugfix #57 use built-in atomic operation instead of the inline-asm one --- include/thread/AtomicInt.h | 7 +--- test/testcase/testThreadSmoke.cpp | 68 ++++++++++++++++++++++++++----- 2 files changed, 60 insertions(+), 15 deletions(-) diff --git a/include/thread/AtomicInt.h b/include/thread/AtomicInt.h index c7d145a..bff7365 100644 --- a/include/thread/AtomicInt.h +++ b/include/thread/AtomicInt.h @@ -12,11 +12,8 @@ class AtomicInt { AtomicInt(int initval = 0) : _intval(initval) {}; int inc(int incval = 1) { - int __i = incval; - asm volatile(LOCK_PREFIX "xaddl %0, %1" - : "+r" (incval), "+m" (_intval) - : : "memory"); - return incval + __i; + int oldValue = __sync_fetch_and_add(&_intval, incval); + return incval + oldValue; }; int dec(int decval = 1) { diff --git a/test/testcase/testThreadSmoke.cpp b/test/testcase/testThreadSmoke.cpp index eeac3b1..0d52aac 100644 --- a/test/testcase/testThreadSmoke.cpp +++ b/test/testcase/testThreadSmoke.cpp @@ -5,21 +5,13 @@ #include "src/thread/Thread.h" #include "src/thread/ThreadPool.h" #include "NacosString.h" +#include "thread/AtomicInt.h" using namespace std; using namespace nacos; -class SmokingTestThreadTask : public Task { -public: - SmokingTestThreadTask(const NacosString &taskName) { setTaskName(taskName); }; - - void run() { - NacosString taskName = getTaskName(); - log_info("Hello world from %s\n", taskName.c_str()); - }; -}; - class ThreadPoolLongRunTask : public Task { + public: ThreadPoolLongRunTask(int taskId) { NacosString taskName = "ThreadPoolLongRunTask" + NacosStringOps::valueOf(taskId); @@ -86,5 +78,61 @@ bool testThreadPoolSmoke() { } cout << "If no coredump, should be successful" << endl; tpStopper.join(); + return true; } + +AtomicInt totalFinishedThreads; + +class SmokingTestThreadTask : public Task { +private: + AtomicInt &_counter; +public: + SmokingTestThreadTask(const NacosString &taskName, AtomicInt &counter) : _counter(counter) { + setTaskName(taskName); + }; + + void run() { + NacosString taskName = getTaskName(); + int currentTotal = 0; + for (int i = 0; i < 100000; i++) { + currentTotal = _counter.inc(); + } + log_info("Hello world from %s, totalCount = %d\n", taskName.c_str(), currentTotal); + totalFinishedThreads.inc(); + }; +}; + +bool testThreadPoolConcurrentWithAtomicCounter() { + cout << "Starting testThreadPoolConcurrentWithAtomicCounter..." << endl; + ThreadPool tp(10); + tp.start(); + cout << "ok, size = 10" << endl; + AtomicInt totalCounter; + + Task *tasks[1000]; + for (size_t i = 0; i < 40; i++) { + cout << "Creating task " << i << "..." << endl; + tasks[i] = new SmokingTestThreadTask("Counter-" + NacosStringOps::valueOf(i), totalCounter); + tp.put(tasks[i]); + cout << "ok" << endl; + } + + + while (totalFinishedThreads.get() != 40) { + sleep(1); + } + tp.stop(); + cout << "If no coredump, should be successful" << endl; + cout << "totalCounter.get() = " << totalCounter.get() << endl; + + SHOULD_BE_TRUE(totalCounter.get() == 40 * 100000, "40 threads, each thread accumulates totalCounter by 100k, result should be 4000k"); + + for (size_t i = 0; i < 40; i++) { + cout << "destroying task " << i << "..." << endl; + delete tasks[i]; + tasks[i] = NULL; + cout << "ok" << endl; + } + return true; +} \ No newline at end of file From af36a8442922077a013b53f26f51d80316653886 Mon Sep 17 00:00:00 2001 From: TTTTTAAAAAKKKKEEEENNNN <48080812+TTTTTAAAAAKKKKEEEENNNN@users.noreply.github.com> Date: Wed, 17 Mar 2021 22:14:10 +0800 Subject: [PATCH 2/3] #57 --- include/thread/AtomicInt.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/thread/AtomicInt.h b/include/thread/AtomicInt.h index bff7365..dc50576 100644 --- a/include/thread/AtomicInt.h +++ b/include/thread/AtomicInt.h @@ -1,9 +1,6 @@ #ifndef __ATOMIC_INT_H_ #define __ATOMIC_INT_H_ -#define __atomic_fool_gcc(x) (*(volatile struct { int a[100]; } *)x) -#define LOCK_PREFIX "lock " - namespace nacos{ class AtomicInt { private: From 88a23391ebfa7c80ffea116c5447dd2e5a877e9b Mon Sep 17 00:00:00 2001 From: TTTTTAAAAAKKKKEEEENNNN <48080812+TTTTTAAAAAKKKKEEEENNNN@users.noreply.github.com> Date: Wed, 17 Mar 2021 22:25:02 +0800 Subject: [PATCH 3/3] add testcase for gcc built-in atomic operation to fix #57 --- test/allinone.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/allinone.cpp b/test/allinone.cpp index f6bf425..be76a67 100644 --- a/test/allinone.cpp +++ b/test/allinone.cpp @@ -128,6 +128,8 @@ bool testDelayedThread2(); bool testNamingServiceAndDeRegisterActively(); +bool testThreadPoolConcurrentWithAtomicCounter(); + TestData disabledTestList[] = TEST_ITEM_START TEST_ITEM_END @@ -197,6 +199,7 @@ TEST_ITEM_START TEST_ITEM("Test delayed task pool", testDelayedThread) TEST_ITEM("Test delayed task pool - multiple tasks triggered at the same time", testDelayedThread2) TEST_ITEM("Register a service instance and remove it actively", testNamingServiceAndDeRegisterActively) + TEST_ITEM("thread pool with concurrent add & atomic operation", testThreadPoolConcurrentWithAtomicCounter) TEST_ITEM_END int main() {