Performance: Creation of DetuningHelper in class Note slows down file load and MIDI import

[michaelgregorius]

I did some profiling with Valgrind and it turns out that the loading of files and the import of MIDI files is slowed down by the creation of DetuneHelper in Note.

You can test the impact of the code for file loading as follows:

1. Compile a vanilla version from master.
2. Start LMMS and load the file DnB.mmpz from the Demos folder.
3. Close LMMS.
4. Comment out the calls to createDetuning in Note::Note and Note::loadSettings.
5. Recompile and start LMMS again.
6. Load DnB.mmpz from the Demos folder again.

The impact for MIDI import can be tested in a similar fashion. You only have to import a MIDI file instead of loading an LMMS file. An example MIDI file can be found at http://www.eurokdj.com/ringtones/midi_files/Twenty_4_Seven-Take_Me_Away.mid. (Please refer to #1971 for other source for MIDI files).

The main problem with createDetuning seems to be the sheer number of DetuningHelpers that are created during the load/import. Even though they are created using the MemoryPool their creation still slows the code down significantly. It's very likely that this problem can only be alleviated by changing the design. Does every note really need its own DetuningHelper? Is it possible to keep the DetuningHelper as an aggregate in Note and not as a pointer?

[softrabbit]

Does every note really need its own DetuningHelper?

I don't think so. You could pass the same empty DetuningHelper to all the Notes. Then you'd need to add a way to swap in another somehow, I don't think there is one now. Or maybe allow Notes with no DetuningHelper, which could mean adding checks for null pointers in a lot of places and potential bugs.

Is it possible to keep the DetuningHelper as an aggregate in Note and not as a pointer?

Would that be faster? There'd still be a pointer to an AutomationPattern down in InlineAutomation if I'm reading this right.

[SecondFlight]

Added the bug label to make this more trackable.

[softrabbit]

Should this be closed as fixed through the memory manager change? #3312 (comment)

[michaelgregorius]

Yes, I think it can be closed. Closing.