Re: [PATCH] virtiofsd: Used glib "shared" thread pool

[Vivek Goyal]

On Tue, Sep 22, 2020 at 01:59:57PM +0100, Daniel P. Berrangé wrote:
> On Mon, Sep 21, 2020 at 05:32:16PM -0400, Vivek Goyal wrote:
> > glib offers thread pools and it seems to support "exclusive" and "shared"
> > thread pools.
> > 
> > https://developer.gnome.org/glib/stable/glib-Thread-Pools.html#g-thread-pool-new
> > 
> > Currently we use "exlusive" thread pools but its performance seems to be
> > poor. I tried using "shared" thread pools and performance seems much
> > better. I posted performance results here.
> > 
> > https://www.redhat.com/archives/virtio-fs/2020-September/msg00080.html
> > 
> > So lets switch to shared thread pools. We can think of making it optional
> > once somebody can show in what cases exclusive thread pools offer better
> > results. For now, my simple performance tests across the board see
> > better results with shared thread pools.
> 
> I'm really curious why  there's any perf difference between shared and
> exclusive thread pools in the GLib impl.
> 
> Looking at the code the main difference between the two is appears to
> be around the way threads are spawned, specifically around the scheduler
> attributes assigned.
> 
> In the shared case, the threads in the pool will have their scheduler
> attributes copied from the very first thread that calls g_thread_pool_new.
> 
> In the exclusive case, the threads in the pool will inherit their
> scheduler attributes from the thread which pushs the job that
> causes the worker thread to be created.
> 
> By schedular attributes, I mean all the items in the 'struct schedattr'
> filled by sched_getattr()
> 
> IOW, if threads in virtiofsd have varying schedular attributes this
> could possibly explain the difference in performance you see between
> the two setups.

Hi Daniel,

Few things.

- I think scheduler attributes are same for the thread creating
  pool as well as for thread pushing the job for virtiofsd.

- My glib2 is old (2.58.3) and I think that did not have sched_getattr()
  stuff.

- One difference I noticed is that in case of shared pool, it does not
  create extra threads if client is doing one request at a time. While
  exclusive pool seemed to push every request to a new thread in pool
  in sort of round robin fashion. It feels keeping requests being served
  from same thread helps in this particilar workload case.

Thanks
Vivek

> 
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  tools/virtiofsd/fuse_virtio.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > Index: qemu/tools/virtiofsd/fuse_virtio.c
> > ===================================================================
> > --- qemu.orig/tools/virtiofsd/fuse_virtio.c 2020-09-21 17:28:27.444438015 
> > -0400
> > +++ qemu/tools/virtiofsd/fuse_virtio.c      2020-09-21 17:28:30.584568910 
> > -0400
> > @@ -695,7 +695,7 @@ static void *fv_queue_thread(void *opaqu
> >      struct fuse_session *se = qi->virtio_dev->se;
> >      GThreadPool *pool;
> >  
> > -    pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, 
> > TRUE,
> > +    pool = g_thread_pool_new(fv_queue_worker, qi, se->thread_pool_size, 
> > FALSE,
> >                               NULL);
> >      if (!pool) {
> >          fuse_log(FUSE_LOG_ERR, "%s: g_thread_pool_new failed\n", __func__);
> > 
> > 
> 
> Regards,
> Daniel
> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|