Skip to content

Commit

Permalink
Reimplement BufferManager
Browse files Browse the repository at this point in the history
Re-implement BufferManager using a single lock-free stack instead of two
fixed-size arrays. This has the following advantages:
 * Less code
 * Immediate re-use of released buffers, no blocking "refresh" required
 * Dynamic size, no crash when out of buffers (fixes #3670)
  • Loading branch information
lukas-w committed Aug 29, 2017
1 parent efd0d34 commit b6421d4
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 79 deletions.
14 changes: 0 additions & 14 deletions include/BufferManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@
#include "export.h"
#include "lmms_basics.h"

const int BM_INITIAL_BUFFERS = 512;
//const int BM_INCREMENT = 64;

class EXPORT BufferManager
{
public:
Expand All @@ -46,17 +43,6 @@ class EXPORT BufferManager
const f_cnt_t offset = 0 );
#endif
static void release( sampleFrame * buf );
static void refresh();
// static void extend( int c );

private:
static sampleFrame ** s_available;
static AtomicInt s_availableIndex;

static sampleFrame ** s_released;
static AtomicInt s_releasedIndex;
// static QReadWriteLock s_mutex;
static int s_size;
};

#endif
2 changes: 1 addition & 1 deletion include/MemoryHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
class MemoryHelper {
public:

static void* alignedMalloc( int );
static void* alignedMalloc(size_t );

static void alignedFree( void* );

Expand Down
25 changes: 21 additions & 4 deletions include/MemoryManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ struct MemoryPool
{
void * m_pool;
char * m_free;
int m_chunks;
size_t m_chunks;
QMutex m_mutex;

MemoryPool() :
Expand All @@ -51,10 +51,10 @@ struct MemoryPool
m_chunks( 0 )
{}

MemoryPool( int chunks ) :
MemoryPool( size_t chunks ) :
m_chunks( chunks )
{
m_free = (char*) MemoryHelper::alignedMalloc( chunks );
m_free = reinterpret_cast<char*>(MemoryHelper::alignedMalloc( chunks ));
memset( m_free, 1, chunks );
}

Expand Down Expand Up @@ -103,6 +103,23 @@ class EXPORT MemoryManager
static QMutex s_pointerMutex;
};

template<typename T>
struct MmAllocator
{
typedef T value_type;
template< class U > struct rebind { typedef MmAllocator<U> other; };

T* allocate(std::size_t n)
{
return reinterpret_cast<T*>( MemoryManager::alloc( n ) );
}

void deallocate(T* p, std::size_t)
{
MemoryManager::free( p );
}
};


#define MM_OPERATORS \
public: \
Expand All @@ -124,7 +141,7 @@ static void operator delete[] ( void * ptr ) \
}

// for use in cases where overriding new/delete isn't a possibility
#define MM_ALLOC( type, count ) (type*) MemoryManager::alloc( sizeof( type ) * count )
#define MM_ALLOC( type, count ) reinterpret_cast<type*>(MemoryManager::alloc( sizeof( type ) * count ))
// and just for symmetry...
#define MM_FREE( ptr ) MemoryManager::free( ptr )

Expand Down
94 changes: 38 additions & 56 deletions src/core/BufferManager.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*
* BufferManager.cpp - A buffer caching/memory management system
*
* Copyright (c) 2017 Lukas W <lukaswhl/at/gmail.com>
* Copyright (c) 2014 Vesa Kivimäki <contact/dot/diizy/at/nbl/dot/fi>
* Copyright (c) 2006-2014 Tobias Doerffel <tobydox/at/users.sourceforge.net>
*
Expand All @@ -25,44 +26,60 @@

#include "BufferManager.h"

#include "Engine.h"
#include "Mixer.h"
#include "MemoryManager.h"

sampleFrame ** BufferManager::s_available;
AtomicInt BufferManager::s_availableIndex = 0;
sampleFrame ** BufferManager::s_released;
AtomicInt BufferManager::s_releasedIndex = 0;
//QReadWriteLock BufferManager::s_mutex;
int BufferManager::s_size;
#include <boost/lockfree/stack.hpp>


