From: Stefan Sperling <stsp@stsp.name>
Subject: Re: make read_imsg() poll only if necessary
To: gameoftrees@openbsd.org
Date: Thu, 19 May 2022 09:33:40 +0200

I committed this but then realized it is wrong.

imsg_read() goes straight to receivemsg() regardless of what the
state of the read buffer says. So we should poll the fd for data
in any case before calling imsg_read().

On Mon, May 16, 2022 at 10:23:00AM +0200, Stefan Sperling wrote:
> Our wrapper around imsg_read() always calls poll before reading data.
> 
> If there is still data left in the read buffer of struct imsgbuf then
> calling poll is unnecessary overhead. This matters when the sender
> batches up many small messages (such as tree entries) and the reader
> keeps looping over messages with read_imsg().
> 
> I assume that we will be left with less than one imsg header worth of
> bytes in case the read buffer has been drained. I could not find a
> better way to tell whether we still have pending buffered data.
> 
> Before anyone asks: ibuf_size() looks at the message write buffer,
> not at the message read buffer. There doesn't seem to be an equivalent
> of ibuf_size() for the read buffer.
> 
> ok?
> 
> diff ff5c2684cac837a3de8ad64e56535dd0aa7ee36d a1478be81e81d253e3b4a3e7906089595c32077c
> blob - 1fe975d678cb56f434aa5be0175de5eaef64f9ed
> blob + 0dfcb1daa48f59a43453104f2e4950d2b21a8506
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -94,9 +94,15 @@ read_imsg(struct imsgbuf *ibuf)
>  	const struct got_error *err;
>  	size_t n;
>  
> -	err = poll_fd(ibuf->fd, POLLIN, INFTIM);
> -	if (err)
> -		return err;
> +	/*
> +	 * There is no imsg API function to tell us whether the
> +	 * read buffer still contains pending data :-(
> +	 */
> +	if (ibuf->r.wpos < IMSG_HEADER_SIZE) {
> +		err = poll_fd(ibuf->fd, POLLIN, INFTIM);
> +		if (err)
> +			return err;
> +	}
>  
>  	n = imsg_read(ibuf);
>  	if (n == -1) {
> 
>