Eric Blake
2022-Jun-09 13:34 UTC
[Libguestfs] [libnbd PATCH v3 5/5] python: Slice structured read callback buffer from original
The Py_BuildValue "y#" format copies data; this is because Python
assumes our C memory can go out of scope, while the user's python
callback can stash off whatever bytearray object it got. But this
copying is inefficient. Now that we already have a Python buffer-like
object in scope for the duration of all structured reads
(pread_structured since commit f8c76c01, aio_pread_structured since
commit 4f15d0e9), we know that the C memory parameter to our
chunk_callback is already (a portion of) the underlying contiguous
memory of an original Python object. If we pass the Python callback a
memoryview object over the correct slice of that memory, then Python
will correctly handle the intricacies of what happens if the callback
saves our memoryview object for later use, all without any data
copying overhead, making use of the pread_structured form much faster.
We basically want to code up the C equivalent to the python expression
'view = memoryview(buf).toreadonly()[start:end]', where start and end
are determined by comparing subbuf/count against the original buf.
But in C, that involves creating PyObject integers to form a PySlice,
then using PyObject_GetItem to apply the slice to a PyMemoryView (I
don't know of any faster tricks; googling does not find any quick
hits). Extract this work into a new utils.c helper function, to avoid
repetition (and in case we figure out a way to do it in fewer lines by
directly manipulating Py_buffer objects).
The testsuite is updated to cover examples of the data persistence
aspect, demonstrating that our stashed slice is still tied to the
returned Python buffer, even though the callback is long since
completed (prior to this patch, stash was an independent bytes copy,
and while it persisted on after the callback, it is frozen in time to
the contents of the buffer at the time the callback was reached). The
fact that data is now shared is is an API change, but one unlikely to
cause problems - it is very unusual for a callback function to try and
stash sub-buffers for later use (neither libnbd nor nbdkit was
utilizing that in their testsuites before this patch).
To demonstrate the speedup now that there is less copying involved per
callback, I tested:
$ export script='
def f(b,o,s,e):
pass
m=1024*1024
size=h.get_size()
h.set_pread_initialize(False)
for i in range(size // m):
buf = h.pread_structured(m, m*i, f)
buf = None
'
$ time ./run nbdkit -U - memory 10G --run 'nbdsh -u "$uri" -c
"$script"'
On my machine, this took 16.9s with libnbd 1.12.3, 15.5s pre-patch,
and 3.6s post-patch, for more than 4x speedup in the best case. Much
of the differences boil down to how efficient the Python garbage
collector is able to spot liveness of various buffers, to reuse rather
than allocate new buffers on the Python side; removing the 'buf None'
line speeds up libnbd 1.12.3 to 4.5s (better reuse of the "y#"
memory in the callback when backed by C memory), 9.0s pre-patch
(copying "y#" from Python memory has scanning effects), and 3.0s
post-patch (zero-copying is always best).
A variation with aio_pread_structured reusing the same nbd.Buffer is
not quite as drastic: 3.9s with libnbd 1.12.3, 4.0s pre-patch, and
3.4s with this patch, but still shows that it is beneficial.
The corresponding diff to generated code is:
| --- python/methods.h.bak 2022-06-09 07:23:51.657260039 -0500
| +++ python/methods.h 2022-06-09 07:23:57.629266878 -0500
| @@ -36,6 +36,7 @@
| 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);
| +extern PyObject *nbd_internal_py_get_subview (PyObject *, const char *,
size_t);
|
| static inline struct nbd_handle *
| get_handle (PyObject *obj)
| --- python/methods.c.bak 2022-06-09 07:23:51.652260034 -0500
| +++ python/methods.c 2022-06-09 07:23:57.644266895 -0500
| @@ -73,13 +73,15 @@ chunk_wrapper (void *user_data, const vo
|
| PyGILState_STATE py_save = PyGILState_UNLOCKED;
| PyObject *py_args, *py_ret;
| + PyObject *py_subbuf = NULL;
| PyObject *py_error = NULL;
|
| + py_subbuf = nbd_internal_py_get_subview (data->view, subbuf, count);
| + if (!py_subbuf) { PyErr_PrintEx (0); goto out; }
| py_error = nbd_internal_py_wrap_errptr (*error);
| if (!py_error) { PyErr_PrintEx (0); goto out; }
|
| - py_args = Py_BuildValue ("(y#KIO)", subbuf, (int) count, offset,
status,
| - py_error);
| + py_args = Py_BuildValue ("(OKIO)", py_subbuf, offset, status,
py_error);
| if (!py_args) { PyErr_PrintEx (0); goto out; }
|
| py_save = PyGILState_Ensure ();
| @@ -106,6 +108,7 @@ chunk_wrapper (void *user_data, const vo
| };
|
| out:
| + Py_XDECREF (py_subbuf);
| if (py_error) {
| PyObject *py_error_ret = PyObject_GetAttrString (py_error,
"value");
| *error = PyLong_AsLong (py_error_ret);
| @@ -2231,6 +2234,7 @@ nbd_internal_py_pread_structured (PyObje
| /* Increment refcount since pointer may be saved by libnbd. */
| Py_INCREF (py_chunk_fn);
| chunk_user_data->fn = py_chunk_fn;
| + chunk_user_data->view = nbd_internal_py_get_aio_view (buf, PyBUF_WRITE);
|
| ret = nbd_pread_structured (h, PyByteArray_AS_STRING (buf), count,
| offset_u64, chunk, flags_u32);
| @@ -3116,6 +3120,7 @@ nbd_internal_py_aio_pread_structured (Py
| /* Increment refcount since pointer may be saved by libnbd. */
| Py_INCREF (py_chunk_fn);
| chunk_user_data->fn = py_chunk_fn;
| + chunk_user_data->view = nbd_internal_py_get_aio_view (buf, PyBUF_WRITE);
|
| if (nbd_internal_py_init_aio_buffer (buf) < 0) goto out;
| ret = nbd_aio_pread_structured (h, py_buf->buf, py_buf->len,
offset_u64,
---
generator/Python.ml | 18 ++++--
python/t/405-pread-structured.py | 95 +++++++++---------------------
python/t/505-aio-pread-callback.py | 93 ++++++++---------------------
python/utils.c | 34 +++++++++++
4 files changed, 99 insertions(+), 141 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index 975cab4..c424239 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -42,6 +42,7 @@ let
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);
+extern PyObject *nbd_internal_py_get_subview (PyObject *, const char *,
size_t);
static inline struct nbd_handle *
get_handle (PyObject *obj)
@@ -163,8 +164,8 @@ let
pr " PyObject *py_args, *py_ret;\n";
List.iter (
function
- | CBArrayAndLen (UInt32 n, _) ->
- pr " PyObject *py_%s = NULL;\n" n
+ | CBArrayAndLen (UInt32 n, _)
+ | CBBytesIn (n, _)
| CBMutable (Int n) ->
pr " PyObject *py_%s = NULL;\n" n
| _ -> ()
@@ -181,7 +182,9 @@ let
pr " if (!py_e_%s) { PyErr_PrintEx (0); goto out; }\n" n;
pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n;
pr " }\n"
- | CBBytesIn _
+ | CBBytesIn (n, len) ->
+ pr " py_%s = nbd_internal_py_get_subview (data->view, %s,
%s);\n" n n len;
+ pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n
| CBInt _
| CBInt64 _ -> ()
| CBMutable (Int n) ->
@@ -198,7 +201,7 @@ let
List.map (
function
| CBArrayAndLen (UInt32 n, _) -> "O", sprintf
"py_%s" n
- | CBBytesIn (n, len) -> "y#", sprintf "%s, (int)
%s" n len
+ | CBBytesIn (n, _) -> "O", sprintf "py_%s" n
| CBInt n -> "i", n
| CBInt64 n -> "L", n
| CBMutable (Int n) -> "O", sprintf "py_%s" n
@@ -250,6 +253,8 @@ let
function
| CBArrayAndLen (UInt32 n, _) ->
pr " Py_XDECREF (py_%s);\n" n
+ | CBBytesIn (n, _) ->
+ pr " Py_XDECREF (py_%s);\n" n
| CBMutable (Int n) ->
pr " if (py_%s) {\n" n;
pr " PyObject *py_%s_ret = PyObject_GetAttrString (py_%s,
\"value\");\n" n n;
@@ -257,7 +262,6 @@ let
pr " Py_DECREF (py_%s_ret);\n" n;
pr " Py_DECREF (py_%s);\n" n;
pr " }\n"
- | CBBytesIn _
| CBInt _ | CBInt64 _
| CBString _
| CBUInt _ | CBUInt64 _ -> ()
@@ -440,7 +444,9 @@ let
pr " }\n";
pr " /* Increment refcount since pointer may be saved by libnbd.
*/\n";
pr " Py_INCREF (py_%s_fn);\n" cbname;
- pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname
+ pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname;
+ if cbname = "chunk" then
+ pr " chunk_user_data->view = nbd_internal_py_get_aio_view
(buf, PyBUF_WRITE);\n"
| Enum _ -> ()
| Flags (n, _) -> pr " %s_u32 = %s;\n" n n
| Fd _ | Int _ -> ()
diff --git a/python/t/405-pread-structured.py b/python/t/405-pread-structured.py
index afd1e7e..9d3df68 100644
--- a/python/t/405-pread-structured.py
+++ b/python/t/405-pread-structured.py
@@ -1,5 +1,5 @@
# libnbd Python bindings
-# Copyright (C) 2010-2019 Red Hat Inc.
+# Copyright (C) 2010-2022 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -17,76 +17,19 @@
import nbd
import errno
+import sys
+from array import array
h = nbd.NBD()
h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
"pattern", "size=512"])
-expected = (b'\x00\x00\x00\x00\x00\x00\x00\x00'
- + b'\x00\x00\x00\x00\x00\x00\x00\x08'
- + b'\x00\x00\x00\x00\x00\x00\x00\x10'
- + b'\x00\x00\x00\x00\x00\x00\x00\x18'
- + b'\x00\x00\x00\x00\x00\x00\x00 '
- + b'\x00\x00\x00\x00\x00\x00\x00('
- + b'\x00\x00\x00\x00\x00\x00\x000'
- + b'\x00\x00\x00\x00\x00\x00\x008'
- + b'\x00\x00\x00\x00\x00\x00\x00@'
- + b'\x00\x00\x00\x00\x00\x00\x00H'
- + b'\x00\x00\x00\x00\x00\x00\x00P'
- + b'\x00\x00\x00\x00\x00\x00\x00X'
- + b'\x00\x00\x00\x00\x00\x00\x00`'
- + b'\x00\x00\x00\x00\x00\x00\x00h'
- + b'\x00\x00\x00\x00\x00\x00\x00p'
- + b'\x00\x00\x00\x00\x00\x00\x00x'
- + b'\x00\x00\x00\x00\x00\x00\x00\x80'
- + b'\x00\x00\x00\x00\x00\x00\x00\x88'
- + b'\x00\x00\x00\x00\x00\x00\x00\x90'
- + b'\x00\x00\x00\x00\x00\x00\x00\x98'
- + b'\x00\x00\x00\x00\x00\x00\x00\xa0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xa8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xb0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xb8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xc0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xc8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xd0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xd8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xe0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xe8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xf0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xf8'
- + b'\x00\x00\x00\x00\x00\x00\x01\x00'
- + b'\x00\x00\x00\x00\x00\x00\x01\x08'
- + b'\x00\x00\x00\x00\x00\x00\x01\x10'
- + b'\x00\x00\x00\x00\x00\x00\x01\x18'
- + b'\x00\x00\x00\x00\x00\x00\x01 '
- + b'\x00\x00\x00\x00\x00\x00\x01('
- + b'\x00\x00\x00\x00\x00\x00\x010'
- + b'\x00\x00\x00\x00\x00\x00\x018'
- + b'\x00\x00\x00\x00\x00\x00\x01@'
- + b'\x00\x00\x00\x00\x00\x00\x01H'
- + b'\x00\x00\x00\x00\x00\x00\x01P'
- + b'\x00\x00\x00\x00\x00\x00\x01X'
- + b'\x00\x00\x00\x00\x00\x00\x01`'
- + b'\x00\x00\x00\x00\x00\x00\x01h'
- + b'\x00\x00\x00\x00\x00\x00\x01p'
- + b'\x00\x00\x00\x00\x00\x00\x01x'
- + b'\x00\x00\x00\x00\x00\x00\x01\x80'
- + b'\x00\x00\x00\x00\x00\x00\x01\x88'
- + b'\x00\x00\x00\x00\x00\x00\x01\x90'
- + b'\x00\x00\x00\x00\x00\x00\x01\x98'
- + b'\x00\x00\x00\x00\x00\x00\x01\xa0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xa8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xb0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xb8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xc0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xc8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xd0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xd8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xe0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xe8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xf0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xf8')
-
+# The nbdkit pattern plugin exposes 64-bit numbers in bigendian order
+arr = array('q', range(0, 512, 8))
+if sys.byteorder == 'little':
+ arr.byteswap()
+expected = memoryview(arr).cast('B')
+stash = None
def f(user_data, buf2, offset, s, err):
assert err.value == 0
@@ -94,8 +37,15 @@ def f(user_data, buf2, offset, s, err):
if user_data != 42:
raise ValueError('unexpected user_data')
assert buf2 == expected
+ try:
+ buf2[0] = 1
+ assert False
+ except TypeError:
+ pass
assert offset == 0
assert s == nbd.READ_DATA
+ global stash
+ stash = buf2
buf = h.pread_structured(512, 0, lambda *args: f(42, *args))
@@ -104,6 +54,19 @@ print("%r" % buf)
assert buf == expected
+# The callback can stash its slice; as long as that is live, we can't
+# resize buf but can view changes in buf through the slice
+try:
+ buf.pop()
+ assert False
+except BufferError:
+ pass
+buf[0] ^= 1
+assert buf == stash
+stash = None
+buf.pop()
+
+# Tests of error handling
buf = h.pread_structured(512, 0, lambda *args: f(42, *args),
nbd.CMD_FLAG_DF)
diff --git a/python/t/505-aio-pread-callback.py
b/python/t/505-aio-pread-callback.py
index 1773aee..5c7b250 100644
--- a/python/t/505-aio-pread-callback.py
+++ b/python/t/505-aio-pread-callback.py
@@ -1,5 +1,5 @@
# libnbd Python bindings
-# Copyright (C) 2010-2019 Red Hat Inc.
+# Copyright (C) 2010-2022 Red Hat Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -17,76 +17,19 @@
import nbd
import errno
+import sys
+from array import array
h = nbd.NBD()
h.connect_command(["nbdkit", "-s",
"--exit-with-parent", "-v",
"pattern", "size=512"])
-expected = (b'\x00\x00\x00\x00\x00\x00\x00\x00'
- + b'\x00\x00\x00\x00\x00\x00\x00\x08'
- + b'\x00\x00\x00\x00\x00\x00\x00\x10'
- + b'\x00\x00\x00\x00\x00\x00\x00\x18'
- + b'\x00\x00\x00\x00\x00\x00\x00 '
- + b'\x00\x00\x00\x00\x00\x00\x00('
- + b'\x00\x00\x00\x00\x00\x00\x000'
- + b'\x00\x00\x00\x00\x00\x00\x008'
- + b'\x00\x00\x00\x00\x00\x00\x00@'
- + b'\x00\x00\x00\x00\x00\x00\x00H'
- + b'\x00\x00\x00\x00\x00\x00\x00P'
- + b'\x00\x00\x00\x00\x00\x00\x00X'
- + b'\x00\x00\x00\x00\x00\x00\x00`'
- + b'\x00\x00\x00\x00\x00\x00\x00h'
- + b'\x00\x00\x00\x00\x00\x00\x00p'
- + b'\x00\x00\x00\x00\x00\x00\x00x'
- + b'\x00\x00\x00\x00\x00\x00\x00\x80'
- + b'\x00\x00\x00\x00\x00\x00\x00\x88'
- + b'\x00\x00\x00\x00\x00\x00\x00\x90'
- + b'\x00\x00\x00\x00\x00\x00\x00\x98'
- + b'\x00\x00\x00\x00\x00\x00\x00\xa0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xa8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xb0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xb8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xc0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xc8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xd0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xd8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xe0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xe8'
- + b'\x00\x00\x00\x00\x00\x00\x00\xf0'
- + b'\x00\x00\x00\x00\x00\x00\x00\xf8'
- + b'\x00\x00\x00\x00\x00\x00\x01\x00'
- + b'\x00\x00\x00\x00\x00\x00\x01\x08'
- + b'\x00\x00\x00\x00\x00\x00\x01\x10'
- + b'\x00\x00\x00\x00\x00\x00\x01\x18'
- + b'\x00\x00\x00\x00\x00\x00\x01 '
- + b'\x00\x00\x00\x00\x00\x00\x01('
- + b'\x00\x00\x00\x00\x00\x00\x010'
- + b'\x00\x00\x00\x00\x00\x00\x018'
- + b'\x00\x00\x00\x00\x00\x00\x01@'
- + b'\x00\x00\x00\x00\x00\x00\x01H'
- + b'\x00\x00\x00\x00\x00\x00\x01P'
- + b'\x00\x00\x00\x00\x00\x00\x01X'
- + b'\x00\x00\x00\x00\x00\x00\x01`'
- + b'\x00\x00\x00\x00\x00\x00\x01h'
- + b'\x00\x00\x00\x00\x00\x00\x01p'
- + b'\x00\x00\x00\x00\x00\x00\x01x'
- + b'\x00\x00\x00\x00\x00\x00\x01\x80'
- + b'\x00\x00\x00\x00\x00\x00\x01\x88'
- + b'\x00\x00\x00\x00\x00\x00\x01\x90'
- + b'\x00\x00\x00\x00\x00\x00\x01\x98'
- + b'\x00\x00\x00\x00\x00\x00\x01\xa0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xa8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xb0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xb8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xc0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xc8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xd0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xd8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xe0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xe8'
- + b'\x00\x00\x00\x00\x00\x00\x01\xf0'
- + b'\x00\x00\x00\x00\x00\x00\x01\xf8')
-
+# The nbdkit pattern plugin exposes 64-bit numbers in bigendian order
+arr = array('q', range(0, 512, 8))
+if sys.byteorder == 'little':
+ arr.byteswap()
+expected = memoryview(arr).cast('B')
+stash = None
def chunk(user_data, buf2, offset, s, err):
print("in chunk, user_data %d" % user_data)
@@ -97,6 +40,8 @@ def chunk(user_data, buf2, offset, s, err):
assert buf2 == expected
assert offset == 0
assert s == nbd.READ_DATA
+ global stash
+ stash = buf2
def callback(user_data, err):
@@ -111,19 +56,29 @@ def callback(user_data, err):
# First try: succeed in both callbacks
-buf = nbd.Buffer(512)
+buf = bytearray(512)
cookie = h.aio_pread_structured(buf, 0,
lambda *args: chunk(42, *args),
lambda *args: callback((42, 42), *args))
while not h.aio_command_completed(cookie):
h.poll(-1)
-buf = buf.to_bytearray()
-
print("%r" % buf)
assert buf == expected
+# The callback can stash its slice; as long as that is live, we can't
+# resize buf but can view changes in buf through the slice
+try:
+ buf.pop()
+ assert False
+except BufferError:
+ pass
+buf[0] ^= 1
+assert buf == stash
+stash = None
+buf.pop()
+
# Second try: fail only during callback
buf = nbd.Buffer(512)
cookie = h.aio_pread_structured(buf, 0,
diff --git a/python/utils.c b/python/utils.c
index cd44b90..d5851f7 100644
--- a/python/utils.c
+++ b/python/utils.c
@@ -192,3 +192,37 @@ nbd_internal_py_wrap_errptr (int err)
return PyObject_CallMethod (py_ctypes_mod, "c_int", "i",
err);
}
+
+/* Helper to compute view.toreadonly()[start:end] in chunk callback */
+PyObject *
+nbd_internal_py_get_subview (PyObject *view, const char *subbuf, size_t count)
+{
+ Py_buffer *orig;
+ const char *base;
+ PyObject *start, *end, *slice;
+ PyObject *ret;
+
+ assert (PyMemoryView_Check (view));
+ orig = PyMemoryView_GET_BUFFER (view);
+ assert (PyBuffer_IsContiguous (orig, 'A'));
+ base = orig->buf;
+ assert (subbuf >= base && count <= orig->len &&
+ subbuf + count <= base + orig->len);
+ start = PyLong_FromLong (subbuf - base);
+ if (!start) return NULL;
+ end = PyLong_FromLong (subbuf - base + count);
+ if (!end) { Py_DECREF (start); return NULL; }
+ slice = PySlice_New (start, end, NULL);
+ Py_DECREF (start);
+ Py_DECREF (end);
+ if (!slice) return NULL;
+ ret = PyObject_GetItem (view, slice);
+ Py_DECREF (slice);
+ /* memoryview.toreadonly() was only added in Python 3.8.
+ * PyMemoryView_GetContiguous (ret, PyBuf_READ, 'A') doesn't
force readonly.
+ * So we mess around directly with the Py_buffer.
+ */
+ if (ret)
+ PyMemoryView_GET_BUFFER (ret)->readonly = 1;
+ return ret;
+}
--
2.36.1
Richard W.M. Jones
2022-Jun-09 14:34 UTC
[Libguestfs] [libnbd PATCH v3 5/5] python: Slice structured read callback buffer from original
On Thu, Jun 09, 2022 at 08:34:47AM -0500, Eric Blake wrote:> The Py_BuildValue "y#" format copies data; this is because Python > assumes our C memory can go out of scope, while the user's python > callback can stash off whatever bytearray object it got. But this > copying is inefficient. Now that we already have a Python buffer-like > object in scope for the duration of all structured reads > (pread_structured since commit f8c76c01, aio_pread_structured since > commit 4f15d0e9), we know that the C memory parameter to our > chunk_callback is already (a portion of) the underlying contiguous > memory of an original Python object. If we pass the Python callback a > memoryview object over the correct slice of that memory, then Python > will correctly handle the intricacies of what happens if the callback > saves our memoryview object for later use, all without any data > copying overhead, making use of the pread_structured form much faster. > > We basically want to code up the C equivalent to the python expression > 'view = memoryview(buf).toreadonly()[start:end]', where start and end > are determined by comparing subbuf/count against the original buf. > But in C, that involves creating PyObject integers to form a PySlice, > then using PyObject_GetItem to apply the slice to a PyMemoryView (I > don't know of any faster tricks; googling does not find any quick > hits). Extract this work into a new utils.c helper function, to avoid > repetition (and in case we figure out a way to do it in fewer lines by > directly manipulating Py_buffer objects). > > The testsuite is updated to cover examples of the data persistence > aspect, demonstrating that our stashed slice is still tied to the > returned Python buffer, even though the callback is long since > completed (prior to this patch, stash was an independent bytes copy, > and while it persisted on after the callback, it is frozen in time to > the contents of the buffer at the time the callback was reached). The > fact that data is now shared is is an API change, but one unlikely to > cause problems - it is very unusual for a callback function to try and > stash sub-buffers for later use (neither libnbd nor nbdkit was > utilizing that in their testsuites before this patch). > > To demonstrate the speedup now that there is less copying involved per > callback, I tested: > > $ export script=' > def f(b,o,s,e): > pass > m=1024*1024 > size=h.get_size() > h.set_pread_initialize(False) > for i in range(size // m): > buf = h.pread_structured(m, m*i, f) > buf = None > ' > $ time ./run nbdkit -U - memory 10G --run 'nbdsh -u "$uri" -c "$script"' > > On my machine, this took 16.9s with libnbd 1.12.3, 15.5s pre-patch, > and 3.6s post-patch, for more than 4x speedup in the best case. Much > of the differences boil down to how efficient the Python garbage > collector is able to spot liveness of various buffers, to reuse rather > than allocate new buffers on the Python side; removing the 'buf > None' line speeds up libnbd 1.12.3 to 4.5s (better reuse of the "y#" > memory in the callback when backed by C memory), 9.0s pre-patch > (copying "y#" from Python memory has scanning effects), and 3.0s > post-patch (zero-copying is always best). > > A variation with aio_pread_structured reusing the same nbd.Buffer is > not quite as drastic: 3.9s with libnbd 1.12.3, 4.0s pre-patch, and > 3.4s with this patch, but still shows that it is beneficial. > > The corresponding diff to generated code is: > > | --- python/methods.h.bak 2022-06-09 07:23:51.657260039 -0500 > | +++ python/methods.h 2022-06-09 07:23:57.629266878 -0500 > | @@ -36,6 +36,7 @@ > | 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); > | +extern PyObject *nbd_internal_py_get_subview (PyObject *, const char *, size_t); > | > | static inline struct nbd_handle * > | get_handle (PyObject *obj) > | --- python/methods.c.bak 2022-06-09 07:23:51.652260034 -0500 > | +++ python/methods.c 2022-06-09 07:23:57.644266895 -0500 > | @@ -73,13 +73,15 @@ chunk_wrapper (void *user_data, const vo > | > | PyGILState_STATE py_save = PyGILState_UNLOCKED; > | PyObject *py_args, *py_ret; > | + PyObject *py_subbuf = NULL; > | PyObject *py_error = NULL; > | > | + py_subbuf = nbd_internal_py_get_subview (data->view, subbuf, count); > | + if (!py_subbuf) { PyErr_PrintEx (0); goto out; } > | py_error = nbd_internal_py_wrap_errptr (*error); > | if (!py_error) { PyErr_PrintEx (0); goto out; } > | > | - py_args = Py_BuildValue ("(y#KIO)", subbuf, (int) count, offset, status, > | - py_error); > | + py_args = Py_BuildValue ("(OKIO)", py_subbuf, offset, status, py_error); > | if (!py_args) { PyErr_PrintEx (0); goto out; } > | > | py_save = PyGILState_Ensure (); > | @@ -106,6 +108,7 @@ chunk_wrapper (void *user_data, const vo > | }; > | > | out: > | + Py_XDECREF (py_subbuf); > | if (py_error) { > | PyObject *py_error_ret = PyObject_GetAttrString (py_error, "value"); > | *error = PyLong_AsLong (py_error_ret); > | @@ -2231,6 +2234,7 @@ nbd_internal_py_pread_structured (PyObje > | /* Increment refcount since pointer may be saved by libnbd. */ > | Py_INCREF (py_chunk_fn); > | chunk_user_data->fn = py_chunk_fn; > | + chunk_user_data->view = nbd_internal_py_get_aio_view (buf, PyBUF_WRITE); > | > | ret = nbd_pread_structured (h, PyByteArray_AS_STRING (buf), count, > | offset_u64, chunk, flags_u32); > | @@ -3116,6 +3120,7 @@ nbd_internal_py_aio_pread_structured (Py > | /* Increment refcount since pointer may be saved by libnbd. */ > | Py_INCREF (py_chunk_fn); > | chunk_user_data->fn = py_chunk_fn; > | + chunk_user_data->view = nbd_internal_py_get_aio_view (buf, PyBUF_WRITE); > | > | if (nbd_internal_py_init_aio_buffer (buf) < 0) goto out; > | ret = nbd_aio_pread_structured (h, py_buf->buf, py_buf->len, offset_u64, > --- > generator/Python.ml | 18 ++++-- > python/t/405-pread-structured.py | 95 +++++++++--------------------- > python/t/505-aio-pread-callback.py | 93 ++++++++--------------------- > python/utils.c | 34 +++++++++++ > 4 files changed, 99 insertions(+), 141 deletions(-) > > diff --git a/generator/Python.ml b/generator/Python.ml > index 975cab4..c424239 100644 > --- a/generator/Python.ml > +++ b/generator/Python.ml > @@ -42,6 +42,7 @@ let > 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); > +extern PyObject *nbd_internal_py_get_subview (PyObject *, const char *, size_t); > > static inline struct nbd_handle * > get_handle (PyObject *obj) > @@ -163,8 +164,8 @@ let > pr " PyObject *py_args, *py_ret;\n"; > List.iter ( > function > - | CBArrayAndLen (UInt32 n, _) -> > - pr " PyObject *py_%s = NULL;\n" n > + | CBArrayAndLen (UInt32 n, _) > + | CBBytesIn (n, _) > | CBMutable (Int n) -> > pr " PyObject *py_%s = NULL;\n" n > | _ -> () > @@ -181,7 +182,9 @@ let > pr " if (!py_e_%s) { PyErr_PrintEx (0); goto out; }\n" n; > pr " PyList_SET_ITEM (py_%s, i_%s, py_e_%s);\n" n n n; > pr " }\n" > - | CBBytesIn _ > + | CBBytesIn (n, len) -> > + pr " py_%s = nbd_internal_py_get_subview (data->view, %s, %s);\n" n n len; > + pr " if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n > | CBInt _ > | CBInt64 _ -> () > | CBMutable (Int n) -> > @@ -198,7 +201,7 @@ let > List.map ( > function > | CBArrayAndLen (UInt32 n, _) -> "O", sprintf "py_%s" n > - | CBBytesIn (n, len) -> "y#", sprintf "%s, (int) %s" n len > + | CBBytesIn (n, _) -> "O", sprintf "py_%s" n > | CBInt n -> "i", n > | CBInt64 n -> "L", n > | CBMutable (Int n) -> "O", sprintf "py_%s" n > @@ -250,6 +253,8 @@ let > function > | CBArrayAndLen (UInt32 n, _) -> > pr " Py_XDECREF (py_%s);\n" n > + | CBBytesIn (n, _) -> > + pr " Py_XDECREF (py_%s);\n" n > | CBMutable (Int n) -> > pr " if (py_%s) {\n" n; > pr " PyObject *py_%s_ret = PyObject_GetAttrString (py_%s, \"value\");\n" n n; > @@ -257,7 +262,6 @@ let > pr " Py_DECREF (py_%s_ret);\n" n; > pr " Py_DECREF (py_%s);\n" n; > pr " }\n" > - | CBBytesIn _ > | CBInt _ | CBInt64 _ > | CBString _ > | CBUInt _ | CBUInt64 _ -> () > @@ -440,7 +444,9 @@ let > pr " }\n"; > pr " /* Increment refcount since pointer may be saved by libnbd. */\n"; > pr " Py_INCREF (py_%s_fn);\n" cbname; > - pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname > + pr " %s_user_data->fn = py_%s_fn;\n" cbname cbname; > + if cbname = "chunk" then > + pr " chunk_user_data->view = nbd_internal_py_get_aio_view (buf, PyBUF_WRITE);\n" > | Enum _ -> () > | Flags (n, _) -> pr " %s_u32 = %s;\n" n n > | Fd _ | Int _ -> () > diff --git a/python/t/405-pread-structured.py b/python/t/405-pread-structured.py > index afd1e7e..9d3df68 100644 > --- a/python/t/405-pread-structured.py > +++ b/python/t/405-pread-structured.py > @@ -1,5 +1,5 @@ > # libnbd Python bindings > -# Copyright (C) 2010-2019 Red Hat Inc. > +# Copyright (C) 2010-2022 Red Hat Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -17,76 +17,19 @@ > > import nbd > import errno > +import sys > +from array import array > > h = nbd.NBD() > h.connect_command(["nbdkit", "-s", "--exit-with-parent", "-v", > "pattern", "size=512"]) > > -expected = (b'\x00\x00\x00\x00\x00\x00\x00\x00' > - + b'\x00\x00\x00\x00\x00\x00\x00\x08' > - + b'\x00\x00\x00\x00\x00\x00\x00\x10' > - + b'\x00\x00\x00\x00\x00\x00\x00\x18' > - + b'\x00\x00\x00\x00\x00\x00\x00 ' > - + b'\x00\x00\x00\x00\x00\x00\x00(' > - + b'\x00\x00\x00\x00\x00\x00\x000' > - + b'\x00\x00\x00\x00\x00\x00\x008' > - + b'\x00\x00\x00\x00\x00\x00\x00@' > - + b'\x00\x00\x00\x00\x00\x00\x00H' > - + b'\x00\x00\x00\x00\x00\x00\x00P' > - + b'\x00\x00\x00\x00\x00\x00\x00X' > - + b'\x00\x00\x00\x00\x00\x00\x00`' > - + b'\x00\x00\x00\x00\x00\x00\x00h' > - + b'\x00\x00\x00\x00\x00\x00\x00p' > - + b'\x00\x00\x00\x00\x00\x00\x00x' > - + b'\x00\x00\x00\x00\x00\x00\x00\x80' > - + b'\x00\x00\x00\x00\x00\x00\x00\x88' > - + b'\x00\x00\x00\x00\x00\x00\x00\x90' > - + b'\x00\x00\x00\x00\x00\x00\x00\x98' > - + b'\x00\x00\x00\x00\x00\x00\x00\xa0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xa8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xb0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xb8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xc0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xc8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xd0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xd8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xe0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xe8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xf0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xf8' > - + b'\x00\x00\x00\x00\x00\x00\x01\x00' > - + b'\x00\x00\x00\x00\x00\x00\x01\x08' > - + b'\x00\x00\x00\x00\x00\x00\x01\x10' > - + b'\x00\x00\x00\x00\x00\x00\x01\x18' > - + b'\x00\x00\x00\x00\x00\x00\x01 ' > - + b'\x00\x00\x00\x00\x00\x00\x01(' > - + b'\x00\x00\x00\x00\x00\x00\x010' > - + b'\x00\x00\x00\x00\x00\x00\x018' > - + b'\x00\x00\x00\x00\x00\x00\x01@' > - + b'\x00\x00\x00\x00\x00\x00\x01H' > - + b'\x00\x00\x00\x00\x00\x00\x01P' > - + b'\x00\x00\x00\x00\x00\x00\x01X' > - + b'\x00\x00\x00\x00\x00\x00\x01`' > - + b'\x00\x00\x00\x00\x00\x00\x01h' > - + b'\x00\x00\x00\x00\x00\x00\x01p' > - + b'\x00\x00\x00\x00\x00\x00\x01x' > - + b'\x00\x00\x00\x00\x00\x00\x01\x80' > - + b'\x00\x00\x00\x00\x00\x00\x01\x88' > - + b'\x00\x00\x00\x00\x00\x00\x01\x90' > - + b'\x00\x00\x00\x00\x00\x00\x01\x98' > - + b'\x00\x00\x00\x00\x00\x00\x01\xa0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xa8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xb0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xb8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xc0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xc8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xd0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xd8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xe0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xe8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xf0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xf8') > - > +# The nbdkit pattern plugin exposes 64-bit numbers in bigendian order > +arr = array('q', range(0, 512, 8)) > +if sys.byteorder == 'little': > + arr.byteswap() > +expected = memoryview(arr).cast('B') > +stash = None > > def f(user_data, buf2, offset, s, err): > assert err.value == 0 > @@ -94,8 +37,15 @@ def f(user_data, buf2, offset, s, err): > if user_data != 42: > raise ValueError('unexpected user_data') > assert buf2 == expected > + try: > + buf2[0] = 1 > + assert False > + except TypeError: > + pass > assert offset == 0 > assert s == nbd.READ_DATA > + global stash > + stash = buf2 > > > buf = h.pread_structured(512, 0, lambda *args: f(42, *args)) > @@ -104,6 +54,19 @@ print("%r" % buf) > > assert buf == expected > > +# The callback can stash its slice; as long as that is live, we can't > +# resize buf but can view changes in buf through the slice > +try: > + buf.pop() > + assert False > +except BufferError: > + pass > +buf[0] ^= 1 > +assert buf == stash > +stash = None > +buf.pop() > + > +# Tests of error handling > buf = h.pread_structured(512, 0, lambda *args: f(42, *args), > nbd.CMD_FLAG_DF) > > diff --git a/python/t/505-aio-pread-callback.py b/python/t/505-aio-pread-callback.py > index 1773aee..5c7b250 100644 > --- a/python/t/505-aio-pread-callback.py > +++ b/python/t/505-aio-pread-callback.py > @@ -1,5 +1,5 @@ > # libnbd Python bindings > -# Copyright (C) 2010-2019 Red Hat Inc. > +# Copyright (C) 2010-2022 Red Hat Inc. > # > # This program is free software; you can redistribute it and/or modify > # it under the terms of the GNU General Public License as published by > @@ -17,76 +17,19 @@ > > import nbd > import errno > +import sys > +from array import array > > h = nbd.NBD() > h.connect_command(["nbdkit", "-s", "--exit-with-parent", "-v", > "pattern", "size=512"]) > > -expected = (b'\x00\x00\x00\x00\x00\x00\x00\x00' > - + b'\x00\x00\x00\x00\x00\x00\x00\x08' > - + b'\x00\x00\x00\x00\x00\x00\x00\x10' > - + b'\x00\x00\x00\x00\x00\x00\x00\x18' > - + b'\x00\x00\x00\x00\x00\x00\x00 ' > - + b'\x00\x00\x00\x00\x00\x00\x00(' > - + b'\x00\x00\x00\x00\x00\x00\x000' > - + b'\x00\x00\x00\x00\x00\x00\x008' > - + b'\x00\x00\x00\x00\x00\x00\x00@' > - + b'\x00\x00\x00\x00\x00\x00\x00H' > - + b'\x00\x00\x00\x00\x00\x00\x00P' > - + b'\x00\x00\x00\x00\x00\x00\x00X' > - + b'\x00\x00\x00\x00\x00\x00\x00`' > - + b'\x00\x00\x00\x00\x00\x00\x00h' > - + b'\x00\x00\x00\x00\x00\x00\x00p' > - + b'\x00\x00\x00\x00\x00\x00\x00x' > - + b'\x00\x00\x00\x00\x00\x00\x00\x80' > - + b'\x00\x00\x00\x00\x00\x00\x00\x88' > - + b'\x00\x00\x00\x00\x00\x00\x00\x90' > - + b'\x00\x00\x00\x00\x00\x00\x00\x98' > - + b'\x00\x00\x00\x00\x00\x00\x00\xa0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xa8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xb0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xb8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xc0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xc8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xd0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xd8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xe0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xe8' > - + b'\x00\x00\x00\x00\x00\x00\x00\xf0' > - + b'\x00\x00\x00\x00\x00\x00\x00\xf8' > - + b'\x00\x00\x00\x00\x00\x00\x01\x00' > - + b'\x00\x00\x00\x00\x00\x00\x01\x08' > - + b'\x00\x00\x00\x00\x00\x00\x01\x10' > - + b'\x00\x00\x00\x00\x00\x00\x01\x18' > - + b'\x00\x00\x00\x00\x00\x00\x01 ' > - + b'\x00\x00\x00\x00\x00\x00\x01(' > - + b'\x00\x00\x00\x00\x00\x00\x010' > - + b'\x00\x00\x00\x00\x00\x00\x018' > - + b'\x00\x00\x00\x00\x00\x00\x01@' > - + b'\x00\x00\x00\x00\x00\x00\x01H' > - + b'\x00\x00\x00\x00\x00\x00\x01P' > - + b'\x00\x00\x00\x00\x00\x00\x01X' > - + b'\x00\x00\x00\x00\x00\x00\x01`' > - + b'\x00\x00\x00\x00\x00\x00\x01h' > - + b'\x00\x00\x00\x00\x00\x00\x01p' > - + b'\x00\x00\x00\x00\x00\x00\x01x' > - + b'\x00\x00\x00\x00\x00\x00\x01\x80' > - + b'\x00\x00\x00\x00\x00\x00\x01\x88' > - + b'\x00\x00\x00\x00\x00\x00\x01\x90' > - + b'\x00\x00\x00\x00\x00\x00\x01\x98' > - + b'\x00\x00\x00\x00\x00\x00\x01\xa0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xa8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xb0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xb8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xc0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xc8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xd0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xd8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xe0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xe8' > - + b'\x00\x00\x00\x00\x00\x00\x01\xf0' > - + b'\x00\x00\x00\x00\x00\x00\x01\xf8') > - > +# The nbdkit pattern plugin exposes 64-bit numbers in bigendian order > +arr = array('q', range(0, 512, 8)) > +if sys.byteorder == 'little': > + arr.byteswap() > +expected = memoryview(arr).cast('B') > +stash = None > > def chunk(user_data, buf2, offset, s, err): > print("in chunk, user_data %d" % user_data) > @@ -97,6 +40,8 @@ def chunk(user_data, buf2, offset, s, err): > assert buf2 == expected > assert offset == 0 > assert s == nbd.READ_DATA > + global stash > + stash = buf2 > > > def callback(user_data, err): > @@ -111,19 +56,29 @@ def callback(user_data, err): > > > # First try: succeed in both callbacks > -buf = nbd.Buffer(512) > +buf = bytearray(512) > cookie = h.aio_pread_structured(buf, 0, > lambda *args: chunk(42, *args), > lambda *args: callback((42, 42), *args)) > while not h.aio_command_completed(cookie): > h.poll(-1) > > -buf = buf.to_bytearray() > - > print("%r" % buf) > > assert buf == expected > > +# The callback can stash its slice; as long as that is live, we can't > +# resize buf but can view changes in buf through the slice > +try: > + buf.pop() > + assert False > +except BufferError: > + pass > +buf[0] ^= 1 > +assert buf == stash > +stash = None > +buf.pop() > + > # Second try: fail only during callback > buf = nbd.Buffer(512) > cookie = h.aio_pread_structured(buf, 0, > diff --git a/python/utils.c b/python/utils.c > index cd44b90..d5851f7 100644 > --- a/python/utils.c > +++ b/python/utils.c > @@ -192,3 +192,37 @@ nbd_internal_py_wrap_errptr (int err) > > return PyObject_CallMethod (py_ctypes_mod, "c_int", "i", err); > } > + > +/* Helper to compute view.toreadonly()[start:end] in chunk callback */ > +PyObject * > +nbd_internal_py_get_subview (PyObject *view, const char *subbuf, size_t count) > +{ > + Py_buffer *orig; > + const char *base; > + PyObject *start, *end, *slice; > + PyObject *ret; > + > + assert (PyMemoryView_Check (view)); > + orig = PyMemoryView_GET_BUFFER (view); > + assert (PyBuffer_IsContiguous (orig, 'A')); > + base = orig->buf; > + assert (subbuf >= base && count <= orig->len && > + subbuf + count <= base + orig->len); > + start = PyLong_FromLong (subbuf - base); > + if (!start) return NULL; > + end = PyLong_FromLong (subbuf - base + count); > + if (!end) { Py_DECREF (start); return NULL; } > + slice = PySlice_New (start, end, NULL); > + Py_DECREF (start); > + Py_DECREF (end); > + if (!slice) return NULL; > + ret = PyObject_GetItem (view, slice); > + Py_DECREF (slice); > + /* memoryview.toreadonly() was only added in Python 3.8. > + * PyMemoryView_GetContiguous (ret, PyBuf_READ, 'A') doesn't force readonly. > + * So we mess around directly with the Py_buffer. > + */ > + if (ret) > + PyMemoryView_GET_BUFFER (ret)->readonly = 1; > + return ret; > +}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 Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW