[Libguestfs] [libnbd PATCH v3 3/5] python: Accept all buffer-like objects in aio_p{read, write}
Richard W.M. Jones
rjones at redhat.com
Thu Jun 9 14:31:02 UTC 2022
On Thu, Jun 09, 2022 at 08:34:45AM -0500, Eric Blake wrote:
> After the work in the previous patches, it is now a trivial feature
> addition to support any buffer-like object as the argument to
> h.aio_{pread,pwrite}, and matching how h.pwrite already did that. For
> example, you can do h.aio_pwrite(b'123', 0), instead of having to copy
> into nbd.Buffer first. More importantly, whereas
> nbd.Buffer.to_bytearray() currently copies out and
> nbd.Buffer.from_bytearray(buffer) copies in, using native types means
> you can reduce that copy overhead.
>
> For a demonstration of the speedups possible, compare two scripts that
> utilize a new buffer for every I/O (important if you are going to do
> parallel operations - even though this demo is serialized), one using
> nbd.Buffer, the other using native python types, with timing on my
> machine:
>
> $ export script1='
> m=1024*1024
> size=h.get_size()
> for i in range(size // m):
> b1 = nbd.Buffer(m)
> c = h.aio_pread(b1, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> b2 = nbd.Buffer.from_bytearray(b1.to_bytearray())
> c = h.aio_pwrite(b2, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> '
> $ export script2='
> m=1024*1024
> size=h.get_size()
> for i in range(size // m):
> b1 = bytearray(m)
> c = h.aio_pread(b1, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> b2 = bytes(b1)
> c = h.aio_pwrite(b2, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> '
>
> $ nbdkit -U - --filter=checkwrite pattern 10G \
> --run 'nbdsh -u "$uri" -c "h.set_pread_initialize(True)" -c "script1"'
Should that be "$script1" ? (and same below)
> takes ~31.5s (this setting for pread_initialize is default)
>
> $ nbdkit -U - --filter=checkwrite pattern 10G \
> --run 'nbdsh -u "$uri" -c "h.set_pread_initialize(False)" -c "script1"'
> takes ~30.8s (not re-zeroing the buffer during aio_pread helped)
>
> $ nbdkit -U - --filter=checkwrite pattern 10G \
> --run 'nbdsh -u "$uri" -c "h.set_pread_initialize(True)" -c "script2"'
> takes ~23.0s (less copying when going from b1 to b2 helped even more)
>
> $ nbdkit -U - --filter=checkwrite pattern 10G \
> --run 'nbdsh -u "$uri" -c "h.set_pread_initialize(False)" -c "script2"'
> takes ~22.8s (again, aio_pread is faster when not re-zeroing)
>
> Of course, you can get better speed still by reusing the same buffer
> for every operation, at which point the difference is in the noise;
> but this approach is no longer parallelizable as written (you don't
> want the same buffer in use by more than one aio operation at a time):
>
> $ export script3='
> m=1024*1024
> size=h.get_size()
> h.set_pread_initialize(False)
> buf=nbd.Buffer(m) # or bytearray(m)
> for i in range(size // m):
> c = h.aio_pread(buf, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> c = h.aio_pwrite(buf, m*i)
> while not h.aio_command_completed(c):
> h.poll(-1)
> '
>
> $ nbdkit -U - --filter=checkwrite pattern 10G \
> --run 'nbdsh -u "$uri" -c "script3"'
> takes about ~15.2s, regardless of whether I used nbd.Buffer(m) or
> bytearray(m) when setting up buf.
> ---
> generator/Python.ml | 8 ++++----
> python/handle.c | 19 +++++++++++++------
> python/t/500-aio-pread.py | 21 +++++++++++++++++++++
> python/t/510-aio-pwrite.py | 17 +++++++++++++++++
> 4 files changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/generator/Python.ml b/generator/Python.ml
> index 20d7e78..cd7f0e3 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -38,7 +38,7 @@ let
> extern void nbd_internal_py_free_string_list (char **);
> extern int nbd_internal_py_get_sockaddr (PyObject *,
> struct sockaddr_storage *, socklen_t *);
> -extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
> +extern PyObject *nbd_internal_py_get_aio_view (PyObject *, int);
> extern int nbd_internal_py_init_aio_buffer (PyObject *);
> extern PyObject *nbd_internal_py_get_nbd_buffer_type (void);
> extern PyObject *nbd_internal_py_wrap_errptr (int);
> @@ -288,7 +288,7 @@ let
> pr " Py_ssize_t %s;\n" count
> | BytesPersistIn (n, _)
> | BytesPersistOut (n, _) ->
> - pr " PyObject *%s; /* Instance of nbd.Buffer */\n" n;
> + pr " PyObject *%s; /* Buffer-like object or nbd.Buffer */\n" n;
> pr " PyObject *%s_view = NULL; /* PyMemoryView of %s */\n" n n;
> pr " Py_buffer *py_%s; /* buffer of %s_view */\n" n n
> | Closure { cbname } ->
> @@ -421,12 +421,12 @@ let
> pr " %s = PyByteArray_FromStringAndSize (NULL, %s);\n" n count;
> pr " if (%s == NULL) goto out;\n" n
> | BytesPersistIn (n, _) ->
> - pr " %s_view = nbd_internal_py_get_aio_view (%s, true);\n" n n;
> + pr " %s_view = nbd_internal_py_get_aio_view (%s, PyBUF_READ);\n" n n;
> pr " if (!%s_view) goto out;\n" n;
> pr " py_%s = PyMemoryView_GET_BUFFER (%s_view);\n" n n;
> pr " completion_user_data->view = %s_view;\n" n
> | BytesPersistOut (n, _) ->
> - pr " %s_view = nbd_internal_py_get_aio_view (%s, false);\n" n n;
> + pr " %s_view = nbd_internal_py_get_aio_view (%s, PyBUF_WRITE);\n" n n;
> pr " if (!%s_view) goto out;\n" n;
> pr " py_%s = PyMemoryView_GET_BUFFER (%s_view);\n" n n;
> pr " completion_user_data->view = %s_view;\n" n
> diff --git a/python/handle.c b/python/handle.c
> index ca6c8e9..f72d200 100644
> --- a/python/handle.c
> +++ b/python/handle.c
> @@ -96,16 +96,23 @@ nbd_internal_py_display_version (PyObject *self, PyObject *args)
> return Py_None;
> }
>
> -/* Return new reference to MemoryView wrapping aio_buffer contents */
> +/* Return new reference to MemoryView wrapping buffer or aio_buffer contents.
> + * buffertype is PyBUF_READ or PyBUF_WRITE, for how the buffer will be used
> + * (remember, aio_pwrite READS the buffer, aio_pread WRITES into the buffer).
> + */
> PyObject *
> -nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
> +nbd_internal_py_get_aio_view (PyObject *object, int buffertype)
> {
> PyObject *buffer = NULL;
>
> - if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) {
> + if (PyObject_CheckBuffer (object))
> + buffer = object;
> + else if (PyObject_IsInstance (object,
> + nbd_internal_py_get_nbd_buffer_type ())) {
> buffer = PyObject_GetAttrString (object, "_o");
>
> - if (require_init && ! PyObject_HasAttrString (object, "_init")) {
> + if (buffertype == PyBUF_READ &&
> + ! PyObject_HasAttrString (object, "_init")) {
> assert (PyByteArray_Check (buffer));
> memset (PyByteArray_AS_STRING (buffer), 0,
> PyByteArray_GET_SIZE (buffer));
> @@ -115,10 +122,10 @@ nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
> }
>
> if (buffer)
> - return PyMemoryView_FromObject (buffer);
> + return PyMemoryView_GetContiguous (buffer, buffertype, 'A');
>
> PyErr_SetString (PyExc_TypeError,
> - "aio_buffer: expecting nbd.Buffer instance");
> + "aio_buffer: expecting buffer or nbd.Buffer instance");
> return NULL;
> }
>
> diff --git a/python/t/500-aio-pread.py b/python/t/500-aio-pread.py
> index 781b709..6851928 100644
> --- a/python/t/500-aio-pread.py
> +++ b/python/t/500-aio-pread.py
> @@ -45,3 +45,24 @@ if sys.byteorder == 'little':
> arr.byteswap()
> assert buf1 == memoryview(arr).cast('B')[:512]
> assert buf2 == memoryview(arr).cast('B')[512:]
> +
> +# It is also possible to read directly into any writeable buffer-like object.
> +# However, aio.Buffer(n) with h.set_pread_initialize(False) may be faster,
> +# because it skips python's pre-initialization of bytearray(n).
> +try:
> + h.aio_pread(bytes(512), 0)
> + assert False
> +except BufferError:
> + pass
> +buf3 = bytearray(512)
> +cookie = h.aio_pread(buf3, 0)
> +# While the read is pending, the buffer cannot be resized
> +try:
> + buf3.pop()
> + assert False
> +except BufferError:
> + pass
> +while not h.aio_command_completed(cookie):
> + h.poll(-1)
> +buf3.append(buf3.pop())
> +assert buf3 == buf1
> diff --git a/python/t/510-aio-pwrite.py b/python/t/510-aio-pwrite.py
> index d09e249..4b4aac8 100644
> --- a/python/t/510-aio-pwrite.py
> +++ b/python/t/510-aio-pwrite.py
> @@ -67,4 +67,21 @@ with open(datafile, "rb") as f:
>
> assert nbd.Buffer.from_bytearray(content).is_zero()
>
> +# It is also possible to write any buffer-like object.
> +# While the write is pending, the buffer cannot be resized
> +cookie = h.aio_pwrite(buf, 0, flags=nbd.CMD_FLAG_FUA)
> +try:
> + buf.pop()
> + assert False
> +except BufferError:
> + pass
> +while not h.aio_command_completed(cookie):
> + h.poll(-1)
> +buf.append(buf.pop())
> +
> +with open(datafile, "rb") as f:
> + content = f.read()
> +
> +assert buf == content
> +
> os.unlink(datafile)
Acked-by: Richard W.M. Jones <rjones at redhat.com>
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit
More information about the Libguestfs
mailing list