void BufferManager::init( fpp_t framesPerPeriod )
namespace {
using namespace boost::lockfree;

static const int BM_INITIAL_BUFFERS = 1024;
static const int BM_INCREMENT = 64;

static stack<sampleFrame*
,allocator<MmAllocator<void>>
> s_available;

static fpp_t framesPerPeriod;

void extend( size_t c )
{
s_available = MM_ALLOC( sampleFrame*, BM_INITIAL_BUFFERS );
s_released = MM_ALLOC( sampleFrame*, BM_INITIAL_BUFFERS );
s_available.reserve(c);

size_t cc = c * framesPerPeriod;

int c = framesPerPeriod * BM_INITIAL_BUFFERS;
sampleFrame * b = MM_ALLOC( sampleFrame, c );
sampleFrame * b = MM_ALLOC( sampleFrame, cc );

for( int i = 0; i < BM_INITIAL_BUFFERS; ++i )
for( size_t i = 0; i < c; ++i )
{
s_available[ i ] = b;
s_available.push(b);
b += framesPerPeriod;
}
s_availableIndex = BM_INITIAL_BUFFERS - 1;
s_size = BM_INITIAL_BUFFERS;
}

}



void BufferManager::init( fpp_t framesPerPeriod )
{
::framesPerPeriod = framesPerPeriod;
extend(BM_INITIAL_BUFFERS);
}


sampleFrame * BufferManager::acquire()
{
if( s_availableIndex < 0 )
sampleFrame* b;
while (! s_available.pop(b))
{
qFatal( "BufferManager: out of buffers" );
qWarning( "BufferManager: out of buffers" );
extend(BM_INCREMENT);
}

int i = s_availableIndex.fetchAndAddOrdered( -1 );
sampleFrame * b = s_available[ i ];

//qDebug( "acquired buffer: %p - index %d", b, i );
return b;
}
Expand All @@ -87,42 +104,7 @@ void BufferManager::clear( surroundSampleFrame * ab, const f_cnt_t frames,
void BufferManager::release( sampleFrame * buf )
{
if (buf == nullptr) return;
int i = s_releasedIndex.fetchAndAddOrdered( 1 );
s_released[ i ] = buf;
s_available.push(buf);
//qDebug( "released buffer: %p - index %d", buf, i );
}


void BufferManager::refresh() // non-threadsafe, hence it's called periodically from mixer at a time when no other threads can interfere
{
if( s_releasedIndex == 0 ) return;
//qDebug( "refresh: %d buffers", int( s_releasedIndex ) );

int j = s_availableIndex;
for( int i = 0; i < s_releasedIndex; ++i )
{
++j;
s_available[ j ] = s_released[ i ];
}
s_availableIndex = j;
s_releasedIndex = 0;
}


/* // non-extensible for now
void BufferManager::extend( int c )
{
s_size += c;
sampleFrame ** tmp = MM_ALLOC( sampleFrame*, s_size );
MM_FREE( s_available );
s_available = tmp;
int cc = c * Engine::mixer()->framesPerPeriod();
sampleFrame * b = MM_ALLOC( sampleFrame, cc );
for( int i = 0; i < c; ++i )
{
s_available[ s_availableIndex.fetchAndAddOrdered( 1 ) + 1 ] = b;
b += Engine::mixer()->framesPerPeriod();
}
}*/
2 changes: 1 addition & 1 deletion src/core/MemoryHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* Allocate a number of bytes and return them.
* @param byteNum is the number of bytes
*/
void* MemoryHelper::alignedMalloc( int byteNum )
void* MemoryHelper::alignedMalloc( size_t byteNum )
{
char *ptr, *ptr2, *aligned_ptr;
int align_mask = ALIGN_SIZE - 1;
Expand Down
3 changes: 0 additions & 3 deletions src/core/Mixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,6 @@ const surroundSampleFrame * Mixer::renderNextBuffer()
Controller::triggerFrameCounter();
AutomatableModel::incrementPeriodCounter();

// refresh buffer pool
BufferManager::refresh();

s_renderingThread = false;

m_profiler.finishPeriod( processingSampleRate(), m_framesPerPeriod );
Expand Down

1 comment on commit b6421d4

@lukas-w
Copy link
Member Author

Choose a reason for hiding this comment

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

Commit message says "fixes #3670", but should say "#3223" instead.

Please sign in to comment.