[pulseaudio-discuss] [PATCH] alsa: Avoid creating tiny memchunks on write iterations

[Georg Chini]

On 09.03.2017 05:37, Arun Raghavan wrote:
> If the ALSA device supports granular pointer reporting, we end up in a
> situation where we write out a bunch of data, iterate, and then find a
> small amount of data available in the buffer (consumed while we were
> writing data into the available buffer space). We do this 10 times
> before quitting the write loop.
>
> This is inefficient in itself, but can also have wider consequences. For
> example, with module-combine-sink, this will end up pushing the same
> small chunks to all other devices too.
>
> Given both of these, it just makes sense to not try to write out data
> unless a minimum threshold is available. This could potentially be a
> fragment, but it's likely most robust to just work with a fraction of
> the total available buffer size.
> ---
>   src/modules/alsa/alsa-sink.c | 20 ++++++++++++++++++--
>   1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index 886c735..43cd965 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -88,6 +88,8 @@
>   #define DEFAULT_REWIND_SAFEGUARD_BYTES (256U) /* 1.33ms @48kHz, we'll never rewind less than this */
>   #define DEFAULT_REWIND_SAFEGUARD_USEC (1330) /* 1.33ms, depending on channels/rate/sample we may rewind more than 256 above */
>   
> +#define DEFAULT_WRITE_ITERATION_THRESHOLD 0.03 /* don't iterate write if < 3% of the buffer is available */
> +
>   struct userdata {
>       pa_core *core;
>       pa_module *module;
> @@ -580,12 +582,19 @@ static int mmap_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
>               break;
>           }
>   
> -        if (++j > 10) {
> +        j++;
> +
> +        if (j > 10) {
>   #ifdef DEBUG_TIMING
>               pa_log_debug("Not filling up, because already too many iterations.");
>   #endif
>   
>               break;
> +        } else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD * (u->hwbuf_size - u->hwbuf_unused)))) {

Why do you check for j >=1 here? This should always be true. The same
applies for unix_write(). Otherwise looks good.

> +#ifdef DEBUG_TIMING
> +            pa_log_debug("Not filling up, because <%g%% available.", DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
> +#endif
> +            break;
>           }
>   
>           n_bytes -= u->hwbuf_unused;
> @@ -754,12 +763,19 @@ static int unix_write(struct userdata *u, pa_usec_t *sleep_usec, bool polled, bo
>               break;
>           }
>   
> -        if (++j > 10) {
> +        j++;
> +
> +        if (j > 10) {
>   #ifdef DEBUG_TIMING
>               pa_log_debug("Not filling up, because already too many iterations.");
>   #endif
>   
>               break;
> +        } else if (j >= 1 && (n_bytes < (DEFAULT_WRITE_ITERATION_THRESHOLD * (u->hwbuf_size - u->hwbuf_unused)))) {
> +#ifdef DEBUG_TIMING
> +            pa_log_debug("Not filling up, because <%g%% available.", DEFAULT_WRITE_ITERATION_THRESHOLD * 100);
> +#endif
> +            break;
>           }
>   
>           n_bytes -= u->hwbuf_unused;