Fixes for performance and memory allocation problems #109
Conversation
Passing the QList of selected events leads to an unnecessary copy in those cases where the list is changed afterwards (due to Qt container's copy-on-write semantics). But, in these cases they are overwritten later via setSelection() anyway. Thus, they should be passed by reference.
If toProtocol is false, it is not necessary to copy the event (and delete it afterwards).
When pasting events, they may extend beyond the current duration of the file. In this case the file duration is increased. When iterating from the first to last event, within each iteration the file duration is incremented only by a fraction of the final duration. Thus, it is generally much faster to start with the last event and iterate backwards.
When inserting midi events, the channel corresponding to each event is copied into the protocol stack. Currently, the channel is copied for each single inserted event. However, this is unnecessary and this leads to a massive increase in allocated memory if many events are to be inserted. It is only necessary to copy all channels involved once, but not for each event separately.
|
This seems to break the Event tab from displaying any information when you click on a midi note. Reverting all changes from this PR, Event tab property & value info displays again and able to edit in it. |
Re-inserted call to setSelection(), which is required, because it emits the selectionChanged signal. However, when selecting more than one event, setSelection should be called after the loop over all events and not for each event individually. Thus, calls to setSelection have been added after all calls of selectEvent() with parameter single = false.
|
@Meowchestra You're right. It seems setSelection does more than just changing the selection, and I erroneously removed the emission of the selectedEvents signal. This should be fixed with #740488. |
|
Thanks! Event tab info seems to be working again. I noticed there's still a small memory leak when selecting all notes and pasting over and over, but it's significantly better than before with the multiple gigabytes allocations. I also noticed some smaller pasting issues like if you select all notes across multiple tracks, copy and paste, then drag, it puts all those notes into a single track instead of preserving. And also an issue where if you don't select an entire track's all notes when pasting (only making a small selection), it pastes those events at the beginning of the midi instead of current timestamp where it was copied from. But these are probably separate issues unrelated. |
When inserting a large amount of events (several thousand) into a file, I discovered that the insertion takes a very long time and the allocated memory increases massively to over a GB. To reproduce, just load a file with > 5000 events, copy the events, open a new file, and paste the events. Watch memory and the CPU load in the task manager.
Basically this is about preventing unnecessary copying of midi events and channels.
See the individual commits for details.