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

Optimize the additive DAC code, fixing performance-related hangs #21662

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

Nebuleon
Copy link
Contributor

@Nebuleon Nebuleon commented Aug 1, 2023

Description

In current code, the interrupt that fires when the DAC needs more samples could take so long to process that another interrupt is already waiting, as does another once that one is done, and so on. As a result, processing DAC interrupts may be the only thing the CPU can do. This prevents any code from working which would be able to request a note to be stopped:

  • the matrix scan code, which would allow a note from the audio feature or Audio Clicky to be stopped if the user had released the key that started the note, had the CPU had the opportunity to get to the next matrix scan;
  • the code for MIDI over USB, which would allow a NOTEOFF event to stop a note, had the CPU had the opportunity to get to the next USB interrupt, which has a lower priority than the DAC interrupt in the ChibiOS platform support code (thanks to sigprof on Discord https://discord.com/channels/440868230475677696/1134720693460815975/1135140965233991801 for this tip!).

Here are the changes in decreasing order of importance:

  • changing fmodf out for a while loop that puts the offsets back within the chosen waveform, which is expected to run only once in most cases (saves hundreds of instructions per active tone per sample);
  • multiplying the frequency by a single compile-time constant rather than using two multiplications and two divisions (saves 4 instructions per active tone per sample);
  • changing types to avoid zero-extension on assignment before the implicit conversion to the return type of dac_value_generate (saves 3 instructions per active tone per sample).

Even if the CPU is not hanging due to being busy all the time on a user's board, these optimizations still lower CPU utilization, so the input latency goes down, and/or the user can increase the maximum number of concurrent notes or the sample rate. fmodf being gone also saves 416 bytes of firmware size on STM32F303. So this is still a win.

However, more stress testing is needed to make sure there aren't more causes for audio-related hangs.

Types of Changes

  • Core
  • Bugfix (arguably)
  • New feature
  • Enhancement/optimization (definitely)
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).
    I have also tested that the changes improve things.

Before this commit, the interrupt that fires when the DAC needs more samples could take so long to process that another interrupt would already be waiting, as would another once that one was done, and so on. As a result, processing DAC interrupts was the only thing the CPU could do. This prevented any code from working which would be able to request a note to be stopped:

* the matrix scan code, which would allow a note from the audio feature or Audio Clicky to be stopped if the user had released the key that started the note, had the CPU had the opportunity to get to the next matrix scan;

* the code for MIDI over USB, which would allow a NOTEOFF event to stop a note, had the CPU had the opportunity to get to the next USB interrupt, which has a lower priority than the DAC interrupt in the ChibiOS platform support code (thanks to sigprof on Discord <https://discord.com/channels/440868230475677696/1134720693460815975/1135140965233991801> for this tip!).

Here are the changes in decreasing order of importance:

* changing `fmodf` out for a `while` loop that puts the offsets back within the chosen waveform, which is expected to run only once in most cases (saves hundreds of instructions per active tone per sample);

* multiplying the frequency by a single compile-time constant rather than using two multiplications and two divisions (saves 4 instructions per active tone per sample);

* changing types to avoid zero-extension on assignment before the implicit conversion to the return type of `dac_value_generate` (saves 3 instructions per active tone per sample).
@github-actions github-actions bot added the core label Aug 1, 2023
@drashna drashna requested a review from a team August 2, 2023 05:43
@tzarc tzarc merged commit 1d94de5 into qmk:develop Sep 25, 2023
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Oct 25, 2023
christrotter pushed a commit to christrotter/qmk_firmware that referenced this pull request Nov 28, 2023
zgagnon pushed a commit to zgagnon/qmk_firmware_waterfowl that referenced this pull request Dec 15, 2023
future-figs pushed a commit to future-figs/qmk_firmware that referenced this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants