Iterate through positional audio program names only once
Context
Right now Mumble iterates through every program name of every PID for every positional audio plugin. This is quite wasted CPU time if either the PID count is high or plugin count is high, as the time complexity is in $\mathcal{O}(PIDNum \cdot pluginNum )$.
Example: https://github.com/mumble-voip/mumble/blob/dd5df45313a937359285327f556557159bab758e/plugins/gtav/gtav.cpp#L81
Description
A better solution would be if each plugin tells Mumble what program name it is looking for, so that only Mumble has to iterate over the PID-list once.
Mumble component
Client
OS-specific?
No
Additional information
This issue originated from https://github.com/mumble-voip/mumble/pull/5704#issuecomment-1141938952
Hi I am Hardik Soni. A Open Source Contributor, Please assign this issue to me.
Hi, First time commenting.
Would a better solution be an observable map?
Mumble could attach each plugin in use as a subscriber to the map for the program name it wants. If the map is updated at [PROGRAM_NAME] you can notify the plugin and send the data it needs. You can send the program name and PIDs directly to the plugin function. That way Mumble only needs to traverse the program name list once.
That seems pretty much in line with OP's original suggestion, yeah. The key point here is that the iteration over available programs has to be done on Mumble's side while all plugins only tell Mumble what name they are looking for. That way the list of processed only has to be traversed once.
Storing the plugin's requests in a map seems conceptually like a good idea that should allow for a reasonable clean implementation of lookups. If performance is of high importance for this, we should consider a flat map in order to make best use of CPU cache (aka: don't use std maps as they generally suck in terms of cache locality)
If performance is of high importance for this, we should consider a flat map in order to make best use of CPU cache (aka: don't use
stdmaps as they generally suck in terms of cache locality)
I wouldn't bikeshed about the data structure here, the performance difference for the map type would be not measurable if the rest is designed correctly.
There actually is a backwards API source-compatible way to implement this:
- For
mumble_initPositionalData()add a new return code, something likeMUMBLE_PDEC_ERROR_WAITS_PID - Add a new method for plugins to call
mumble_waitForProcess(const char* name), which tells Mumble to stop callingmumble_initPositionalData()every time for this plugin and instead defers the work to Mumble to check fornameto appear in the process list. Once the name appears in the process list, Mumble will callmumble_initPositionalData()on the plugin again, butprogramNamesandprogramPIDsare filtered for processes that matchnamepassed earlier from the plugin - In the GTAV example above, we could leave almost everything as-is, but if no matching PID is found, instead of returning
MUMBLE_PDEC_ERROR_TEMP, we first callmumble_waitForProcess("GTA5.exe")and then return withMUMBLE_PDEC_ERROR_WAITS_PID. With this, Mumble would not call the plugin again until "GTA5.exe" appears in the process list, at which point it would call into the plugin again but only with processes that match "GTA5.exe" (should hopefully be only one, but leave it to the plugin to decide if multiple match).
And again, the data structure to use is not really that relevant, the main performance improvement here is that not every plugin has to iterate through the entire process list.