Eric Blake
2022-Jun-09 13:34 UTC
[Libguestfs] [libnbd PATCH v3 3/5] python: Accept all buffer-like objects in aio_p{read, write}
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"' 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) -- 2.36.1
Richard W.M. Jones
2022-Jun-09 14:31 UTC
[Libguestfs] [libnbd PATCH v3 3/5] python: Accept all buffer-like objects in aio_p{read, write}
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