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

[Arun Raghavan]

On Thu, 9 Mar 2017, at 09:47 PM, Georg Chini wrote:
> 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.

Right, that should be >= 2. Will fix it up.

Thank you for the reviews!

-- Arun