Axe atomic int (#4326)

* ThreadableJob: Move from AtomicInt to std::atomic

This is the first in a series of commits that migrates away from
AtomicInt towards the C++11 standard library types.
This would allow the removal of AtomicInt and related Qt
version-specific code.

While we're at it, also make ProcessingState an `enum class` so that
it's not implicitly convertible to integers.

* LocklessAllocator: Switch from AtomicInt to std::atomic_int

If it looks like some assignments around the Compare & Swap loops went
missing, it's because compare_exchange_weak() updates the first argument
to the current value when it fails (i.e., returns false).

* BufferManager: Remove extra AtomicInt include

* MixerWorkerThread: AtomicInt to std::atomic_int

* NotePlayHandle: AtomicInt to std::atomic_int

* Remove AtomicInt.h

* Move from QAtomicPointer to std::atomic<T*>

This removes some #ifdef trickery that was being used to get
load-acquire and store-release from Qt5 and "plain" (presumably
sequentially-consistent) loads and stores from Qt4.

include/AtomicInt.h

@@ -1,45 +0,0 @@
-/// \file  AtomicInt.h
-/// \brief Compatibility subclass of QAtomicInt for supporting both Qt4 and Qt5
-
-#ifndef LMMS_ATOMIC_H
-#define LMMS_ATOMIC_H
-
-#include <QtCore/QAtomicInt>
-
-#if QT_VERSION < 0x050300
-
-class AtomicInt : public QAtomicInt
-{
-public:
-	AtomicInt( int value = 0 ) :
-		QAtomicInt( value )
-	{
-	}
-
-	int fetchAndAndOrdered( int valueToAnd )
-	{
-		int value;
-		do
-		{
-			value = (int)*this;
-		}
-		while( !testAndSetOrdered( value, value & valueToAnd ) );
-		return value;
-	}
-
-#if QT_VERSION >= 0x050000 && QT_VERSION < 0x050300
-	operator int() const
-	{
-		return loadAcquire();
-	}
-#endif
-
-};
-
-#else
-
-typedef QAtomicInt AtomicInt;
-
-#endif // QT_VERSION < 0x050300
-
-#endif

include/BufferManager.h

@@ -26,7 +26,6 @@
 #ifndef BUFFER_MANAGER_H
 #define BUFFER_MANAGER_H
 
-#include "AtomicInt.h"
 #include "export.h"
 #include "lmms_basics.h"
 

include/FxMixer.h

@@ -30,6 +30,7 @@
 #include "JournallingObject.h"
 #include "ThreadableJob.h"
 
+#include <atomic>
 
 class FxRoute;
 typedef QVector<FxRoute *> FxRouteVector;
@@ -70,7 +71,7 @@ class FxChannel : public ThreadableJob
 		void unmuteForSolo();
 
 	
-		QAtomicInt m_dependenciesMet;
+		std::atomic_int m_dependenciesMet;
 		void incrementDeps();
 		void processed();
 		

include/InstrumentPlayHandle.h

@@ -59,7 +59,8 @@ public:
 			for( const NotePlayHandle * constNotePlayHandle : nphv )
 			{
 				NotePlayHandle * notePlayHandle = const_cast<NotePlayHandle *>( constNotePlayHandle );
-				if( notePlayHandle->state() != ThreadableJob::Done && ! notePlayHandle->isFinished() )
+				if( notePlayHandle->state() != ThreadableJob::ProcessingState::Done &&
+					!notePlayHandle->isFinished())
 				{
 					nphsLeft = true;
 					notePlayHandle->process();

include/LocklessAllocator.h

@@ -25,10 +25,9 @@
 #ifndef LOCKLESS_ALLOCATOR_H
 #define LOCKLESS_ALLOCATOR_H
 
+#include <atomic>
 #include <stddef.h>
 
-#include "AtomicInt.h"
-
 class LocklessAllocator
 {
 public:
@@ -43,11 +42,11 @@ private:
 	size_t m_capacity;
 	size_t m_elementSize;
 
-	AtomicInt * m_freeState;
+	std::atomic_int * m_freeState;
 	size_t m_freeStateSets;
 
-	AtomicInt m_available;
-	AtomicInt m_startIndex;
+	std::atomic_int m_available;
+	std::atomic_int m_startIndex;
 
 } ;
 

include/LocklessList.h

@@ -25,10 +25,10 @@
 #ifndef LOCKLESS_LIST_H
 #define LOCKLESS_LIST_H
 
-#include <QAtomicPointer>
-
 #include "LocklessAllocator.h"
 
+#include <atomic>
+
 template<typename T>
 class LocklessList
 {
@@ -39,9 +39,10 @@ public:
 		Element * next;
 	} ;
 
-	LocklessList( size_t size )
+	LocklessList( size_t size ) :
+		m_first(nullptr),
+		m_allocator(new LocklessAllocatorT<Element>(size))
 	{
-		m_allocator = new LocklessAllocatorT<Element>( size );
 	}
 
 	~LocklessList()
@@ -53,39 +54,29 @@ public:
 	{
 		Element * e = m_allocator->alloc();
 		e->value = value;
+		e->next = m_first.load(std::memory_order_relaxed);
 
-		do
+		while (!m_first.compare_exchange_weak(e->next, e,
+				std::memory_order_release,
+				std::memory_order_relaxed))
 		{
-#if QT_VERSION >= 0x050000
-			e->next = m_first.loadAcquire();
-#else
-			e->next = m_first;
-#endif
+			// Empty loop (compare_exchange_weak updates e->next)
 		}
-		while( !m_first.testAndSetOrdered( e->next, e ) );
 	}
 
 	Element * popList()
 	{
-		return m_first.fetchAndStoreOrdered( NULL );
+		return m_first.exchange(nullptr);
 	}
 
 	Element * first()
 	{
-#if QT_VERSION >= 0x050000
-		return m_first.loadAcquire();
-#else
-		return m_first;
-#endif
+		return m_first.load(std::memory_order_acquire);
 	}
 
 	void setFirst( Element * e )
 	{
-#if QT_VERSION >= 0x050000
-		m_first.storeRelease( e );
-#else
-		m_first = e;
-#endif
+		m_first.store(e, std::memory_order_release);
 	}
 
 	void free( Element * e )
@@ -95,7 +86,7 @@ public:
 
 
 private:
-	QAtomicPointer<Element> m_first;
+	std::atomic<Element*> m_first;
 	LocklessAllocatorT<Element> * m_allocator;
 
 } ;

include/MixerWorkerThread.h

@@ -25,10 +25,10 @@
 #ifndef MIXER_WORKER_THREAD_H
 #define MIXER_WORKER_THREAD_H
 
-#include <AtomicInt.h>
-#include <QtCore/QAtomicPointer>
 #include <QtCore/QThread>
 
+#include <atomic>
+
 class QWaitCondition;
 class Mixer;
 class ThreadableJob;
@@ -46,12 +46,14 @@ public:
 			Dynamic	// jobs can be added while processing queue
 		} ;
 
+#define JOB_QUEUE_SIZE 1024
 		JobQueue() :
 			m_items(),
 			m_queueSize( 0 ),
 			m_itemsDone( 0 ),
 			m_opMode( Static )
 		{
+			std::fill(m_items, m_items + JOB_QUEUE_SIZE, nullptr);
 		}
 
 		void reset( OperationMode _opMode );
@@ -62,10 +64,9 @@ public:
 		void wait();
 
 	private:
-#define JOB_QUEUE_SIZE 1024
-		QAtomicPointer<ThreadableJob> m_items[JOB_QUEUE_SIZE];
-		AtomicInt m_queueSize;
-		AtomicInt m_itemsDone;
+		std::atomic<ThreadableJob*> m_items[JOB_QUEUE_SIZE];
+		std::atomic_int m_queueSize;
+		std::atomic_int m_itemsDone;
 		OperationMode m_opMode;
 
 	} ;

include/NotePlayHandle.h

@@ -28,7 +28,6 @@
 
 #include <memory>
 
-#include "AtomicInt.h"
 #include "BasicFilters.h"
 #include "Note.h"
 #include "PlayHandle.h"
@@ -350,7 +349,7 @@ public:
 private:
 	static NotePlayHandle ** s_available;
 	static QReadWriteLock s_mutex;
-	static AtomicInt s_availableIndex;
+	static std::atomic_int s_availableIndex;
 	static int s_size;
 };
 

include/ThreadableJob.h

@@ -25,16 +25,15 @@
 #ifndef THREADABLE_JOB_H
 #define THREADABLE_JOB_H
 
-#include "AtomicInt.h"
-
 #include "lmms_basics.h"
 
+#include <atomic>
 
 class ThreadableJob
 {
 public:
 
-	enum ProcessingState
+	enum class ProcessingState : int
 	{
 		Unstarted,
 		Queued,
@@ -43,36 +42,37 @@ public:
 	};
 
 	ThreadableJob() :
-		m_state( ThreadableJob::Unstarted )
+		m_state(ProcessingState::Unstarted)
 	{
 	}
 
 	inline ProcessingState state() const
 	{
-		return static_cast<ProcessingState>( (int) m_state );
+		return m_state.load();
 	}
 
 	inline void reset()
 	{
-		m_state = Unstarted;
+		m_state = ProcessingState::Unstarted;
 	}
 
 	inline void queue()
 	{
-		m_state = Queued;
+		m_state = ProcessingState::Queued;
 	}
 	
 	inline void done()
 	{
-		m_state = Done;
+		m_state = ProcessingState::Done;
 	}
 
 	void process()
 	{
-		if( m_state.testAndSetOrdered( Queued, InProgress ) )
+		auto expected = ProcessingState::Queued;
+		if (m_state.compare_exchange_strong(expected, ProcessingState::InProgress))
 		{
 			doProcessing();
-			m_state = Done;
+			m_state = ProcessingState::Done;
 		}
 	}
 
@@ -82,8 +82,7 @@ public:
 protected:
 	virtual void doProcessing() = 0;
 
-	AtomicInt m_state;
-
+	std::atomic<ProcessingState> m_state;
 } ;
 
 #endif

