Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 86 additions & 0 deletions include/LocklessList.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
* LocklessList.h - list with lockless push and pop
*
* Copyright (c) 2016 Javier Serrano Polo <[email protected]>
*
* This file is part of LMMS - http://lmms.io
*
* 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 (see COPYING); if not, write to the
* Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301 USA.
*
*/

#ifndef LOCKLESS_LIST_H
#define LOCKLESS_LIST_H

#include <QAtomicPointer>

template<typename T>
class LocklessList
{
public:
struct Element
{
T value;
Element * next;
} ;

void push( T value )
{
Element * e = new Element;
e->value = value;

do
{
#if QT_VERSION >= 0x050000
e->next = m_first.loadAcquire();
#else
e->next = m_first;
#endif
}
while( !m_first.testAndSetOrdered( e->next, e ) );

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a busy-wait... is it likely to cause a performance problem, or can you use some kind of blocking thing here?

Copy link

@DestyNova DestyNova Jul 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, upon reading a little bit about lockless lists, it seems that the while condition is much more likely to succeed than fail (because, of course, no lock is being held). So this should be fine. 👍

}

Element * popList()
{
return m_first.fetchAndStoreOrdered( NULL );
}

Element * first()
{
#if QT_VERSION >= 0x050000
return m_first.loadAcquire();
#else
return m_first;
#endif
}

void setFirst( Element * e )
{
#if QT_VERSION >= 0x050000
m_first.storeRelease( e );
#else
m_first = e;
#endif
}


private:
QAtomicPointer<Element> m_first;

} ;


#endif
4 changes: 3 additions & 1 deletion include/Mixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@


#include "lmms_basics.h"
#include "LocklessList.h"
#include "Note.h"
#include "fifo_buffer.h"
#include "MixerProfiler.h"
Expand Down Expand Up @@ -379,7 +380,8 @@ class EXPORT Mixer : public QObject

// playhandle stuff
PlayHandleList m_playHandles;
PlayHandleList m_newPlayHandles; // place where new playhandles are added temporarily
// place where new playhandles are added temporarily
LocklessList<PlayHandle *> m_newPlayHandles;
ConstPlayHandleList m_playHandlesToRemove;


Expand Down
42 changes: 29 additions & 13 deletions src/core/Mixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@



typedef LocklessList<PlayHandle *>::Element LocklessListElement;


static __thread bool s_renderingThread;


Expand Down Expand Up @@ -412,8 +415,13 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
song->processNextBuffer();

// add all play-handles that have to be added
m_playHandles += m_newPlayHandles;
m_newPlayHandles.clear();
for( LocklessListElement * e = m_newPlayHandles.popList(); e; )
{
m_playHandles += e->value;
LocklessListElement * next = e->next;
delete e;
e = next;
}

// STAGE 1: run and render all play handles
MixerWorkerThread::fillJobQueue<PlayHandleList>( m_playHandles );
Expand Down Expand Up @@ -635,10 +643,8 @@ bool Mixer::addPlayHandle( PlayHandle* handle )
{
if( criticalXRuns() == false )
{
requestChangeInModel();
m_newPlayHandles.append( handle );
handle->audioPort()->addPlayHandle( handle );
doneChangeInModel();
m_newPlayHandles.push( handle );
handle->audioPort()->addPlayHandle( handle );
return true;
}

Expand All @@ -664,16 +670,26 @@ void Mixer::removePlayHandle( PlayHandle * _ph )
bool removedFromList = false;
// Check m_newPlayHandles first because doing it the other way around
// creates a race condition
PlayHandleList::Iterator it =
qFind( m_newPlayHandles.begin(),
m_newPlayHandles.end(), _ph );
if( it != m_newPlayHandles.end() )
for( LocklessListElement * e = m_newPlayHandles.first(),
* ePrev = NULL; e; ePrev = e, e = e->next )
{
m_newPlayHandles.erase( it );
removedFromList = true;
if( e->value == _ph )
{
if( ePrev )
{
ePrev->next = e->next;
}
else
{
m_newPlayHandles.setFirst( e->next );
}
delete e;
removedFromList = true;
break;
}
}
// Now check m_playHandles
it = qFind( m_playHandles.begin(),
PlayHandleList::Iterator it = qFind( m_playHandles.begin(),
m_playHandles.end(), _ph );
if( it != m_playHandles.end() )
{
Expand Down