thr3ads.net - Libguestfs - [Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks [Jun 2022]

If this information is useful, please help other people find it:
Share via:

Eric Blake

2022-Jun-09 13:34 UTC

[Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks

Instead of open-coding our code to create a PyObject wrapper of the
mutable *error to be passed to each callback, extract this into a
helper function.  We can then slightly optimize things to hang on to a
single pointer to the ctypes module, rather than looking it up on
every callback.  The generated code shrinks by more than the added
code in utils.c:

| --- python/methods.h.bak	2022-06-08 14:35:00.454786844 -0500
| +++ python/methods.h	2022-06-08 14:43:24.681596853 -0500
| @@ -35,6 +35,7 @@
|  extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
|  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);
|
|  static inline struct nbd_handle *
|  get_handle (PyObject *obj)
| --- python/methods.c.bak	2022-06-08 14:35:00.450786838 -0500
| +++ python/methods.c	2022-06-08 14:43:24.696596877 -0500
| @@ -75,13 +75,7 @@ chunk_wrapper (void *user_data, const vo
|    PyObject *py_args, *py_ret;
|    PyObject *py_error = NULL;
|
| -  PyObject *py_error_modname = PyUnicode_FromString ("ctypes");
| -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
| -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
| -  Py_DECREF (py_error_modname);
| -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
| -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
| -  Py_DECREF (py_error_mod);
| +  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,
| @@ -132,13 +126,7 @@ completion_wrapper (void *user_data, int
|    PyObject *py_args, *py_ret;
|    PyObject *py_error = NULL;
|
| -  PyObject *py_error_modname = PyUnicode_FromString ("ctypes");
| -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
| -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
| -  Py_DECREF (py_error_modname);
| -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
| -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
| -  Py_DECREF (py_error_mod);
| +  py_error = nbd_internal_py_wrap_errptr (*error);
|    if (!py_error) { PyErr_PrintEx (0); goto out; }
|
|    py_args = Py_BuildValue ("(O)", py_error);
| @@ -239,13 +227,7 @@ extent_wrapper (void *user_data, const c
|      if (!py_e_entries) { PyErr_PrintEx (0); goto out; }
|      PyList_SET_ITEM (py_entries, i_entries, py_e_entries);
|    }
| -  PyObject *py_error_modname = PyUnicode_FromString ("ctypes");
| -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
| -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
| -  Py_DECREF (py_error_modname);
| -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
| -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
| -  Py_DECREF (py_error_mod);
| +  py_error = nbd_internal_py_wrap_errptr (*error);
|    if (!py_error) { PyErr_PrintEx (0); goto out; }
|
|    py_args = Py_BuildValue ("(sKOO)", metacontext, offset,
py_entries,
---
 generator/Python.ml |  9 ++-------
 python/utils.c      | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/generator/Python.ml b/generator/Python.ml
index e94f37b..6980034 100644
--- a/generator/Python.ml
+++ b/generator/Python.ml
@@ -41,6 +41,7 @@ let
 extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
 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);

 static inline struct nbd_handle *
 get_handle (PyObject *obj)
@@ -184,13 +185,7 @@ let
     | CBInt _
     | CBInt64 _ -> ()
     | CBMutable (Int n) ->
-       pr "  PyObject *py_%s_modname = PyUnicode_FromString
(\"ctypes\");\n" n;
-       pr "  if (!py_%s_modname) { PyErr_PrintEx (0); goto out; }\n"
n;
-       pr "  PyObject *py_%s_mod = PyImport_Import
(py_%s_modname);\n" n n;
-       pr "  Py_DECREF (py_%s_modname);\n" n;
-       pr "  if (!py_%s_mod) { PyErr_PrintEx (0); goto out; }\n" n;
-       pr "  py_%s = PyObject_CallMethod (py_%s_mod, \"c_int\",
\"i\", *%s);\n" n n n;
-       pr "  Py_DECREF (py_%s_mod);\n" n;
+       pr "  py_%s = nbd_internal_py_wrap_errptr (*%s);\n" n n;
        pr "  if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n;
     | CBString _
     | CBUInt _
diff --git a/python/utils.c b/python/utils.c
index e0df181..cd44b90 100644
--- a/python/utils.c
+++ b/python/utils.c
@@ -173,3 +173,22 @@ nbd_internal_py_get_nbd_buffer_type (void)
   }
   return type;
 }
+
+/* Helper to package callback *error into modifiable PyObject */
+PyObject *
+nbd_internal_py_wrap_errptr (int err)
+{
+  static PyObject *py_ctypes_mod;
+
+  if (!py_ctypes_mod) {
+    PyObject *py_modname = PyUnicode_FromString ("ctypes");
+    if (!py_modname)
+      return NULL;
+    py_ctypes_mod = PyImport_Import (py_modname);
+    Py_DECREF (py_modname);
+    if (!py_ctypes_mod)
+      return NULL;
+  }
+
+  return PyObject_CallMethod (py_ctypes_mod, "c_int", "i",
err);
+}
-- 
2.36.1

Richard W.M. Jones

2022-Jun-09 14:22 UTC

head link

[Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks

On Thu, Jun 09, 2022 at 08:34:43AM -0500, Eric Blake
wrote:> Instead of open-coding our code to create a PyObject wrapper of the
> mutable *error to be passed to each callback, extract this into a
> helper function.  We can then slightly optimize things to hang on to a
> single pointer to the ctypes module, rather than looking it up on
> every callback.  The generated code shrinks by more than the added
> code in utils.c:
> 
> | --- python/methods.h.bak	2022-06-08 14:35:00.454786844 -0500
> | +++ python/methods.h	2022-06-08 14:43:24.681596853 -0500
> | @@ -35,6 +35,7 @@
> |  extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
> |  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);
> |
> |  static inline struct nbd_handle *
> |  get_handle (PyObject *obj)
> | --- python/methods.c.bak	2022-06-08 14:35:00.450786838 -0500
> | +++ python/methods.c	2022-06-08 14:43:24.696596877 -0500
> | @@ -75,13 +75,7 @@ chunk_wrapper (void *user_data, const vo
> |    PyObject *py_args, *py_ret;
> |    PyObject *py_error = NULL;
> |
> | -  PyObject *py_error_modname = PyUnicode_FromString
("ctypes");
> | -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
> | -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
> | -  Py_DECREF (py_error_modname);
> | -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
> | -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
> | -  Py_DECREF (py_error_mod);
> | +  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,
> | @@ -132,13 +126,7 @@ completion_wrapper (void *user_data, int
> |    PyObject *py_args, *py_ret;
> |    PyObject *py_error = NULL;
> |
> | -  PyObject *py_error_modname = PyUnicode_FromString
("ctypes");
> | -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
> | -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
> | -  Py_DECREF (py_error_modname);
> | -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
> | -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
> | -  Py_DECREF (py_error_mod);
> | +  py_error = nbd_internal_py_wrap_errptr (*error);
> |    if (!py_error) { PyErr_PrintEx (0); goto out; }
> |
> |    py_args = Py_BuildValue ("(O)", py_error);
> | @@ -239,13 +227,7 @@ extent_wrapper (void *user_data, const c
> |      if (!py_e_entries) { PyErr_PrintEx (0); goto out; }
> |      PyList_SET_ITEM (py_entries, i_entries, py_e_entries);
> |    }
> | -  PyObject *py_error_modname = PyUnicode_FromString
("ctypes");
> | -  if (!py_error_modname) { PyErr_PrintEx (0); goto out; }
> | -  PyObject *py_error_mod = PyImport_Import (py_error_modname);
> | -  Py_DECREF (py_error_modname);
> | -  if (!py_error_mod) { PyErr_PrintEx (0); goto out; }
> | -  py_error = PyObject_CallMethod (py_error_mod, "c_int",
"i", *error);
> | -  Py_DECREF (py_error_mod);
> | +  py_error = nbd_internal_py_wrap_errptr (*error);
> |    if (!py_error) { PyErr_PrintEx (0); goto out; }
> |
> |    py_args = Py_BuildValue ("(sKOO)", metacontext, offset,
py_entries,
> ---
>  generator/Python.ml |  9 ++-------
>  python/utils.c      | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/generator/Python.ml b/generator/Python.ml
> index e94f37b..6980034 100644
> --- a/generator/Python.ml
> +++ b/generator/Python.ml
> @@ -41,6 +41,7 @@ let
>  extern PyObject *nbd_internal_py_get_aio_view (PyObject *, bool);
>  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);
> 
>  static inline struct nbd_handle *
>  get_handle (PyObject *obj)
> @@ -184,13 +185,7 @@ let
>      | CBInt _
>      | CBInt64 _ -> ()
>      | CBMutable (Int n) ->
> -       pr "  PyObject *py_%s_modname = PyUnicode_FromString
(\"ctypes\");\n" n;
> -       pr "  if (!py_%s_modname) { PyErr_PrintEx (0); goto out;
}\n" n;
> -       pr "  PyObject *py_%s_mod = PyImport_Import
(py_%s_modname);\n" n n;
> -       pr "  Py_DECREF (py_%s_modname);\n" n;
> -       pr "  if (!py_%s_mod) { PyErr_PrintEx (0); goto out; }\n"
n;
> -       pr "  py_%s = PyObject_CallMethod (py_%s_mod,
\"c_int\", \"i\", *%s);\n" n n n;
> -       pr "  Py_DECREF (py_%s_mod);\n" n;
> +       pr "  py_%s = nbd_internal_py_wrap_errptr (*%s);\n" n n;
>         pr "  if (!py_%s) { PyErr_PrintEx (0); goto out; }\n" n;
>      | CBString _
>      | CBUInt _
> diff --git a/python/utils.c b/python/utils.c
> index e0df181..cd44b90 100644
> --- a/python/utils.c
> +++ b/python/utils.c
> @@ -173,3 +173,22 @@ nbd_internal_py_get_nbd_buffer_type (void)
>    }
>    return type;
>  }
> +
> +/* Helper to package callback *error into modifiable PyObject */
> +PyObject *
> +nbd_internal_py_wrap_errptr (int err)
> +{
> +  static PyObject *py_ctypes_mod;
> +
> +  if (!py_ctypes_mod) {
> +    PyObject *py_modname = PyUnicode_FromString ("ctypes");
> +    if (!py_modname)
> +      return NULL;
> +    py_ctypes_mod = PyImport_Import (py_modname);
> +    Py_DECREF (py_modname);
> +    if (!py_ctypes_mod)
> +      return NULL;
> +  }
> +
> +  return PyObject_CallMethod (py_ctypes_mod, "c_int",
"i", err);
> +}
I guess because of the Python GIL we don't need locking here?

We don't run the Python tests with valgrinding so there should be no
issue there.

Reviewed-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
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Libguestfs - Jun 2022 - [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks

[Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks

[Libguestfs] [libnbd PATCH v3 1/5] python: Simplify passing of mutable *error to callbacks