src/core/FxMixer.cpp

@@ -71,7 +71,7 @@ FxChannel::FxChannel( int idx, Model * _parent ) :
 	m_lock(),
 	m_channelIndex( idx ),
 	m_queued( false ),
-	m_dependenciesMet( 0 )
+	m_dependenciesMet(0)
 {
 	BufferManager::clear( m_buffer, Engine::mixer()->framesPerPeriod() );
 }
@@ -98,7 +98,7 @@ inline void FxChannel::processed()
 
 void FxChannel::incrementDeps()
 {
-	int i = m_dependenciesMet.fetchAndAddOrdered( 1 ) + 1;
+	int i = m_dependenciesMet++ + 1;
 	if( i >= m_receives.size() && ! m_queued )
 	{
 		m_queued = true;
@@ -594,14 +594,14 @@ void FxMixer::masterMix( sampleFrame * _buf )
 			MixerWorkerThread::addJob( ch );
 		}
 	}
-	while( m_fxChannels[0]->state() != ThreadableJob::Done )
+	while (m_fxChannels[0]->state() != ThreadableJob::ProcessingState::Done)
 	{
 		bool found = false;
 		for( FxChannel * ch : m_fxChannels )
 		{
-			int s = ch->state();
-			if( s == ThreadableJob::Queued
-				|| s == ThreadableJob::InProgress )
+			const auto s = ch->state();
+			if (s == ThreadableJob::ProcessingState::Queued
+				|| s == ThreadableJob::ProcessingState::InProgress)
 			{
 				found = true;
 				break;

src/core/LocklessAllocator.cpp

@@ -24,6 +24,7 @@
 
 #include "LocklessAllocator.h"
 
+#include <algorithm>
 #include <stdio.h>
 
 #include "lmmsconfig.h"
@@ -55,9 +56,11 @@ LocklessAllocator::LocklessAllocator( size_t nmemb, size_t size )
 	m_pool = new char[m_capacity * m_elementSize];
 
 	m_freeStateSets = m_capacity / SIZEOF_SET;
-	m_freeState = new AtomicInt[m_freeStateSets];
+	m_freeState = new std::atomic_int[m_freeStateSets];
+	std::fill(m_freeState, m_freeState + m_freeStateSets, 0);
 
 	m_available = m_capacity;
+	m_startIndex = 0;
 }
 
 
@@ -101,27 +104,27 @@ static int ffs( int i )
 
 void * LocklessAllocator::alloc()
 {
-	int available;
+	// Some of these CAS loops could probably use relaxed atomics, as discussed
+	// in http://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange.
+	// Let's use sequentially-consistent ops to be safe for now.
+	int available = m_available.load();
 	do
 	{
-		available = m_available;
 		if( !available )
 		{
 			fprintf( stderr, "LocklessAllocator: No free space\n" );
 			return NULL;
 		}
 	}
-	while( !m_available.testAndSetOrdered( available, available - 1 ) );
+	while (!m_available.compare_exchange_weak(available, available - 1));
 
-	size_t startIndex = m_startIndex.fetchAndAddOrdered( 1 )
-							% m_freeStateSets;
-	for( size_t set = startIndex;; set = ( set + 1 ) % m_freeStateSets )
+	const size_t startIndex = m_startIndex++ % m_freeStateSets;
+	for (size_t set = startIndex;; set = ( set + 1 ) % m_freeStateSets)
 	{
-		for( int freeState = m_freeState[set]; freeState != -1;
-						freeState = m_freeState[set] )
+		for (int freeState = m_freeState[set]; freeState != -1;)
 		{
 			int bit = ffs( ~freeState ) - 1;
-			if( m_freeState[set].testAndSetOrdered( freeState,
+			if (m_freeState[set].compare_exchange_weak(freeState,
 							freeState | 1 << bit ) )
 			{
 				return m_pool + ( SIZEOF_SET * set + bit )
@@ -151,11 +154,11 @@ invalid:
 	size_t set = offset / SIZEOF_SET;
 	int bit = offset % SIZEOF_SET;
 	int mask = 1 << bit;
-	int prevState = m_freeState[set].fetchAndAndOrdered( ~mask );
+	int prevState = m_freeState[set].fetch_and(~mask);
 	if ( !( prevState & mask ) )
 	{
 		fprintf( stderr, "LocklessAllocator: Block not in use\n" );
 		return;
 	}
-	m_available.fetchAndAddOrdered( 1 );
+	++m_available;
 }

src/core/MixerWorkerThread.cpp

@@ -52,7 +52,7 @@ void MixerWorkerThread::JobQueue::addJob( ThreadableJob * _job )
 		// update job state
 		_job->queue();
 		// actually queue the job via atomic operations
-		m_items[m_queueSize.fetchAndAddOrdered(1)] = _job;
+		m_items[m_queueSize++] = _job;
 	}
 }
 
@@ -61,17 +61,17 @@ void MixerWorkerThread::JobQueue::addJob( ThreadableJob * _job )
 void MixerWorkerThread::JobQueue::run()
 {
 	bool processedJob = true;
-	while( processedJob && (int) m_itemsDone < (int) m_queueSize )
+	while (processedJob && m_itemsDone < m_queueSize)
 	{
 		processedJob = false;
 		for( int i = 0; i < m_queueSize; ++i )
 		{
-			ThreadableJob * job = m_items[i].fetchAndStoreOrdered( NULL );
+			ThreadableJob * job = m_items[i].exchange(nullptr);
 			if( job )
 			{
 				job->process();
 				processedJob = true;
-				m_itemsDone.fetchAndAddOrdered( 1 );
+				++m_itemsDone;
 			}
 		}
 		// always exit loop if we're not in dynamic mode
@@ -84,7 +84,7 @@ void MixerWorkerThread::JobQueue::run()
 
 void MixerWorkerThread::JobQueue::wait()
 {
-	while( (int) m_itemsDone < (int) m_queueSize )
+	while (m_itemsDone < m_queueSize)
 	{
 #if defined(LMMS_HOST_X86) || defined(LMMS_HOST_X86_64)
 		_mm_pause();

src/core/NotePlayHandle.cpp

@@ -551,7 +551,7 @@ void NotePlayHandle::resize( const bpm_t _new_tempo )
 
 NotePlayHandle ** NotePlayHandleManager::s_available;
 QReadWriteLock NotePlayHandleManager::s_mutex;
-AtomicInt NotePlayHandleManager::s_availableIndex;
+std::atomic_int NotePlayHandleManager::s_availableIndex;
 int NotePlayHandleManager::s_size;
 
 
@@ -586,7 +586,7 @@ NotePlayHandle * NotePlayHandleManager::acquire( InstrumentTrack* instrumentTrac
 		s_mutex.unlock();
 	}
 	s_mutex.lockForRead();
-	NotePlayHandle * nph = s_available[ s_availableIndex.fetchAndAddOrdered( -1 ) ];
+	NotePlayHandle * nph = s_available[s_availableIndex--];
 	s_mutex.unlock();
 
 	new( (void*)nph ) NotePlayHandle( instrumentTrack, offset, frames, noteToPlay, parent, midiEventChannel, origin );
@@ -598,7 +598,7 @@ void NotePlayHandleManager::release( NotePlayHandle * nph )
 {
 	nph->NotePlayHandle::~NotePlayHandle();
 	s_mutex.lockForRead();
-	s_available[ s_availableIndex.fetchAndAddOrdered( 1 ) + 1 ] = nph;
+	s_available[++s_availableIndex] = nph;
 	s_mutex.unlock();
 }
 
@@ -614,7 +614,7 @@ void NotePlayHandleManager::extend( int c )
 
 	for( int i=0; i < c; ++i )
 	{
-		s_available[ s_availableIndex.fetchAndAddOrdered( 1 ) + 1 ] = n;
+		s_available[++s_availableIndex] = n;
 		++n;
 	}
 }

src/core/PresetPreviewPlayHandle.cpp

@@ -22,7 +22,6 @@
  *
  */
 
-#include <QAtomicPointer>
 #include <QFileInfo>
 
 #include "PresetPreviewPlayHandle.h"
@@ -34,7 +33,7 @@
 #include "ProjectJournal.h"
 #include "TrackContainer.h"
 
-
+#include <atomic>
 
 // invisible track-container which is needed as parent for preview-channels
 class PreviewTrackContainer : public TrackContainer
@@ -67,25 +66,17 @@ public:
 
 	NotePlayHandle* previewNote()
 	{
-	#if QT_VERSION >= 0x050000
-		return m_previewNote.loadAcquire();
-	#else
-		return m_previewNote;
-	#endif
+		return m_previewNote.load(std::memory_order_acquire);
 	}
 
 	void setPreviewNote( NotePlayHandle * _note )
 	{
-	#if QT_VERSION >= 0x050000
-		m_previewNote.storeRelease( _note );
-	#else
-		m_previewNote = _note;
-	#endif
+		m_previewNote.store(_note, std::memory_order_release);
 	}
 
 	bool testAndSetPreviewNote( NotePlayHandle * expectedVal, NotePlayHandle * newVal )
 	{
-		return m_previewNote.testAndSetOrdered( expectedVal, newVal );
+		return m_previewNote.compare_exchange_strong(expectedVal, newVal);
 	}
 
 	void lockData()
@@ -111,7 +102,7 @@ public:
 
 private:
 	InstrumentTrack* m_previewInstrumentTrack;
-	QAtomicPointer<NotePlayHandle> m_previewNote;
+	std::atomic<NotePlayHandle*> m_previewNote;
 	QMutex m_dataMutex;
 
 	friend class PresetPreviewPlayHandle;
@@ -125,7 +116,7 @@ PreviewTrackContainer * PresetPreviewPlayHandle::s_previewTC;
 
 PresetPreviewPlayHandle::PresetPreviewPlayHandle( const QString & _preset_file, bool _load_by_plugin, DataFile *dataFile ) :
 	PlayHandle( TypePresetPreviewHandle ),
-	m_previewNote( NULL )
+	m_previewNote(nullptr)
 {
 	setUsesBuffer( false );