mumble icon indicating copy to clipboard operation
mumble copied to clipboard

Iterate through positional audio program names only once

Open vimpostor opened this issue 3 years ago • 4 comments

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

vimpostor avatar Jun 01 '22 14:06 vimpostor

Hi I am Hardik Soni. A Open Source Contributor, Please assign this issue to me.

hs094 avatar Jul 14 '22 16:07 hs094

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.

GrantODP avatar Nov 07 '23 12:11 GrantODP

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)

Krzmbrzl avatar Nov 29 '23 19:11 Krzmbrzl

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)

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 like MUMBLE_PDEC_ERROR_WAITS_PID
  • Add a new method for plugins to call mumble_waitForProcess(const char* name), which tells Mumble to stop calling mumble_initPositionalData() every time for this plugin and instead defers the work to Mumble to check for name to appear in the process list. Once the name appears in the process list, Mumble will call mumble_initPositionalData() on the plugin again, but programNames and programPIDs are filtered for processes that match name passed 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 call mumble_waitForProcess("GTA5.exe") and then return with MUMBLE_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.

vimpostor avatar Dec 06 '23 12:12 vimpostor