[Libguestfs] [PATCH nbdkit] python: Pass memoryview to pwrite()

[Richard W.M. Jones]

On Sat, Nov 23, 2019 at 05:49:05PM +0200, Nir Soffer wrote:
> Passing a memoryview we avoid unneeded copy of the original buffer. On
> the python side memoryview object can be used for slicing, writing to
> file, or sending to socket.
> 
> This may break plugins assuming that the they get a bytearray, but
> good python code should not care about the type of the buffer, only
> about the behaviour.
> 
> Testing with a plugin writing to /dev/null shows 2.7x speedup. Real
> plugin will probably show much smaller improvement.
> 
> Without patch:
> 
> $ time qemu-img convert -p -f raw -O raw -n /var/tmp/disk.img nbd://localhost/
>     (100.00/100%)
> 
> real	0m1.284s
> user	0m0.091s
> sys	0m0.576s
> 
> With patch:
> 
> $ time qemu-img convert -p -f raw -O raw -n /var/tmp/disk.img nbd://localhost/
>     (100.00/100%)
> 
> real	0m0.477s
> user	0m0.078s
> sys	0m0.653s
> ---
> 
> More info on how I tested this:
> 
> # Creating test image
> $ dd if=/dev/zero bs=1M count=1024 | tr "\0" "U" > /var/tmp/disk.img
> 
> $ cat zero.py
> import builtins
> 
> def open(readonly):
>     return builtins.open("/dev/zero", "r+b")
> 
> def get_size(h):
>     return 1024**3
> 
> def pwrite(h, buf, offset):
>     h.write(buf)
> 
> def pread(h, count, offset):
>     raise NotImplementedError
> 
> def close(h):
>     h.close()
> 
> # Running nbdkit
> $ ./nbdkit -f -v python zero.py
> 
>  plugins/python/python.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/python/python.c b/plugins/python/python.c
> index 214fffb..2b4361a 100644
> --- a/plugins/python/python.c
> +++ b/plugins/python/python.c
> @@ -496,8 +496,8 @@ py_pwrite (void *handle, const void *buf,
>      PyErr_Clear ();
>  
>      r = PyObject_CallFunction (fn, "ONL", obj,
> -                               PyByteArray_FromStringAndSize (buf, count),
> -                               offset, NULL);
> +            PyMemoryView_FromMemory ((char *)buf, count, PyBUF_READ),
> +            offset, NULL);
>      Py_DECREF (fn);
>      if (check_python_failure ("pwrite") == -1)
>        return -1;

I had a second look at this patch this morning, and at how Python 3
memoryview works, and I think we should accept this.  Callers (even v1
callers) should be prepared for any buffer type object.  It also seems
to work with v2v.

Therefore ACK.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW