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

soundd: improve handling of silent periods by using fill(0) Instead of array multiplication #33105

Closed
wants to merge 1 commit into from

Conversation

deanlee
Copy link
Contributor

@deanlee deanlee commented Jul 27, 2024

Returning np.zeros(frames, dtype=np.float32) * self.current_volume when no sound needs to be played is a simple way to ensure silence. but introduces unnecessary CPU overhead and memory allocation:

  1. Performance Impact: Creating a new numpy array and multiplying it by self.current_volume every time the callback is called can introduce unnecessary CPU overhead, especially if the callback is invoked frequently.
  2. Memory Allocation: Continuously allocating memory for zeros could lead to inefficient memory usage over time.

This PR improves performance by handling silent periods more efficiently:

  1. Efficient Silent Period Handling: Replaces array creation and multiplication with direct buffer filling using fill(0), eliminating redundant operations.
  2. Reduced CPU and Memory Usage: Minimizes unnecessary computations and memory allocations, potentially saving up to 1% in CPU usage during silent periods based on preliminary tests.

Note::
To further optimize, consider using stream.stop() and stream.start() to effectively pause and resume the audio stream dynamically based on the current_alert state.

@github-actions github-actions bot added the ui label Jul 27, 2024
Copy link
Contributor

github-actions bot commented Jul 27, 2024

Thanks for contributing to openpilot! In order for us to review your PR as quickly as possible, check the following:

  • Convert your PR to a draft unless it's ready to review
  • Read the contributing docs
  • Before marking as "ready for review", ensure:
    • the goal is clearly stated in the description
    • all the tests are passing
    • the change is something we merge
    • include a route or your device' dongle ID if relevant

Copy link
Contributor

github-actions bot commented Jul 27, 2024

UI Screenshots

@adeebshihadeh
Copy link
Contributor

The current handling is intentional to keep resource usage consistent.

Ideally, every second openpilot runs is identical to every other second in terms of CPU, memory, etc.

@deanlee
Copy link
Contributor Author

deanlee commented Jul 28, 2024

I understand. I've removed the note section for further optimization.

The CPU usage of the original code differs between silent and play modes, increasing during playback. This PR aims to reduce unnecessary resource usage in silent mode without changing the original structure, which should not violate the principle of maintaining consistent resource usage. What do you think?

Copy link
Contributor

github-actions bot commented Aug 7, 2024

This PR has had no activity for 9 days. It will be automatically closed in 2 days if there is no activity.

@github-actions github-actions bot added the stale label Aug 7, 2024
Copy link
Contributor

github-actions bot commented Aug 9, 2024

This PR has been automatically closed due to inactivity. Feel free to re-open once activity resumes.

@github-actions github-actions bot closed this Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants