On Tue, May 02, 2017 at 03:49:52PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (berra...@redhat.com) wrote:
> > On Tue, May 02, 2017 at 09:34:55AM -0500, Eric Blake wrote:
> > > On 05/02/2017 08:47 AM, Denis V. Lunev wrote:
> > > > Right now QMP and HMP monitors read 1 byte at a time from the socket,
> > > > which
> > > > is very inefficient. With 100+ VMs on the host this easily reasults in
> > >
> > > s/reasults/results/
> > >
> > > > a lot of unnecessary system calls and CPU usage in the system.
> > > >
> > > > This patch changes the amount of data to read to 4096 bytes, which
> > > > matches
> > > > buffer size on the channel level. Fortunately, monitor protocol is
> > > > synchronous right now thus we should not face side effects in reality.
> > >
> > > Do you have any easy benchmarks or measurements to prove what sort of
> > > efficiencies we get? (I believe they exist, but quantifying them never
> > > hurts)
> > >
> > > >
> > > > Signed-off-by: Denis V. Lunev <d...@openvz.org>
> > > > CC: Markus Armbruster <arm...@redhat.com>
> > > > CC: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> > > > CC: Eric Blake <ebl...@redhat.com>
> > > > ---
> > > > monitor.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/monitor.c b/monitor.c
> > > > index be282ec..00df5d0 100644
> > > > --- a/monitor.c
> > > > +++ b/monitor.c
> > > > @@ -3698,7 +3698,7 @@ static int monitor_can_read(void *opaque)
> > > > {
> > > > Monitor *mon = opaque;
> > > >
> > > > - return (mon->suspend_cnt == 0) ? 1 : 0;
> > > > + return (mon->suspend_cnt == 0) ? 4096 : 0;
> > >
> > > Is a hard-coded number correct, or should we be asking the channel for
> > > an actual number?
> >
> > There's no need - this will cause the chardev code to just do a
> > gio_channel_read() with a 4096 byte buffer. The chardev backend
> > impl will then happily return fewer bytes than this - just whatever
> > happens to be pending. IOW this is just acting as an upper bound
> > on the amount of data we read at once. So 4k seems reasonable to
> > me, given the typical size of QMP/HMP command strings.
>
> So there's *no* situation in which that will block?
Correct.
> I'm assuming the reason it read one byte was thats the only thing
> that poll() coming back to you guarantees.
Poll returning with POLLIN set, guarantees there is at least one byte
pending. A read on a pipe, socket or other FD, will return at long as
it has read at least one byte. It'll never block to fill the entire
buffer [1].
Regards,
Daniel
[1] Except if reading from a regular file, in which case POSIX I/O
is broken and it'll happily block while the disk seeks to wherever
the data lives, but that's not an issue for the monitor.
--
|: 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 :|
