diff --git a/build/aclocal/atomic_builtins.m4 b/build/aclocal/atomic_builtins.m4 index eac5d3336a..6ebc2279af 100644 --- a/build/aclocal/atomic_builtins.m4 +++ b/build/aclocal/atomic_builtins.m4 @@ -6,16 +6,13 @@ AC_DEFUN([WX_ATOMIC_BUILTINS], [ AC_REQUIRE([AC_PROG_CC]) if test -n "$GCC"; then - AC_MSG_CHECKING([for __sync_fetch_and_add and __sync_sub_and_fetch builtins]) + AC_MSG_CHECKING([for __sync_xxx_and_fetch builtins]) AC_CACHE_VAL(wx_cv_cc_gcc_atomic_builtins, [ AC_TRY_LINK( [], [ unsigned int value=0; - /* wxAtomicInc doesn't use return value here */ - __sync_fetch_and_add(&value, 2); - __sync_sub_and_fetch(&value, 1); - /* but wxAtomicDec does, so mimic that: */ + volatile unsigned int r1 = __sync_add_and_fetch(&value, 2); volatile unsigned int r2 = __sync_sub_and_fetch(&value, 1); ], wx_cv_cc_gcc_atomic_builtins=yes, diff --git a/configure b/configure index 9279c2d587..4e807f144a 100755 --- a/configure +++ b/configure @@ -25176,8 +25176,8 @@ fi if test -n "$GCC"; then - { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __sync_fetch_and_add and __sync_sub_and_fetch builtins" >&5 -$as_echo_n "checking for __sync_fetch_and_add and __sync_sub_and_fetch builtins... " >&6; } + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for __sync_xxx_and_fetch builtins" >&5 +$as_echo_n "checking for __sync_xxx_and_fetch builtins... " >&6; } if ${wx_cv_cc_gcc_atomic_builtins+:} false; then : $as_echo_n "(cached) " >&6 else @@ -25190,10 +25190,7 @@ main () { unsigned int value=0; - /* wxAtomicInc doesn't use return value here */ - __sync_fetch_and_add(&value, 2); - __sync_sub_and_fetch(&value, 1); - /* but wxAtomicDec does, so mimic that: */ + volatile unsigned int r1 = __sync_add_and_fetch(&value, 2); volatile unsigned int r2 = __sync_sub_and_fetch(&value, 1); ; diff --git a/include/wx/atomic.h b/include/wx/atomic.h index 25bd049834..5112623515 100644 --- a/include/wx/atomic.h +++ b/include/wx/atomic.h @@ -17,10 +17,7 @@ // get the value of wxUSE_THREADS configuration flag #include "wx/defs.h" -// constraints on the various functions: -// - wxAtomicDec must return a zero value if the value is zero once -// decremented else it must return any non-zero value (the true value is OK -// but not necessary). +// these functions return the new value, after the operation #if wxUSE_THREADS @@ -31,9 +28,9 @@ // http://bugs.mysql.com/bug.php?id=28456 // http://golubenco.org/blog/atomic-operations/ -inline void wxAtomicInc (wxUint32 &value) +inline wxUint32 wxAtomicInc (wxUint32 &value) { - __sync_fetch_and_add(&value, 1); + return __sync_add_and_fetch(&value, 1); } inline wxUint32 wxAtomicDec (wxUint32 &value) @@ -47,9 +44,9 @@ inline wxUint32 wxAtomicDec (wxUint32 &value) // include standard Windows headers #include "wx/msw/wrapwin.h" -inline void wxAtomicInc (wxUint32 &value) +inline wxUint32 wxAtomicInc (wxUint32 &value) { - InterlockedIncrement ((LONG*)&value); + return InterlockedIncrement ((LONG*)&value); } inline wxUint32 wxAtomicDec (wxUint32 &value) @@ -60,9 +57,9 @@ inline wxUint32 wxAtomicDec (wxUint32 &value) #elif defined(__DARWIN__) #include "libkern/OSAtomic.h" -inline void wxAtomicInc (wxUint32 &value) +inline wxUint32 wxAtomicInc (wxUint32 &value) { - OSAtomicIncrement32 ((int32_t*)&value); + return OSAtomicIncrement32 ((int32_t*)&value); } inline wxUint32 wxAtomicDec (wxUint32 &value) @@ -76,7 +73,7 @@ inline wxUint32 wxAtomicDec (wxUint32 &value) inline void wxAtomicInc (wxUint32 &value) { - atomic_add_32 ((uint32_t*)&value, 1); + return atomic_add_32_nv ((uint32_t*)&value, 1); } inline wxUint32 wxAtomicDec (wxUint32 &value) @@ -94,7 +91,7 @@ inline wxUint32 wxAtomicDec (wxUint32 &value) #else // else of wxUSE_THREADS // if no threads are used we can safely use simple ++/-- -inline void wxAtomicInc (wxUint32 &value) { ++value; } +inline wxUint32 wxAtomicInc (wxUint32 &value) { return ++value; } inline wxUint32 wxAtomicDec (wxUint32 &value) { return --value; } #endif // !wxUSE_THREADS @@ -120,10 +117,10 @@ public: wxAtomicInt32& operator=(wxInt32 v) { m_value = v; return *this; } - void Inc() + wxInt32 Inc() { wxCriticalSectionLocker lock(m_locker); - ++m_value; + return ++m_value; } wxInt32 Dec() @@ -137,14 +134,14 @@ private: wxCriticalSection m_locker; }; -inline void wxAtomicInc(wxAtomicInt32 &value) { value.Inc(); } +inline wxInt32 wxAtomicInc(wxAtomicInt32 &value) { return value.Inc(); } inline wxInt32 wxAtomicDec(wxAtomicInt32 &value) { return value.Dec(); } #else // !wxNEEDS_GENERIC_ATOMIC_OPS #define wxHAS_ATOMIC_OPS -inline void wxAtomicInc(wxInt32 &value) { wxAtomicInc((wxUint32&)value); } +inline wxInt32 wxAtomicInc(wxInt32 &value) { return wxAtomicInc((wxUint32&)value); } inline wxInt32 wxAtomicDec(wxInt32 &value) { return wxAtomicDec((wxUint32&)value); } typedef wxInt32 wxAtomicInt32; diff --git a/interface/wx/atomic.h b/interface/wx/atomic.h index 441e6eb610..bf0b22d255 100644 --- a/interface/wx/atomic.h +++ b/interface/wx/atomic.h @@ -16,21 +16,27 @@ /** This function increments @a value in an atomic manner. + @note It is recommended to use @c std::atomic available in C++11 and later + instead of this function in any new code. + Whenever possible wxWidgets provides an efficient, CPU-specific, implementation of this function. If such implementation is available, the symbol wxHAS_ATOMIC_OPS is defined. Otherwise this function still exists but is implemented in a generic way using a critical section which can be prohibitively expensive for use in performance-sensitive code. + Returns the new value after the increment (the return value is only + available since wxWidgets 3.1.7, this function doesn't return anything in + previous versions of the library). + @header{wx/atomic.h} */ -void wxAtomicInc(wxAtomicInt& value); +wxInt32 wxAtomicInc(wxAtomicInt& value); /** This function decrements value in an atomic manner. - Returns 0 if value is 0 after decrement or any non-zero value (not - necessarily equal to the value of the variable) otherwise. + Returns the new value after decrementing it. @see wxAtomicInc diff --git a/src/common/init.cpp b/src/common/init.cpp index 6b2cc19710..e6d33d088d 100644 --- a/src/common/init.cpp +++ b/src/common/init.cpp @@ -28,7 +28,7 @@ #endif #include "wx/init.h" -#include "wx/thread.h" +#include "wx/atomic.h" #include "wx/scopedptr.h" #include "wx/except.h" @@ -130,21 +130,20 @@ static inline void Use(void *) { } static struct InitData { InitData() + : nInitCount(0) { - nInitCount = 0; - #if wxUSE_UNICODE argc = argcOrig = 0; // argv = argvOrig = NULL; -- not even really needed #endif // wxUSE_UNICODE } - // critical section protecting this struct - wxCRIT_SECT_DECLARE_MEMBER(csInit); - // number of times wxInitialize() was called minus the number of times // wxUninitialize() was - size_t nInitCount; + // + // it is atomic to allow more than one thread to call wxInitialize() but + // only one of them to actually initialize the library + wxAtomicInt nInitCount; #if wxUSE_UNICODE int argc; @@ -530,9 +529,7 @@ bool wxInitialize() bool wxInitialize(int& argc, wxChar **argv) { - wxCRIT_SECT_LOCKER(lockInit, gs_initData.csInit); - - if ( gs_initData.nInitCount++ ) + if ( wxAtomicInc(gs_initData.nInitCount) != 1 ) { // already initialized return true; @@ -544,9 +541,7 @@ bool wxInitialize(int& argc, wxChar **argv) #if wxUSE_UNICODE bool wxInitialize(int& argc, char **argv) { - wxCRIT_SECT_LOCKER(lockInit, gs_initData.csInit); - - if ( gs_initData.nInitCount++ ) + if ( wxAtomicInc(gs_initData.nInitCount) != 1 ) { // already initialized return true; @@ -558,10 +553,8 @@ bool wxInitialize(int& argc, char **argv) void wxUninitialize() { - wxCRIT_SECT_LOCKER(lockInit, gs_initData.csInit); + if ( wxAtomicDec(gs_initData.nInitCount) != 0 ) + return; - if ( --gs_initData.nInitCount == 0 ) - { - wxEntryCleanup(); - } + wxEntryCleanup(); } diff --git a/tests/thread/atomic.cpp b/tests/thread/atomic.cpp index 2d8ce31a97..81c0672d7d 100644 --- a/tests/thread/atomic.cpp +++ b/tests/thread/atomic.cpp @@ -20,30 +20,22 @@ #include "wx/thread.h" #include "wx/dynarray.h" #include "wx/log.h" +#include "wx/vector.h" -WX_DEFINE_ARRAY_PTR(wxThread *, wxArrayThread); +typedef wxVector wxArrayThread; // ---------------------------------------------------------------------------- // constants // ---------------------------------------------------------------------------- -// number of times to run the loops: the code takes too long to run if we use -// the bigger value with generic atomic operations implementation -#ifdef wxHAS_ATOMIC_OPS - static const wxInt32 ITERATIONS_NUM = 10000000; -#else - static const wxInt32 ITERATIONS_NUM = 1000; -#endif +static const wxInt32 ITERATIONS_NUM = 10000; // ---------------------------------------------------------------------------- -// test class +// test helper thread // ---------------------------------------------------------------------------- -class AtomicTestCase : public CppUnit::TestCase +namespace { -public: - AtomicTestCase() { } - enum ETestType { IncAndDecMixed, @@ -51,7 +43,6 @@ public: DecOnly }; -private: class MyThread : public wxThread { public: @@ -65,34 +56,13 @@ private: wxAtomicInt &m_operateOn; ETestType m_testType; }; +} // anonymous namespace - CPPUNIT_TEST_SUITE( AtomicTestCase ); - CPPUNIT_TEST( TestNoThread ); - CPPUNIT_TEST( TestDecReturn ); - CPPUNIT_TEST( TestTwoThreadsMix ); - CPPUNIT_TEST( TestTenThreadsMix ); - CPPUNIT_TEST( TestTwoThreadsSeparate ); - CPPUNIT_TEST( TestTenThreadsSeparate ); - CPPUNIT_TEST_SUITE_END(); +// ---------------------------------------------------------------------------- +// the tests themselves +// ---------------------------------------------------------------------------- - void TestNoThread(); - void TestDecReturn(); - void TestTenThreadsMix() { TestWithThreads(10, IncAndDecMixed); } - void TestTwoThreadsMix() { TestWithThreads(2, IncAndDecMixed); } - void TestTenThreadsSeparate() { TestWithThreads(10, IncOnly); } - void TestTwoThreadsSeparate() { TestWithThreads(2, IncOnly); } - void TestWithThreads(int count, ETestType testtype); - - wxDECLARE_NO_COPY_CLASS(AtomicTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( AtomicTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( AtomicTestCase, "AtomicTestCase" ); - -void AtomicTestCase::TestNoThread() +TEST_CASE("Atomic::NoThread", "[atomic]") { wxAtomicInt int1 = 0, int2 = 0; @@ -103,23 +73,30 @@ void AtomicTestCase::TestNoThread() wxAtomicDec(int2); } - CPPUNIT_ASSERT( int1 == ITERATIONS_NUM ); - CPPUNIT_ASSERT( int2 == -ITERATIONS_NUM ); + CHECK( int1 == ITERATIONS_NUM ); + CHECK( int2 == -ITERATIONS_NUM ); } -void AtomicTestCase::TestDecReturn() +TEST_CASE("Atomic::ReturnValue", "[atomic]") { wxAtomicInt i(0); - wxAtomicInc(i); - wxAtomicInc(i); - CPPUNIT_ASSERT( i == 2 ); + REQUIRE( wxAtomicInc(i) == 1 ); + REQUIRE( wxAtomicInc(i) == 2 ); - CPPUNIT_ASSERT( wxAtomicDec(i) > 0 ); - CPPUNIT_ASSERT( wxAtomicDec(i) == 0 ); + REQUIRE( wxAtomicDec(i) == 1 ); + REQUIRE( wxAtomicDec(i) == 0 ); } -void AtomicTestCase::TestWithThreads(int count, ETestType testType) +TEST_CASE("Atomic::WithThreads", "[atomic]") { + int count; + ETestType testType; + + SECTION( "2 threads using inc and dec") { count = 2; testType = IncAndDecMixed; } + SECTION("10 threads using inc and dec") { count = 10; testType = IncAndDecMixed; } + SECTION( "2 threads using inc or dec" ) { count = 2; testType = IncOnly; } + SECTION("10 threads using inc or dec" ) { count = 10; testType = IncOnly; } + wxAtomicInt int1=0; wxArrayThread threads; @@ -146,7 +123,7 @@ void AtomicTestCase::TestWithThreads(int count, ETestType testType) delete thread; } else - threads.Add(thread); + threads.push_back(thread); } for ( i = 0; i < count; ++i ) @@ -158,16 +135,16 @@ void AtomicTestCase::TestWithThreads(int count, ETestType testType) for ( i = 0; i < count; ++i ) { // each thread should return 0, else it detected some problem - CPPUNIT_ASSERT (threads[i]->Wait() == (wxThread::ExitCode)0); + CHECK (threads[i]->Wait() == (wxThread::ExitCode)0); delete threads[i]; } - CPPUNIT_ASSERT( int1 == 0 ); + CHECK( int1 == 0 ); } // ---------------------------------------------------------------------------- -void *AtomicTestCase::MyThread::Entry() +void *MyThread::Entry() { wxInt32 negativeValuesSeen = 0; @@ -175,19 +152,17 @@ void *AtomicTestCase::MyThread::Entry() { switch ( m_testType ) { - case AtomicTestCase::IncAndDecMixed: + case IncAndDecMixed: wxAtomicInc(m_operateOn); - wxAtomicDec(m_operateOn); - - if (m_operateOn < 0) + if ( wxAtomicDec(m_operateOn) < 0 ) ++negativeValuesSeen; break; - case AtomicTestCase::IncOnly: + case IncOnly: wxAtomicInc(m_operateOn); break; - case AtomicTestCase::DecOnly: + case DecOnly: wxAtomicDec(m_operateOn); break; }