[Libguestfs] [libnbd PATCH v2 8/8] python: Make nbd.Buffer lighter-weight
Eric Blake
eblake at redhat.com
Tue Jun 7 02:08:33 UTC 2022
Instead of storing a PyCapsule in _o and repeatedly doing lookups to
dereference a stored malloc'd pointer, it is easier to just directly
store a Python buffer-like object as _o. Track initialization via
._init: absent for nbd.Buffer(int), present for
nbd.Buffer.from_bytearray or after we have forced initialization due
to aio_pread or .to_bytearray. At this point, we are still copying in
and out of .{from,to}_bytearray, but we are getting closer to reducing
the number of copies. The sheer size in reduced lines of code is a
testament to the ease of doing things closer to Python, rather than
hidden behind unpacking a PyCapsule layer.
Acceptable side effect: nbd.Buffer(-1) changes from:
RuntimeError: length < 0
to
SystemError: Negative size passed to PyByteArray_FromStringAndSize
The generated code changes as follows:
| --- python/methods.h.bak 2022-06-06 20:36:01.973327739 -0500
| +++ python/methods.h 2022-06-06 20:36:16.146352508 -0500
| @@ -62,9 +62,6 @@
| extern PyObject *nbd_internal_py_close (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_display_version (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args);
| -extern PyObject *nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args);
| -extern PyObject *nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args);
| -extern PyObject *nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_set_debug (PyObject *self, PyObject *args);
| extern PyObject *nbd_internal_py_get_debug (PyObject *self, PyObject *args);
| --- python/nbd.py.bak 2022-06-06 20:36:01.978327748 -0500
| +++ python/nbd.py 2022-06-06 20:56:41.866729560 -0500
| @@ -134,18 +134,21 @@ class Buffer(object):
| bytearray constructor. Otherwise, ba is copied. Either way, the
| resulting AIO buffer is independent from the original.
| '''
| - o = libnbdmod.aio_buffer_from_bytearray(ba)
| self = cls(0)
| - self._o = o
| + self._o = bytearray(ba)
| + self._init = True
| return self
|
| def to_bytearray(self):
| '''Copy an AIO buffer into a bytearray.'''
| - return libnbdmod.aio_buffer_to_bytearray(self)
| + if not hasattr(self, '_init'):
| + self._o = bytearray(len(self._o))
| + self._init = True
| + return bytearray(self._o)
|
| def size(self):
| '''Return the size of an AIO buffer.'''
| - return libnbdmod.aio_buffer_size(self)
| + return len(self._o)
|
| def is_zero(self, offset=0, size=-1):
| '''Returns true if and only if all bytes in the buffer are zeroes.
| @@ -161,7 +164,8 @@ class Buffer(object):
| always returns true. If size > 0, we check the interval
| [offset..offset+size-1].
| '''
| - return libnbdmod.aio_buffer_is_zero(self, offset, size)
| + return libnbdmod.aio_buffer_is_zero(self._o, offset, size,
| + hasattr(self, '_init'))
|
|
| class NBD(object):
---
generator/Python.ml | 20 ++--
python/handle.c | 242 +++++++++-----------------------------------
2 files changed, 56 insertions(+), 206 deletions(-)
diff --git a/generator/Python.ml b/generator/Python.ml
index c49af4f..86781ac 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -73,9 +73,6 @@ let
) ([ "create"; "close";
"display_version";
"alloc_aio_buffer";
- "aio_buffer_from_bytearray";
- "aio_buffer_to_bytearray";
- "aio_buffer_size";
"aio_buffer_is_zero"] @ List.map fst handle_calls);
pr "\n";
@@ -105,9 +102,6 @@ let
) ([ "create"; "close";
"display_version";
"alloc_aio_buffer";
- "aio_buffer_from_bytearray";
- "aio_buffer_to_bytearray";
- "aio_buffer_size";
"aio_buffer_is_zero"] @ List.map fst handle_calls);
pr " { NULL, NULL, 0, NULL }\n";
pr "};\n";
@@ -748,18 +742,21 @@ let
bytearray constructor. Otherwise, ba is copied. Either way, the
resulting AIO buffer is independent from the original.
'''
- o = libnbdmod.aio_buffer_from_bytearray(ba)
self = cls(0)
- self._o = o
+ self._o = bytearray(ba)
+ self._init = True
return self
def to_bytearray(self):
'''Copy an AIO buffer into a bytearray.'''
- return libnbdmod.aio_buffer_to_bytearray(self)
+ if not hasattr(self, '_init'):
+ self._o = bytearray(len(self._o))
+ self._init = True
+ return bytearray(self._o)
def size(self):
'''Return the size of an AIO buffer.'''
- return libnbdmod.aio_buffer_size(self)
+ return len(self._o)
def is_zero(self, offset=0, size=-1):
'''Returns true if and only if all bytes in the buffer are zeroes.
@@ -775,7 +772,8 @@ let
always returns true. If size > 0, we check the interval
[offset..offset+size-1].
'''
- return libnbdmod.aio_buffer_is_zero(self, offset, size)
+ return libnbdmod.aio_buffer_is_zero(self._o, offset, size,
+ hasattr(self, '_init'))
class NBD(object):
diff --git a/python/handle.c b/python/handle.c
index 79b369d..ca6c8e9 100644
--- a/python/handle.c
+++ b/python/handle.c
@@ -40,12 +40,6 @@
#include "methods.h"
-struct py_aio_buffer {
- Py_ssize_t len;
- void *data;
- bool initialized;
-};
-
static inline PyObject *
put_handle (struct nbd_handle *h)
{
@@ -102,242 +96,100 @@ nbd_internal_py_display_version (PyObject *self, PyObject *args)
return Py_None;
}
-static const char aio_buffer_name[] = "nbd.Buffer";
-
-/* Return internal buffer pointer of nbd.Buffer */
-static struct py_aio_buffer *
-nbd_internal_py_get_aio_buffer (PyObject *object)
+/* Return new reference to MemoryView wrapping aio_buffer contents */
+PyObject *
+nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
{
+ PyObject *buffer = NULL;
+
if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ())) {
- PyObject *capsule = PyObject_GetAttrString(object, "_o");
+ buffer = PyObject_GetAttrString (object, "_o");
- return PyCapsule_GetPointer (capsule, aio_buffer_name);
+ if (require_init && ! PyObject_HasAttrString (object, "_init")) {
+ assert (PyByteArray_Check (buffer));
+ memset (PyByteArray_AS_STRING (buffer), 0,
+ PyByteArray_GET_SIZE (buffer));
+ if (PyObject_SetAttrString (object, "_init", Py_True) < 0)
+ return NULL;
+ }
}
+ if (buffer)
+ return PyMemoryView_FromObject (buffer);
+
PyErr_SetString (PyExc_TypeError,
"aio_buffer: expecting nbd.Buffer instance");
return NULL;
}
-/* Return new reference to MemoryView wrapping aio_buffer contents */
-PyObject *
-nbd_internal_py_get_aio_view (PyObject *object, bool require_init)
-{
- struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
-
- if (!buf)
- return NULL;
-
- if (require_init && !buf->initialized) {
- memset (buf->data, 0, buf->len);
- buf->initialized = true;
- }
-
- return PyMemoryView_FromMemory (buf->data, buf->len,
- require_init ? PyBUF_READ : PyBUF_WRITE);
-}
-
int
nbd_internal_py_init_aio_buffer (PyObject *object)
{
- struct py_aio_buffer *buf = nbd_internal_py_get_aio_buffer (object);
-
- assert (buf);
- buf->initialized = true;
+ if (PyObject_IsInstance (object, nbd_internal_py_get_nbd_buffer_type ()))
+ return PyObject_SetAttrString (object, "_init", Py_True);
return 0;
}
-static void
-free_aio_buffer (PyObject *capsule)
-{
- struct py_aio_buffer *buf = PyCapsule_GetPointer (capsule, aio_buffer_name);
-
- if (buf)
- free (buf->data);
- free (buf);
-}
-
-/* Allocate a persistent buffer used for nbd_aio_pread. */
+/* Allocate an uninitialized persistent buffer used for nbd_aio_pread. */
PyObject *
nbd_internal_py_alloc_aio_buffer (PyObject *self, PyObject *args)
{
- struct py_aio_buffer *buf;
- PyObject *ret;
+ Py_ssize_t len;
- buf = malloc (sizeof *buf);
- if (buf == NULL) {
- PyErr_NoMemory ();
- return NULL;
- }
-
- buf->initialized = false;
if (!PyArg_ParseTuple (args, "n:nbd_internal_py_alloc_aio_buffer",
- &buf->len)) {
- free (buf);
+ &len))
return NULL;
- }
- if (buf->len < 0) {
- PyErr_SetString (PyExc_RuntimeError, "length < 0");
- free (buf);
- return NULL;
- }
- buf->data = malloc (buf->len);
- if (buf->data == NULL) {
- PyErr_NoMemory ();
- free (buf);
- return NULL;
- }
-
- ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
- if (ret == NULL) {
- free (buf->data);
- free (buf);
- return NULL;
- }
-
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_from_bytearray (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- PyObject *arr = NULL;
- Py_ssize_t len;
- void *data;
- struct py_aio_buffer *buf;
- PyObject *ret;
-
- if (!PyArg_ParseTuple (args,
- "O:nbd_internal_py_aio_buffer_from_bytearray",
- &obj))
- return NULL;
-
- if (! PyByteArray_Check (obj)) {
- arr = PyByteArray_FromObject (obj);
- if (arr == NULL)
- return NULL;
- obj = arr;
- }
- data = PyByteArray_AsString (obj);
- if (!data) {
- PyErr_SetString (PyExc_RuntimeError,
- "parameter is not a bytearray or buffer");
- Py_XDECREF (arr);
- return NULL;
- }
- len = PyByteArray_Size (obj);
-
- buf = malloc (sizeof *buf);
- if (buf == NULL) {
- PyErr_NoMemory ();
- Py_XDECREF (arr);
- return NULL;
- }
-
- buf->len = len;
- buf->data = malloc (len);
- if (buf->data == NULL) {
- PyErr_NoMemory ();
- free (buf);
- Py_XDECREF (arr);
- return NULL;
- }
- memcpy (buf->data, data, len);
- Py_XDECREF (arr);
- buf->initialized = true;
-
- ret = PyCapsule_New (buf, aio_buffer_name, free_aio_buffer);
- if (ret == NULL) {
- free (buf->data);
- free (buf);
- return NULL;
- }
-
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_to_bytearray (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- PyObject *view;
- PyObject *ret;
-
- if (!PyArg_ParseTuple (args,
- "O:nbd_internal_py_aio_buffer_to_bytearray",
- &obj))
- return NULL;
-
- view = nbd_internal_py_get_aio_view (obj, true);
- if (view == NULL)
- return NULL;
-
- ret = PyByteArray_FromObject (view);
- Py_DECREF (view);
- return ret;
-}
-
-PyObject *
-nbd_internal_py_aio_buffer_size (PyObject *self, PyObject *args)
-{
- PyObject *obj;
- struct py_aio_buffer *buf;
-
- if (!PyArg_ParseTuple (args,
- "O:nbd_internal_py_aio_buffer_size",
- &obj))
- return NULL;
-
- buf = nbd_internal_py_get_aio_buffer (obj);
- if (buf == NULL)
- return NULL;
-
- return PyLong_FromSsize_t (buf->len);
+ /* Constructing bytearray(len) in python zeroes the memory; doing it this
+ * way gives uninitialized memory. This correctly flags negative len.
+ */
+ return PyByteArray_FromStringAndSize (NULL, len);
}
PyObject *
nbd_internal_py_aio_buffer_is_zero (PyObject *self, PyObject *args)
{
- PyObject *obj;
- struct py_aio_buffer *buf;
+ Py_buffer buf;
Py_ssize_t offset, size;
+ int init;
+ PyObject *ret = NULL;
if (!PyArg_ParseTuple (args,
- "Onn:nbd_internal_py_aio_buffer_is_zero",
- &obj, &offset, &size))
+ "y*nnp:nbd_internal_py_aio_buffer_is_zero",
+ &buf, &offset, &size, &init))
return NULL;
- if (size == 0)
- Py_RETURN_TRUE;
-
- buf = nbd_internal_py_get_aio_buffer (obj);
- if (buf == NULL)
- return NULL;
+ if (size == 0) {
+ ret = Py_True;
+ goto out;
+ }
/* Check the bounds of the offset. */
- if (offset < 0 || offset > buf->len) {
+ if (offset < 0 || offset > buf.len) {
PyErr_SetString (PyExc_IndexError, "offset out of range");
- return NULL;
+ goto out;
}
/* Compute or check the length. */
if (size == -1)
- size = buf->len - offset;
+ size = buf.len - offset;
else if (size < 0) {
PyErr_SetString (PyExc_IndexError,
"size cannot be negative, "
"except -1 to mean to the end of the buffer");
- return NULL;
+ goto out;
}
- else if ((size_t) offset + size > buf->len) {
+ else if ((size_t) offset + size > buf.len) {
PyErr_SetString (PyExc_IndexError, "size out of range");
- return NULL;
+ goto out;
}
- if (!buf->initialized || is_zero (buf->data + offset, size))
- Py_RETURN_TRUE;
+ if (!init || is_zero (buf.buf + offset, size))
+ ret = Py_True;
else
- Py_RETURN_FALSE;
+ ret = Py_False;
+ out:
+ PyBuffer_Release (&buf);
+ Py_XINCREF (ret);
+ return ret;
}
--
2.36.1
More information about the Libguestfs
mailing list