Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce allocations to improve performance. #3868

Closed
wants to merge 6 commits into from
Closed

Reduce allocations to improve performance. #3868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ed24343
Allocate all six oscillators as a single chunk of memory
BumblingCoder Oct 10, 2017
0a9aba9
Merge pull request #1 from LMMS/stable-1.2
BumblingCoder Oct 10, 2017
474b042
Allocate all six oscillators as a single chunk of memory
BumblingCoder Oct 10, 2017
7d6745f
Merge remote-tracking branch 'origin/ReduceTripleOscAlloc' into Reduc…
BumblingCoder Oct 10, 2017
316be79
Reduce the number of allocations.
BumblingCoder Oct 10, 2017
01fcab8
Fix whitespace
BumblingCoder Oct 10, 2017
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
4 changes: 4 additions & 0 deletions include/Oscillator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ class IntModel;
class EXPORT Oscillator
{
MM_OPERATORS
void* operator new( std::size_t, void* p )
Copy link
Member

Choose a reason for hiding this comment

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

Why is this override needed? Isn't this the default implementation?

{
return p;
}
public:
enum WaveShapes
{
Expand Down
27 changes: 12 additions & 15 deletions plugins/organic/organic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,9 @@ void organicInstrument::playNote( NotePlayHandle * _n,

if( _n->totalFramesPlayed() == 0 || _n->m_pluginData == NULL )
{
Oscillator * oscs_l[m_numOscillators];
Oscillator * oscs_r[m_numOscillators];
Oscillator * oscs_all = MM_ALLOC( Oscillator, m_numOscillators*2 );
Copy link
Member

Choose a reason for hiding this comment

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

Seeing that Oscillator has overloaded new operators through the MM_OPERATORS macro, we should be able to replace this with

Oscillator* oscs_all = new Oscillator[m_numOscillators * 2]

Oscillator * oscs_l = oscs_all;
Oscillator * oscs_r = oscs_all + m_numOscillators;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this alternative is clearer to readers, but I'll leave it up to you

Oscillator * oscs_l = & oscs_all[0];
Oscillator * oscs_r = & oscs_all[m_numOscillators];


_n->m_pluginData = new oscPtr;

Expand All @@ -250,15 +251,15 @@ void organicInstrument::playNote( NotePlayHandle * _n,
if( i == m_numOscillators - 1 )
{
// create left oscillator
oscs_l[i] = new Oscillator(
new (&oscs_l[i]) Oscillator(
&m_osc[i]->m_waveShape,
&m_modulationAlgo,
_n->frequency(),
m_osc[i]->m_detuningLeft,
static_cast<oscPtr *>( _n->m_pluginData )->phaseOffsetLeft[i],
m_osc[i]->m_volumeLeft );
// create right oscillator
oscs_r[i] = new Oscillator(
new (&oscs_r[i]) Oscillator(
&m_osc[i]->m_waveShape,
&m_modulationAlgo,
_n->frequency(),
Expand All @@ -269,30 +270,30 @@ void organicInstrument::playNote( NotePlayHandle * _n,
else
{
// create left oscillator
oscs_l[i] = new Oscillator(
new (&oscs_l[i]) Oscillator(
&m_osc[i]->m_waveShape,
&m_modulationAlgo,
_n->frequency(),
m_osc[i]->m_detuningLeft,
static_cast<oscPtr *>( _n->m_pluginData )->phaseOffsetLeft[i],
m_osc[i]->m_volumeLeft,
oscs_l[i + 1] );
&oscs_l[i + 1] );
// create right oscillator
oscs_r[i] = new Oscillator(
new (&oscs_r[i]) Oscillator(
&m_osc[i]->m_waveShape,
&m_modulationAlgo,
_n->frequency(),
m_osc[i]->m_detuningRight,
static_cast<oscPtr *>( _n->m_pluginData )->phaseOffsetRight[i],
m_osc[i]->m_volumeRight,
oscs_r[i + 1] );
&oscs_r[i + 1] );
}


}

static_cast<oscPtr *>( _n->m_pluginData )->oscLeft = oscs_l[0];
static_cast<oscPtr *>( _n->m_pluginData )->oscRight = oscs_r[0];
static_cast<oscPtr *>( _n->m_pluginData )->oscLeft = &oscs_l[0];
static_cast<oscPtr *>( _n->m_pluginData )->oscRight = &oscs_r[0];
Copy link
Member

Choose a reason for hiding this comment

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

&oscs_l[0] and &oscs_r[0] can be written as oscs_l and oscs_r respectively.

}

Oscillator * osc_l = static_cast<oscPtr *>( _n->m_pluginData )->oscLeft;
Expand Down Expand Up @@ -325,11 +326,7 @@ void organicInstrument::playNote( NotePlayHandle * _n,

void organicInstrument::deleteNotePluginData( NotePlayHandle * _n )
{
delete static_cast<Oscillator *>( static_cast<oscPtr *>(
_n->m_pluginData )->oscLeft );
delete static_cast<Oscillator *>( static_cast<oscPtr *>(
_n->m_pluginData )->oscRight );

MM_FREE( static_cast<oscPtr *>(_n->m_pluginData )->oscLeft);
Copy link
Member

Choose a reason for hiding this comment

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

In analogy to the allocation, this should be written as

delete[] static_cast<oscPtr *>(_n->m_pluginData )->oscLeft

Using MM_FREE won't call the object's destructors.

delete static_cast<oscPtr *>( _n->m_pluginData );
}

Expand Down
33 changes: 16 additions & 17 deletions plugins/triple_oscillator/TripleOscillator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,23 +300,24 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
{
Copy link
Member

Choose a reason for hiding this comment

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

Same comments apply to TripleOscillator.cpp.

if( _n->totalFramesPlayed() == 0 || _n->m_pluginData == NULL )
{
Oscillator * oscs_l[NUM_OF_OSCILLATORS];
Oscillator * oscs_r[NUM_OF_OSCILLATORS];
auto allOscs = MM_ALLOC( Oscillator, NUM_OF_OSCILLATORS*2 );
Oscillator * oscs_l = allOscs;
Oscillator * oscs_r = allOscs + NUM_OF_OSCILLATORS;

for( int i = NUM_OF_OSCILLATORS - 1; i >= 0; --i )
{

// the last oscs needs no sub-oscs...
if( i == NUM_OF_OSCILLATORS - 1 )
{
oscs_l[i] = new Oscillator(
new (&oscs_l[i]) Oscillator(
&m_osc[i]->m_waveShapeModel,
&m_osc[i]->m_modulationAlgoModel,
_n->frequency(),
m_osc[i]->m_detuningLeft,
m_osc[i]->m_phaseOffsetLeft,
m_osc[i]->m_volumeLeft );
oscs_r[i] = new Oscillator(
new (&oscs_r[i])Oscillator(
&m_osc[i]->m_waveShapeModel,
&m_osc[i]->m_modulationAlgoModel,
_n->frequency(),
Expand All @@ -326,33 +327,32 @@ void TripleOscillator::playNote( NotePlayHandle * _n,
}
else
{
oscs_l[i] = new Oscillator(
new (&oscs_l[i]) Oscillator(
&m_osc[i]->m_waveShapeModel,
&m_osc[i]->m_modulationAlgoModel,
_n->frequency(),
m_osc[i]->m_detuningLeft,
m_osc[i]->m_phaseOffsetLeft,
m_osc[i]->m_volumeLeft,
oscs_l[i + 1] );
oscs_r[i] = new Oscillator(
&oscs_l[i + 1] );
new (&oscs_r[i]) Oscillator(
&m_osc[i]->m_waveShapeModel,
&m_osc[i]->m_modulationAlgoModel,
_n->frequency(),
m_osc[i]->m_detuningRight,
m_osc[i]->m_phaseOffsetRight,
m_osc[i]->m_volumeRight,
oscs_r[i + 1] );
&oscs_r[i + 1] );
}

oscs_l[i]->setUserWave( m_osc[i]->m_sampleBuffer );
oscs_r[i]->setUserWave( m_osc[i]->m_sampleBuffer );
oscs_l[i].setUserWave( m_osc[i]->m_sampleBuffer );
oscs_r[i].setUserWave( m_osc[i]->m_sampleBuffer );

}

_n->m_pluginData = new oscPtr;
static_cast<oscPtr *>( _n->m_pluginData )->oscLeft = oscs_l[0];
static_cast< oscPtr *>( _n->m_pluginData )->oscRight =
oscs_r[0];
static_cast<oscPtr *>( _n->m_pluginData )->oscLeft = &oscs_l[0];
static_cast<oscPtr *>( _n->m_pluginData )->oscRight = &oscs_r[0];
}

Oscillator * osc_l = static_cast<oscPtr *>( _n->m_pluginData )->oscLeft;
Expand All @@ -374,10 +374,9 @@ void TripleOscillator::playNote( NotePlayHandle * _n,

void TripleOscillator::deleteNotePluginData( NotePlayHandle * _n )
{
delete static_cast<Oscillator *>( static_cast<oscPtr *>(
_n->m_pluginData )->oscLeft );
delete static_cast<Oscillator *>( static_cast<oscPtr *>(
_n->m_pluginData )->oscRight );
auto ptr = static_cast<oscPtr*>(_n->m_pluginData)->oscLeft;
//It is safe to simply free the oscillators because they don't own anything.
MM_FREE( ptr );
delete static_cast<oscPtr *>( _n->m_pluginData );
}

Expand Down