Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Iterate through positional audio program names only once #5712

Open
vimpostor opened this issue Jun 1, 2022 · 4 comments
Open

Iterate through positional audio program names only once #5712

vimpostor opened this issue Jun 1, 2022 · 4 comments
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors

Comments

@vimpostor
Copy link
Contributor

vimpostor commented Jun 1, 2022

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:

if (strcmp(programNames[i], "GTA5.exe") != 0) {

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 #5704 (comment)

@vimpostor vimpostor added feature-request This issue or PR deals with a new feature triage This issue is waiting to be triaged by one of the project members labels Jun 1, 2022
@Krzmbrzl Krzmbrzl added client good first issue Good for first-time contributors and removed triage This issue is waiting to be triaged by one of the project members labels Jun 2, 2022
@hs094
Copy link

hs094 commented Jul 14, 2022

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

@GrantODP
Copy link

GrantODP commented Nov 7, 2023

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.

@Krzmbrzl
Copy link
Member

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)

@vimpostor
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature good first issue Good for first-time contributors
Projects
None yet
Development

No branches or pull requests

4 participants