Re: [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)