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