MM tweaks still

[paulfd]

One of the consuming operation is looping through the sources for nothing since we're basically never hitting the right sources (or targets) for a given voice id. This patch adds a container that maps a NumericId<Region> to a set of source indices. This way we have one lookup through the container, and an iteration to go through all the relevant sources.

http://box.ferrand.cc/sfizz-reports/2020-09-09-mm-tweaks-still.html

Notice that this completely remove the "residual" part almost, which was where all these failed lookups were going to. What's left now is the lookup per-voice for each modulation target on rendering each block, but I think this could be memorized within the voice.

[paulfd]

4ccce85 seems to be the last of the low hanging fruit there. Searching through the targets for the key was quite costly, and the search happens for n_targets * n_modulation_keys in each render block. Saving the target ids on voice start seems to do the trick here, at the cost of more private variable for the voice.

http://box.ferrand.cc/sfizz-reports/2020-09-09-mm-tweaks-still-2.html

[paulfd]

We now see more clearly that there are some big spikes in dispatch duration (Dispatch duration (avg/max): 4.42/1278.73 µs) that probably stem from all the polyphony checks.

[jpcima]

This PR modifies the vstgui submodule, you should probably undo that.

[jpcima]

Yes, it's a bit a solution like this I wish to have, but it can be even better.

1. Given the maximum region is a known number after loading sfz, the hashtable can be removed in favor of an array.
2. The index vector can be replaced with a linked list. The link pointers are stored inside the source or target.

Imo, sfizz really needs a generic intrusive list structure.
The midi processing could take avantage of it as well.

[paulfd]

Yes, it's a bit a solution like this I wish to have, but it can be even better.

1. Given the maximum region is a known number after loading sfz, the hashtable can be removed in favor of an array.

I chose the hash table because it seemed to me that the number() could actually be -1 when the source was a CC and not linked to a region. I thus wondered if you had only this special case in mind or if, in the future, we could really expect this number to be anything and figured this was an easy compromise to make and check if this was indeed a big gain. I'm not against an array though. We could even have 2, one for positive and one for negative NumericId, assuming int stays as the representation of it 🙂

2. The index vector can be replaced with a linked list. The link pointers are stored inside the source or target.

Imo, sfizz really needs a generic intrusive list structure. The midi processing could take avantage of it as well.

I guess the "sister voices" are kind of like this no? I did this for the sister voices because they can change and be reassigned to other rings, but in this case this data structure is quite fixed, so I don't know. I don't have a strong argument against though. How does this help midi?

[paulfd]

This PR modifies the vstgui submodule, you should probably undo that.

Yes I'm always happy when submodules do this to me !

[jpcima]

How does this help midi?

In the PR #407 it's basically implementing an adhoc linked list.
The sister ring it's the same thing, except circular.

[paulfd]

Yes, it's a bit a solution like this I wish to have, but it can be even better.

1. Given the maximum region is a known number after loading sfz, the hashtable can be removed in favor of an array.

So I tested this using 2 arrays (for negative and positive indices). The difference in performance seems non-existent, so it might not be necessary to bother.

https://box.ferrand.cc/sfizz-reports/2020-09-11-hashmap-vs-vector-mm.html

[jpcima]

I don't understand: why does this not use an ordinary vector?
Regions don't have negative identifiers.

[paulfd]

I guess I am just extremely confused about the need and use of this NumericId then, but OK... Anyway, the result is still the same, using any kind of vector over hashmaps there seems to have no impact on actual performance.