[pulseaudio-discuss] Vacuuming per-client mempools (was: [PATCH v2 02/12] pulsecore: srbchannel: Introduce per-client SHM files)

[Ahmed S. Darwish]

On Fri, Feb 12, 2016 at 02:09:31AM +0200, Ahmed S. Darwish wrote:
> The PA daemon currently uses a single SHM file for all the clients
> sending and receiving commands over the low-latency srbchannel
> mechanism.
> 
> To safely run PA daemon in system mode later using memfds, and to
> provide the necessary ground work for sandboxing, create the
> srbchannel SHM files on a per-client basis.
>
...
> @@ -285,8 +278,16 @@ void pa_core_maybe_vacuum(pa_core *c) {
>  
>      pa_mempool_vacuum(c->mempool);
>  
> -    if (c->rw_mempool)
> -        pa_mempool_vacuum(c->rw_mempool);
> +    /*
> +     * FIXME: Vacuum the per-client rw (srbchannel) mempools
> +     *
> +     * We need to vacuum the per-client mempool owned by each connection
> +     * at pa_native_protocol::connections. Nonetheless, libpulsecore can
> +     * not access any symbol from protocol-native.
> +     *
> +     * Vacuuming a per-client mempool on its streams deletion and
> +     * uncorking also proved to be problematic.
> +     */
>  }
>

During patch review of v1 submission, a consensus was reached to
vacuum the per-client mempool according to streams activity. [*]

So the logic goes:

- Upon corking a stream, if there are no more 'active' streams, let's
  vacuum per-client mempool

- Upon deleting a stream, if all the remaining streams are not active
  (corked) or no more streams left, vacuum

- Upon finishing an upload stream, if no other streams remain or all
  are inactive, vacuum

When implementing this seemingly-innocent logic, I've found that we
are vacuuming way too much. 

Here's a case-by-case analysis:

#####################################################################

1) GNOME music, official gnome3 music application
   https://wiki.gnome.org/Apps/Music

   After starting application, _clicking_ a song to play we get:

       CREATING/CONNECTING stream with name = pulsesink probe
       DELETING stream with name = pulsesink probe
       <----------------- VACUUM ------------------->
       CREATING/CONNECTING stream with name = Playback Stream
       UN_CORKING stream with name = Playback Stream
   
   Whenever the app _transitions_ from song to song, we get:

       CORKING stream with name = Playback Stream
       <----------------- VACUUM ------------------->
       CORKING stream with name = Playback Stream       
       DELETING stream with name = Playback Stream
  

       CREATING/CONNECTING stream with name = pulsesink probe
       DELETING stream with name = pulsesink probe
       <----------------- VACUUM ------------------->
       CREATING/CONNECTING stream with name = Playback Stream
       UN-CORKING stream with name = Playback Stream

   When _seeking_ a playing song, we get:

       CORKING stream with name = Playback Stream
       <----------------- VACUUM ------------------->
       UN-CORKING stream with name = Playback Stream

   FINAL RESULT:
       2 vacuums per song transition
       1 vacuum per song seek
       
#####################################################################

2) Audacious, XMMS successor, http://audacious-media-player.org/
   Audacious uses ALSA output by default.

   After starting application, clicking a song to play we get

       CREATING/CONNECTING stream with name = ALSA Playback
       DELETING stream with name = ALSA Playback
       <----------------- VACUUM ------------------->
       CREATING/CONNECTING stream with name = ALSA Playback
       UN-CORKING stream with name = ALSA Playback

   Whenever the app transitions from song to song, we get:

       CORKING stream with name = ALSA Playback
       <----------------- VACUUM ------------------->
       DELETING stream with name = ALSA Playback
       CREATING/CONNECTING stream with name = ALSA Playback
       UN-CORKING stream with name = ALSA Playback

   When seeking a playing song, we get:

       CORKING stream with name = ALSA Playback
       <----------------- VACUUM ------------------->
       DELETING stream with name = ALSA Playback
       CREATING/CONNECTING stream with name = ALSA Playback
       UN-CORKING stream with name = ALSA Playback

   FINAL RESULT:
       1 vacuum per song transition
       1 vacuum per song seek

#####################################################################

So unfortunately it seems this approach will produce a vacuuming
overkill and possibly hurt our performance.

The original logic depended on the core.c to detect that all sources
and sinks are idle, and if so, vacuum all the server mempools. Trying
to maintain this logic faces the issue of libpulsecore not being
able to access any symbol from protocol-native.

So what is the best way to handle this issue?

Thanks!

[*] http://article.gmane.org/gmane.comp.audio.pulseaudio.general/24752

Darwish
http://darwish.chasingpointers.com