[PATCH wayland 8/8] client: Use read preparation API in wl_display_dispatch_queue()

[Bryce Harrington]

On Fri, Oct 02, 2015 at 01:42:03PM -0700, Bryce Harrington wrote:
> On Fri, Oct 02, 2015 at 05:32:59PM +0800, Jonas Ådahl wrote:
> > Instead of doing things that do the equivalent of using
> > wl_display_prepare_read() and friends, just use the public API. The
> > only semantical difference is that we will now unlock and lock the mutex
> > more times compared to before.
> > 
> > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>

In this series, patches 1,2,3,5 are mainly doc cleanup and look fine to
land.  If no one comments to the contrary by EOD today I'll just go
ahead and land them.  On 4 I spotted a grammar issue but otherwise also
looks good to go.

Patches 6-8 look to me like they deserve a bit deeper review.

Thanks,
Bryce


> > There is no real need to merge this patch, it only makes it easier to
> > understand that the _dispatch(_queue)() functions are compatible with the
> > prepare_read() family.
> 
> I'm all for making code more concise and clear.  Is there any concern
> though that the added locking activity impacts performance negatively?
> 
> Technically I see nothing wrong with the implementation.
> 
> > Jonas
> > 
> > 
> >  src/wayland-client.c | 35 ++++++++---------------------------
> >  1 file changed, 8 insertions(+), 27 deletions(-)
> > 
> > diff --git a/src/wayland-client.c b/src/wayland-client.c
> > index ca2495f..8f53f6f 100644
> > --- a/src/wayland-client.c
> > +++ b/src/wayland-client.c
> > @@ -1507,15 +1507,9 @@ wl_display_dispatch_queue(struct wl_display *display,
> >  	struct pollfd pfd[2];
> >  	int ret;
> >  
> > -	pthread_mutex_lock(&display->mutex);
> > -
> > -	ret = dispatch_queue(display, queue);
> > -	if (ret == -1)
> > -		goto err_unlock;
> > -	if (ret > 0) {
> > -		pthread_mutex_unlock(&display->mutex);
> > +	ret = wl_display_dispatch_queue_pending(display, queue);
> > +	if (ret != 0)
> >  		return ret;
> > -	}
> >  
> >  	/* We ignore EPIPE here, so that we try to read events before
> >  	 * returning an error.  When the compositor sends an error it
> > @@ -1524,12 +1518,11 @@ wl_display_dispatch_queue(struct wl_display *display,
> >  	ret = wl_connection_flush(display->connection);
> >  	if (ret < 0 && errno != EAGAIN && errno != EPIPE) {
> >  		display_fatal_error(display, errno);
> > -		goto err_unlock;
> > +		return -1;
> >  	}
> >  
> > -	display->reader_count++;
> > -
> > -	pthread_mutex_unlock(&display->mutex);
> > +	if (wl_display_prepare_read_queue(display, queue) != 0)
> > +		return -1;
> >  
> >  	pfd[0].fd = display->fd;
> >  	pfd[0].events = POLLIN;
> > @@ -1542,22 +1535,10 @@ wl_display_dispatch_queue(struct wl_display *display,
> >  		return -1;
> >  	}
> >  
> > -	pthread_mutex_lock(&display->mutex);
> > -
> > -	if (read_events(display) == -1)
> > -		goto err_unlock;
> > -
> > -	ret = dispatch_queue(display, queue);
> > -	if (ret == -1)
> > -		goto err_unlock;
> > -
> > -	pthread_mutex_unlock(&display->mutex);
> > -
> > -	return ret;
> > +	if (wl_display_read_events(display) == -1)
> > +		return -1;
> >  
> > - err_unlock:
> > -	pthread_mutex_unlock(&display->mutex);
> > -	return -1;
> > +	return wl_display_dispatch_queue_pending(display, queue);
> >  }
> >  
> >  /** Dispatch pending events in an event queue
> > -- 
> > 2.4.3
> > 
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel