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 <rjo...@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 mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
