[pygobject] Use ffi_call directly instead of g_callable_info_invoke



commit 5798f94b6a727b930b07fe840b0aef264f98a80e
Author: Simon Feltman <sfeltman src gnome org>
Date:   Fri Feb 7 20:16:21 2014 -0800

    Use ffi_call directly instead of g_callable_info_invoke
    
    Cleanup internal callable cache and state tracking by removing multiple
    counting schemes for differently sized "in" and "out" argument arrays.
    Use a single count based on the total number of arguments passed to C
    (inclusive of instance argument and GError exception where applicable).
    Size all state tracking arrays to the same size and ensure argument cache
    indices always line up with these arrays. This cleans up logic which was
    required by g_callable_info_invoke for splitting "in" and "out" arguments
    up.
    
    Cleanup array marshaling which can now rely on the new scheme which ensures
    the "arg_values" array always points to the correct location for length
    argument values.
    
    Cache the ffi_cif struct in PyGICallableCache via GIFunctionInvoker and
    related GI methods. Overall, these changes can give a performance boost of
    almost 2x for simple function calls (see ticket for micro benchmarks).
    
    https://bugzilla.gnome.org/show_bug.cgi?id=723642

 gi/pygi-array.c               |   26 +----
 gi/pygi-cache.c               |   61 +++++++++--
 gi/pygi-cache.h               |   10 +-
 gi/pygi-ccallback.c           |    3 +-
 gi/pygi-closure.c             |    6 +-
 gi/pygi-invoke-state-struct.h |   42 +++++---
 gi/pygi-invoke.c              |  241 +++++++++++++++++++++++------------------
 gi/pygi-invoke.h              |    2 +-
 gi/pygi-marshal-cleanup.c     |    4 +-
 9 files changed, 230 insertions(+), 165 deletions(-)
---
diff --git a/gi/pygi-array.c b/gi/pygi-array.c
index a50c181..c17ace0 100644
--- a/gi/pygi-array.c
+++ b/gi/pygi-array.c
@@ -370,24 +370,10 @@ array_success:
         PyGIArgCache *child_cache =
             _pygi_callable_cache_get_arg (callable_cache, array_cache->len_arg_index);
 
-        if (child_cache->direction == PYGI_DIRECTION_BIDIRECTIONAL) {
-            gint *len_arg = (gint *)state->in_args[child_cache->c_arg_index].v_pointer;
-            /* if we are not setup yet just set the in arg */
-            if (len_arg == NULL) {
-                if (!gi_argument_from_py_ssize_t (&state->in_args[child_cache->c_arg_index],
-                                                  length,
-                                                  child_cache->type_tag)) {
-                    goto err;
-                }
-            } else {
-                *len_arg = length;
-            }
-        } else {
-            if (!gi_argument_from_py_ssize_t (&state->in_args[child_cache->c_arg_index],
-                                              length,
-                                              child_cache->type_tag)) {
-                goto err;
-            }
+        if (!gi_argument_from_py_ssize_t (&state->arg_values[child_cache->c_arg_index],
+                                          length,
+                                          child_cache->type_tag)) {
+            goto err;
         }
     }
 
@@ -528,7 +514,7 @@ _pygi_marshal_to_py_array (PyGIInvokeState   *state,
                 len = g_strv_length ((gchar **)arg->v_pointer);
             }
         } else {
-            GIArgument *len_arg = state->args[array_cache->len_arg_index];
+            GIArgument *len_arg = &state->arg_values[array_cache->len_arg_index];
             PyGIArgCache *arg_cache = _pygi_callable_cache_get_arg (callable_cache,
                                                                     array_cache->len_arg_index);
 
@@ -684,7 +670,7 @@ _wrap_c_array (PyGIInvokeState   *state,
     } else if (array_cache->is_zero_terminated) {
         len = g_strv_length ((gchar **)data);
     } else if (array_cache->len_arg_index >= 0) {
-        GIArgument *len_arg = state->args[array_cache->len_arg_index];
+        GIArgument *len_arg = &state->arg_values[array_cache->len_arg_index];
         len = len_arg->v_long;
     }
 
diff --git a/gi/pygi-cache.c b/gi/pygi-cache.c
index abf8e10..c29733f 100644
--- a/gi/pygi-cache.c
+++ b/gi/pygi-cache.c
@@ -22,6 +22,7 @@
 
 #include <girepository.h>
 
+#include "pyglib.h"
 #include "pygi-info.h"
 #include "pygi-cache.h"
 #include "pygi-marshal-cleanup.h"
@@ -132,6 +133,7 @@ pygi_callable_cache_free (PyGICallableCache *cache)
     if (cache->return_cache != NULL)
         pygi_arg_cache_free (cache->return_cache);
 
+    g_function_invoker_destroy (&cache->invoker);
     g_slice_free (PyGICallableCache, cache);
 }
 
@@ -530,7 +532,6 @@ _args_cache_generate (GICallableInfo *callable_info,
         _pygi_callable_cache_set_arg (callable_cache, arg_index, instance_cache);
 
         arg_index++;
-        callable_cache->n_from_py_args++;
         callable_cache->n_py_args++;
     }
 
@@ -552,8 +553,6 @@ _args_cache_generate (GICallableInfo *callable_info,
             arg_cache->meta_type = PYGI_META_ARG_TYPE_CLOSURE;
             arg_cache->c_arg_index = i;
 
-            callable_cache->n_from_py_args++;
-
         } else {
             GITypeInfo *type_info;
 
@@ -567,16 +566,15 @@ _args_cache_generate (GICallableInfo *callable_info,
              */
             arg_cache = _pygi_callable_cache_get_arg (callable_cache, arg_index);
             if (arg_cache != NULL) {
+                /* ensure c_arg_index always aligns with callable_cache->args_cache
+                 * and all of the various PyGIInvokeState arrays. */
+                arg_cache->c_arg_index = arg_index;
+
                 if (arg_cache->meta_type == PYGI_META_ARG_TYPE_CHILD_WITH_PYARG) {
                     arg_cache->py_arg_index = callable_cache->n_py_args;
                     callable_cache->n_py_args++;
                 }
 
-                if (direction & PYGI_DIRECTION_FROM_PYTHON) {
-                    arg_cache->c_arg_index = callable_cache->n_from_py_args;
-                    callable_cache->n_from_py_args++;
-                }
-
                 if (direction & PYGI_DIRECTION_TO_PYTHON) {
                     callable_cache->n_to_py_args++;
                 }
@@ -590,7 +588,6 @@ _args_cache_generate (GICallableInfo *callable_info,
 
                 if (direction & PYGI_DIRECTION_FROM_PYTHON) {
                     py_arg_index = callable_cache->n_py_args;
-                    callable_cache->n_from_py_args++;
                     callable_cache->n_py_args++;
                 }
 
@@ -686,8 +683,47 @@ _args_cache_generate (GICallableInfo *callable_info,
     return TRUE;
 }
 
+static gboolean
+_setup_invoker (GICallableInfo *callable_info,
+                GIInfoType info_type,
+                GIFunctionInvoker *invoker,
+                GCallback function_ptr)
+{
+    GError *error = NULL;
+
+    if (info_type == GI_INFO_TYPE_FUNCTION) {
+        if (g_function_info_prep_invoker ((GIFunctionInfo *)callable_info,
+                                          invoker,
+                                          &error)) {
+            return TRUE;
+        }
+        if (!pyglib_error_check (&error)) {
+            PyErr_Format (PyExc_RuntimeError,
+                          "unknown error creating invoker for %s",
+                          g_base_info_get_name ((GIBaseInfo *)callable_info));
+        }
+        return FALSE;
+
+    } else {
+        if (!g_function_invoker_new_for_address (function_ptr,
+                                                 (GIFunctionInfo *)callable_info,
+                                                 invoker,
+                                                 &error)) {
+            if (!pyglib_error_check (&error)) {
+                PyErr_Format (PyExc_RuntimeError,
+                              "unknown error creating invoker for %s",
+                              g_base_info_get_name ((GIBaseInfo *)callable_info));
+            }
+            return FALSE;
+        }
+    }
+    return TRUE;
+}
+
 PyGICallableCache *
-pygi_callable_cache_new (GICallableInfo *callable_info, gboolean is_ccallback)
+pygi_callable_cache_new (GICallableInfo *callable_info,
+                         GCallback function_ptr,
+                         gboolean is_ccallback)
 {
     gint n_args;
     PyGICallableCache *cache;
@@ -699,6 +735,7 @@ pygi_callable_cache_new (GICallableInfo *callable_info, gboolean is_ccallback)
         return NULL;
 
     cache->name = g_base_info_get_name ((GIBaseInfo *)callable_info);
+    cache->throws = g_callable_info_can_throw_gerror ((GIBaseInfo *)callable_info);
 
     if (g_base_info_is_deprecated (callable_info)) {
         const gchar *deprecated = g_base_info_get_attribute (callable_info, "deprecated");
@@ -749,6 +786,10 @@ pygi_callable_cache_new (GICallableInfo *callable_info, gboolean is_ccallback)
     if (!_args_cache_generate (callable_info, cache))
         goto err;
 
+    if (!_setup_invoker (callable_info, type, &cache->invoker, function_ptr)) {
+        goto err;
+    }
+
     return cache;
 err:
     pygi_callable_cache_free (cache);
diff --git a/gi/pygi-cache.h b/gi/pygi-cache.h
index 5521605..407e38c 100644
--- a/gi/pygi-cache.h
+++ b/gi/pygi-cache.h
@@ -25,6 +25,7 @@
 
 #include <Python.h>
 #include <girepository.h>
+#include <girffi.h>
 
 #include "pygi-invoke-state-struct.h"
 
@@ -167,14 +168,11 @@ struct _PyGICallableCache
     GSList *to_py_args;
     GSList *arg_name_list; /* for keyword arg matching */
     GHashTable *arg_name_hash;
+    gboolean throws;
 
     /* Index of user_data arg that can eat variable args passed to a callable. */
     gssize user_data_varargs_index;
 
-    /* Number of in args passed to g_function_info_invoke.
-     * This is used for the length of PyGIInvokeState.in_args */
-    gssize n_from_py_args;
-
     /* Number of out args passed to g_function_info_invoke.
      * This is used for the length of PyGIInvokeState.out_values */
     gssize n_to_py_args;
@@ -191,6 +189,9 @@ struct _PyGICallableCache
     /* Minimum number of args required to call the callable from Python.
      * This count does not include args with defaults. */
     gssize n_py_required_args;
+
+    /* An invoker with ffi_cif already setup */
+    GIFunctionInvoker invoker;
 };
 
 gboolean
@@ -243,6 +244,7 @@ pygi_callable_cache_free (PyGICallableCache *cache);
 
 PyGICallableCache *
 pygi_callable_cache_new  (GICallableInfo *callable_info,
+                          GCallback function_ptr,
                           gboolean is_ccallback);
 
 #define _pygi_callable_cache_args_len(cache) ((cache)->args_cache)->len
diff --git a/gi/pygi-ccallback.c b/gi/pygi-ccallback.c
index 01e109b..2a6c520 100644
--- a/gi/pygi-ccallback.c
+++ b/gi/pygi-ccallback.c
@@ -34,7 +34,7 @@ _ccallback_call(PyGICCallback *self, PyObject *args, PyObject *kwargs)
     PyObject *result;
 
     if (self->cache == NULL) {
-        self->cache = pygi_callable_cache_new (self->info, TRUE);
+        self->cache = pygi_callable_cache_new (self->info, self->callback, TRUE);
         if (self->cache == NULL)
             return NULL;
     }
@@ -43,7 +43,6 @@ _ccallback_call(PyGICCallback *self, PyObject *args, PyObject *kwargs)
                                          args,
                                          kwargs,
                                          self->cache,
-                                         self->callback,
                                          self->user_data);
     return result;
 }
diff --git a/gi/pygi-closure.c b/gi/pygi-closure.c
index a30363f..b803541 100644
--- a/gi/pygi-closure.c
+++ b/gi/pygi-closure.c
@@ -800,7 +800,7 @@ _pygi_marshal_from_py_interface_callback (PyGIInvokeState   *state,
      * The return trip to python will marshal this back and pull the python user data out.
      */
     if (user_data_cache != NULL) {
-        state->in_args[user_data_cache->c_arg_index].v_pointer = closure;
+        state->arg_values[user_data_cache->c_arg_index].v_pointer = closure;
     }
 
     /* Setup a GDestroyNotify callback if this method supports it along with
@@ -817,7 +817,7 @@ _pygi_marshal_from_py_interface_callback (PyGIInvokeState   *state,
     if (destroy_cache) {
         if (user_data_cache != NULL) {
             PyGICClosure *destroy_notify = _pygi_destroy_notify_create ();
-            state->in_args[destroy_cache->c_arg_index].v_pointer = destroy_notify->closure;
+            state->arg_values[destroy_cache->c_arg_index].v_pointer = destroy_notify->closure;
         } else {
             gchar *msg = g_strdup_printf("Callables passed to %s will leak references because "
                                          "the method does not support a user_data argument. "
@@ -829,7 +829,7 @@ _pygi_marshal_from_py_interface_callback (PyGIInvokeState   *state,
                 return FALSE;
             }
             g_free(msg);
-            state->in_args[destroy_cache->c_arg_index].v_pointer = _pygi_destroy_notify_dummy;
+            state->arg_values[destroy_cache->c_arg_index].v_pointer = _pygi_destroy_notify_dummy;
         }
     }
 
diff --git a/gi/pygi-invoke-state-struct.h b/gi/pygi-invoke-state-struct.h
index 139b878..174f473 100644
--- a/gi/pygi-invoke-state-struct.h
+++ b/gi/pygi-invoke-state-struct.h
@@ -11,39 +11,49 @@ typedef struct _PyGIInvokeState
 {
     PyObject *py_in_args;
     gssize n_py_in_args;
-    gssize current_arg;
 
     GType implementor_gtype;
 
+    /* Number of arguments the ffi wrapped C function takes. Used as the exact
+     * count for argument related arrays held in this struct.
+     */
+    gssize n_args;
+
+    /* List of arguments passed to ffi. Elements can point directly to values held in
+     * arg_values for "in/from Python" or indirectly via arg_pointers for
+     * "out/inout/to Python". In the latter case, the arg_pointers[x]->v_pointer
+     * member points to memory for the value storage.
+     */
     GIArgument **args;
-    GIArgument *in_args;
+
+    /* Holds memory for the C value of arguments marshaled "to" or "from" Python. */
+    GIArgument *arg_values;
+
+    /* Holds pointers to values in arg_values or a caller allocated chunk of
+     * memory via arg_pointers[x].v_pointer.
+     */
+    GIArgument *arg_pointers;
 
     /* Array of pointers allocated to the same length as args which holds from_py
      * marshaler cleanup data.
      */
     gpointer *args_cleanup_data;
 
-    /* Out args and out values
-     * In order to pass a parameter and get something back out in C
-     * we need to pass a pointer to the value, e.g.
-     *    int *out_integer;
-     *
-     * so while out_args == out_integer, out_value == *out_integer
-     * or in other words out_args = &out_values
-     *
-     * We do all of our processing on out_values but we pass out_args to 
-     * the actual function.
-     */
-    GIArgument *out_args;
-    GIArgument *out_values;
-
+    /* Memory to receive the result of the C ffi function call. */
     GIArgument return_arg;
 
+    /* A GError exception which is indirectly bound into the last position of
+     * the "args" array if the callable caches "throws" member is set.
+     */
     GError *error;
 
     gboolean failed;
 
     gpointer user_data;
+
+    /* Function pointer to call with ffi. */
+    gpointer function_ptr;
+
 } PyGIInvokeState;
 
 G_END_DECLS
diff --git a/gi/pygi-invoke.c b/gi/pygi-invoke.c
index 1d89912..3bb4dc6 100644
--- a/gi/pygi-invoke.c
+++ b/gi/pygi-invoke.c
@@ -29,61 +29,24 @@
 static inline gboolean
 _invoke_callable (PyGIInvokeState *state,
                   PyGICallableCache *cache,
-                  GICallableInfo *callable_info,
-                  GCallback function_ptr)
+                  GICallableInfo *callable_info)
 {
-    GError *error;
-    gint retval;
-
-    error = NULL;
+    GIFFIReturnValue ffi_return_value = {0};
 
     Py_BEGIN_ALLOW_THREADS;
 
-    /* FIXME: use this for now but we can streamline the calls */
-    if (cache->function_type == PYGI_FUNCTION_TYPE_VFUNC)
-        retval = g_vfunc_info_invoke ( callable_info,
-                                       state->implementor_gtype,
-                                       state->in_args,
-                                       cache->n_from_py_args,
-                                       state->out_args,
-                                       cache->n_to_py_args,
-                                      &state->return_arg,
-                                      &error);
-    else if (g_base_info_get_type (callable_info) == GI_INFO_TYPE_CALLBACK)
-        retval = g_callable_info_invoke (callable_info,
-                                         function_ptr,
-                                         state->in_args,
-                                         cache->n_from_py_args,
-                                         state->out_args,
-                                         cache->n_to_py_args,
-                                         &state->return_arg,
-                                         FALSE,
-                                         FALSE,
-                                         &error);
-    else
-        retval = g_function_info_invoke ( callable_info,
-                                          state->in_args,
-                                          cache->n_from_py_args,
-                                          state->out_args,
-                                          cache->n_to_py_args,
-                                         &state->return_arg,
-                                         &error);
-    Py_END_ALLOW_THREADS;
-
-    if (!retval) {
-        g_assert (error != NULL);
-        pyglib_error_check (&error);
-
-        /* It is unclear if the error occured before or after the C
-         * function was invoked so for now assume success
-         * We eventually should marshal directly to FFI so we no longer
-         * have to use the reference implementation
-         */
-        pygi_marshal_cleanup_args_from_py_marshal_success (state, cache);
+        ffi_call (&cache->invoker.cif,
+                  state->function_ptr,
+                  (void *)&ffi_return_value,
+                  (void **)state->args);
 
-        return FALSE;
-    }
+    Py_END_ALLOW_THREADS;
 
+    /* If the callable throws, the address of state->error will be bound into
+     * the state->args as the last value. When the callee sets an error using
+     * the state->args passed, it will have the side effect of setting
+     * state->error allowing for easy checking here.
+     */
     if (state->error != NULL) {
         if (pyglib_error_check (&(state->error))) {
             /* even though we errored out, the call itself was successful,
@@ -93,6 +56,12 @@ _invoke_callable (PyGIInvokeState *state,
         }
     }
 
+    if (cache->return_cache) {
+        gi_type_info_extract_ffi_return_value (cache->return_cache->type_info,
+                                               &ffi_return_value,
+                                               &state->return_arg);
+    }
+
     return TRUE;
 }
 
@@ -280,13 +249,24 @@ _py_args_combine_and_check_length (PyGICallableCache *cache,
 }
 
 static inline gboolean
-_invoke_state_init_from_callable_cache (PyGIInvokeState *state,
+_invoke_state_init_from_callable_cache (GIBaseInfo *info,
+                                        PyGIInvokeState *state,
                                         PyGICallableCache *cache,
                                         PyObject *py_args,
                                         PyObject *kwargs)
 {
     PyObject *combined_args = NULL;
     state->implementor_gtype = 0;
+    state->n_args = _pygi_callable_cache_args_len (cache);
+
+    if (cache->throws) {
+        state->n_args++;
+    }
+
+    /* Copy the function pointer to the state for the normal case. For vfuncs,
+     * this will be filled out based on the implementor_gtype calculated below.
+     */
+    state->function_ptr = cache->invoker.native_address;
 
     /* TODO: We don't use the class parameter sent in by  the structure
      * so we remove it from the py_args tuple but we can keep it 
@@ -308,6 +288,8 @@ _invoke_state_init_from_callable_cache (PyGIInvokeState *state,
         }
     } else if (cache->function_type == PYGI_FUNCTION_TYPE_VFUNC) {
         PyObject *py_gtype;
+        GError *error = NULL;
+
         py_gtype = PyTuple_GetItem (py_args, 0);
         if (py_gtype == NULL) {
             PyErr_SetString (PyExc_TypeError,
@@ -319,6 +301,18 @@ _invoke_state_init_from_callable_cache (PyGIInvokeState *state,
 
         if (state->implementor_gtype == 0)
             return FALSE;
+
+        /* vfunc addresses are pulled into the state at call time and cannot be
+         * cached because the call site can specify a different portion of the
+         * class hierarchy. e.g. Object.do_func vs. SubObject.do_func might
+         * retrieve a different vfunc address but GI gives us the same vfunc info.
+         */
+        state->function_ptr = g_vfunc_info_get_address ((GIVFuncInfo *)info,
+                                                        state->implementor_gtype,
+                                                        &error);
+        if (pyglib_error_check (&error)) {
+            return FALSE;
+        }
     }
 
     if  (cache->function_type == PYGI_FUNCTION_TYPE_CONSTRUCTOR ||
@@ -344,68 +338,65 @@ _invoke_state_init_from_callable_cache (PyGIInvokeState *state,
     }
     state->n_py_in_args = PyTuple_Size (state->py_in_args);
 
-    state->args = g_slice_alloc0 (_pygi_callable_cache_args_len (cache) * sizeof (GIArgument *));
-    if (state->args == NULL && _pygi_callable_cache_args_len (cache) != 0) {
+    state->args = g_slice_alloc0 (state->n_args * sizeof (GIArgument *));
+    if (state->args == NULL && state->n_args != 0) {
         PyErr_NoMemory();
         return FALSE;
     }
 
-    state->args_cleanup_data = g_slice_alloc0 (_pygi_callable_cache_args_len (cache) * sizeof (gpointer));
-    if (state->args_cleanup_data == NULL && _pygi_callable_cache_args_len (cache) != 0) {
+    state->args_cleanup_data = g_slice_alloc0 (state->n_args * sizeof (gpointer));
+    if (state->args_cleanup_data == NULL && state->n_args != 0) {
         PyErr_NoMemory();
         return FALSE;
     }
 
-    state->in_args = g_slice_alloc0 (cache->n_from_py_args * sizeof(GIArgument));
-    if (state->in_args == NULL && cache->n_from_py_args != 0) {
+    state->arg_values = g_slice_alloc0 (state->n_args * sizeof(GIArgument));
+    if (state->arg_values == NULL && state->n_args != 0) {
         PyErr_NoMemory ();
         return FALSE;
     }
 
-    state->out_values = g_slice_alloc0 (cache->n_to_py_args * sizeof(GIArgument));
-    if (state->out_values == NULL && cache->n_to_py_args != 0) {
-        PyErr_NoMemory ();
-        return FALSE;
-    }
-
-    state->out_args = g_slice_alloc0 (cache->n_to_py_args * sizeof(GIArgument));
-    if (state->out_args == NULL && cache->n_to_py_args != 0) {
+    state->arg_pointers = g_slice_alloc0 (state->n_args * sizeof(GIArgument));
+    if (state->arg_pointers == NULL && state->n_args != 0) {
         PyErr_NoMemory ();
         return FALSE;
     }
 
     state->error = NULL;
 
+    if (cache->throws) {
+        gssize error_index = state->n_args - 1;
+        /* The ffi argument for GError needs to be a triple pointer. */
+        state->arg_pointers[error_index].v_pointer = &state->error;
+        state->args[error_index] = &(state->arg_pointers[error_index]);
+    }
+
     return TRUE;
 }
 
 static inline void
 _invoke_state_clear (PyGIInvokeState *state, PyGICallableCache *cache)
 {
-    g_slice_free1 (_pygi_callable_cache_args_len (cache) * sizeof(GIArgument *), state->args);
-    g_slice_free1 (_pygi_callable_cache_args_len (cache) * sizeof(gpointer), state->args_cleanup_data);
-    g_slice_free1 (cache->n_from_py_args * sizeof(GIArgument), state->in_args);
-    g_slice_free1 (cache->n_to_py_args * sizeof(GIArgument), state->out_args);
-    g_slice_free1 (cache->n_to_py_args * sizeof(GIArgument), state->out_values);
+    g_slice_free1 (state->n_args * sizeof(GIArgument *), state->args);
+    g_slice_free1 (state->n_args * sizeof(gpointer), state->args_cleanup_data);
+    g_slice_free1 (state->n_args * sizeof(GIArgument), state->arg_values);
+    g_slice_free1 (state->n_args * sizeof(GIArgument), state->arg_pointers);
 
     Py_XDECREF (state->py_in_args);
 }
 
-static gboolean _caller_alloc (PyGIInvokeState *state,
-                               PyGIArgCache *arg_cache,
-                               gssize arg_count,
-                               gssize out_count)
+static gboolean
+_caller_alloc (PyGIArgCache *arg_cache, GIArgument *arg)
 {
     if (arg_cache->type_tag == GI_TYPE_TAG_INTERFACE) {
         PyGIInterfaceCache *iface_cache = (PyGIInterfaceCache *)arg_cache;
 
-        state->out_args[out_count].v_pointer = NULL;
-        state->args[arg_count] = &state->out_args[out_count];
+        arg->v_pointer = NULL;
         if (g_type_is_a (iface_cache->g_type, G_TYPE_BOXED)) {
-            state->args[arg_count]->v_pointer =
+            arg->v_pointer =
                 _pygi_boxed_alloc (iface_cache->interface_info, NULL);
         } else if (iface_cache->g_type == G_TYPE_VALUE) {
-            state->args[arg_count]->v_pointer = g_slice_new0 (GValue);
+            arg->v_pointer = g_slice_new0 (GValue);
         } else if (iface_cache->is_foreign) {
             PyObject *foreign_struct =
                 pygi_struct_foreign_convert_from_g_argument (
@@ -415,34 +406,59 @@ static gboolean _caller_alloc (PyGIInvokeState *state,
                 pygi_struct_foreign_convert_to_g_argument (foreign_struct,
                                                            iface_cache->interface_info,
                                                            GI_TRANSFER_EVERYTHING,
-                                                           state->args[arg_count]);
+                                                           arg);
         } else {
                 gssize size = g_struct_info_get_size(
                     (GIStructInfo *)iface_cache->interface_info);
-                state->args[arg_count]->v_pointer = g_malloc0 (size);
+                arg->v_pointer = g_malloc0 (size);
         }
     } else if (arg_cache->type_tag == GI_TYPE_TAG_ARRAY) {
         PyGIArgGArray *array_cache = (PyGIArgGArray *)arg_cache;
 
-        state->out_args[out_count].v_pointer = g_array_new (TRUE, TRUE, array_cache->item_size);
-        state->args[arg_count] = &state->out_args[out_count];
+        arg->v_pointer = g_array_new (TRUE, TRUE, array_cache->item_size);
     } else {
         return FALSE;
     }
 
-    if (state->args[arg_count]->v_pointer == NULL)
+    if (arg->v_pointer == NULL)
         return FALSE;
 
 
     return TRUE;
 }
 
+/* _invoke_marshal_in_args:
+ *
+ * Fills out the state struct argument lists. arg_values will always hold
+ * actual values marshaled either to or from Python and C. arg_pointers will
+ * hold pointers (via v_pointer) to auxilary value storage. This will normally
+ * point to values stored in arg_values. In the case of caller allocated
+ * out args, arg_pointers[x].v_pointer will point to newly allocated memory.
+ * arg_pointers inserts a level of pointer indirection between arg_values
+ * and the argument list ffi receives when dealing with non-caller allocated
+ * out arguments.
+ *
+ * For example:
+ * [[
+ *  void callee (int *i, int j) { *i = 50 - j; }
+ *  void caller () {
+ *    int i = 0;
+ *    callee (&i, 8);
+ *  }
+ *
+ *  args[0] == &arg_pointers[0];
+ *  arg_pointers[0].v_pointer == &arg_values[0];
+ *  arg_values[0].v_int == 42;
+ *
+ *  args[1] == &arg_values[1];
+ *  arg_values[1].v_int == 8;
+ * ]]
+ *
+ */
 static inline gboolean
 _invoke_marshal_in_args (PyGIInvokeState *state, PyGICallableCache *cache)
 {
-    gssize i, in_count, out_count;
-    in_count = 0;
-    out_count = 0;
+    gssize i;
 
     if (state->n_py_in_args > cache->n_py_args) {
         PyErr_Format (PyExc_TypeError,
@@ -454,14 +470,14 @@ _invoke_marshal_in_args (PyGIInvokeState *state, PyGICallableCache *cache)
     }
 
     for (i = 0; i < _pygi_callable_cache_args_len (cache); i++) {
-        GIArgument *c_arg;
+        GIArgument *c_arg = &state->arg_values[i];
         PyGIArgCache *arg_cache = g_ptr_array_index (cache->args_cache, i);
         PyObject *py_arg = NULL;
 
         switch (arg_cache->direction) {
             case PYGI_DIRECTION_FROM_PYTHON:
-                state->args[i] = &(state->in_args[in_count]);
-                in_count++;
+                /* The ffi argument points directly at memory in arg_values. */
+                state->args[i] = c_arg;
 
                 if (arg_cache->meta_type == PYGI_META_ARG_TYPE_CLOSURE) {
                     state->args[i]->v_pointer = state->user_data;
@@ -491,13 +507,6 @@ _invoke_marshal_in_args (PyGIInvokeState *state, PyGICallableCache *cache)
 
                 break;
             case PYGI_DIRECTION_BIDIRECTIONAL:
-                /* this will be filled in if it is an child value */
-                if (state->in_args[in_count].v_pointer != NULL)
-                    state->out_values[out_count] = state->in_args[in_count];
-
-                state->in_args[in_count].v_pointer = &state->out_values[out_count];
-                in_count++;
-
                 if (arg_cache->meta_type != PYGI_META_ARG_TYPE_CHILD) {
                     if (arg_cache->py_arg_index >= state->n_py_in_args) {
                         PyErr_Format (PyExc_TypeError,
@@ -515,9 +524,27 @@ _invoke_marshal_in_args (PyGIInvokeState *state, PyGICallableCache *cache)
                         PyTuple_GET_ITEM (state->py_in_args,
                                           arg_cache->py_arg_index);
                 }
+                /* Fall through */
+
             case PYGI_DIRECTION_TO_PYTHON:
+                /* arg_pointers always stores a pointer to the data to be marshaled "to python"
+                 * even in cases where arg_pointers is not being used as indirection between
+                 * ffi and arg_values. This gives a guarantee that out argument marshaling
+                 * (_invoke_marshal_out_args) can always rely on arg_pointers pointing to
+                 * the correct chunk of memory to marshal.
+                 */
+                state->arg_pointers[i].v_pointer = c_arg;
+
                 if (arg_cache->is_caller_allocates) {
-                    if (!_caller_alloc (state, arg_cache, i, out_count)) {
+                    /* In the case of caller allocated out args, we don't use
+                     * an extra level of indirection and state->args will point
+                     * directly at the data to be marshaled. However, as noted
+                     * above, arg_pointers will also point to this caller allocated
+                     * chunk of memory used by out argument marshaling.
+                     */
+                    state->args[i] = c_arg;
+
+                    if (!_caller_alloc (arg_cache, c_arg)) {
                         PyErr_Format (PyExc_TypeError,
                                       "Could not caller allocate argument %zd of callable %s",
                                       i, cache->name);
@@ -527,14 +554,14 @@ _invoke_marshal_in_args (PyGIInvokeState *state, PyGICallableCache *cache)
                         return FALSE;
                     }
                 } else {
-                    state->out_args[out_count].v_pointer = &state->out_values[out_count];
-                    state->args[i] = &state->out_values[out_count];
+                    /* Non-caller allocated out args will use arg_pointers as an
+                     * extra level of indirection */
+                    state->args[i] = &state->arg_pointers[i];
                 }
-                out_count++;
+
                 break;
         }
 
-        c_arg = state->args[i];
         if (py_arg == _PyGIDefaultArgPlaceholder) {
             *c_arg = arg_cache->default_value;
         } else if (arg_cache->from_py_marshaller != NULL) {
@@ -642,7 +669,7 @@ _invoke_marshal_out_args (PyGIInvokeState *state, PyGICallableCache *cache)
         py_out = arg_cache->to_py_marshaller (state,
                                               cache,
                                               arg_cache,
-                                              state->args[arg_cache->c_arg_index]);
+                                              state->arg_pointers[arg_cache->c_arg_index].v_pointer);
         if (py_out == NULL) {
             pygi_marshal_cleanup_args_to_py_parameter_fail (state,
                                                             cache,
@@ -665,7 +692,7 @@ _invoke_marshal_out_args (PyGIInvokeState *state, PyGICallableCache *cache)
             PyObject *py_obj = arg_cache->to_py_marshaller (state,
                                                             cache,
                                                             arg_cache,
-                                                            state->args[arg_cache->c_arg_index]);
+                                                            
state->arg_pointers[arg_cache->c_arg_index].v_pointer);
 
             if (py_obj == NULL) {
                 if (has_return)
@@ -688,12 +715,12 @@ _invoke_marshal_out_args (PyGIInvokeState *state, PyGICallableCache *cache)
 PyObject *
 pygi_callable_info_invoke (GIBaseInfo *info, PyObject *py_args,
                            PyObject *kwargs, PyGICallableCache *cache,
-                           GCallback function_ptr, gpointer user_data)
+                           gpointer user_data)
 {
     PyGIInvokeState state = { 0, };
     PyObject *ret = NULL;
 
-    if (!_invoke_state_init_from_callable_cache (&state, cache, py_args, kwargs))
+    if (!_invoke_state_init_from_callable_cache (info, &state, cache, py_args, kwargs))
         goto err;
 
     if (cache->function_type == PYGI_FUNCTION_TYPE_CCALLBACK)
@@ -702,7 +729,7 @@ pygi_callable_info_invoke (GIBaseInfo *info, PyObject *py_args,
     if (!_invoke_marshal_in_args (&state, cache))
         goto err;
 
-    if (!_invoke_callable (&state, cache, info, function_ptr))
+    if (!_invoke_callable (&state, cache, info))
         goto err;
 
     ret = _invoke_marshal_out_args (&state, cache);
@@ -720,10 +747,10 @@ _wrap_g_callable_info_invoke (PyGIBaseInfo *self, PyObject *py_args,
                               PyObject *kwargs)
 {
     if (self->cache == NULL) {
-        self->cache = pygi_callable_cache_new (self->info, FALSE);
+        self->cache = pygi_callable_cache_new (self->info, NULL, FALSE);
         if (self->cache == NULL)
             return NULL;
     }
 
-    return pygi_callable_info_invoke (self->info, py_args, kwargs, self->cache, NULL, NULL);
+    return pygi_callable_info_invoke (self->info, py_args, kwargs, self->cache, NULL);
 }
diff --git a/gi/pygi-invoke.h b/gi/pygi-invoke.h
index 051bc8d..a481be3 100644
--- a/gi/pygi-invoke.h
+++ b/gi/pygi-invoke.h
@@ -32,7 +32,7 @@ G_BEGIN_DECLS
 
 PyObject *pygi_callable_info_invoke (GIBaseInfo *info, PyObject *py_args,
                                      PyObject *kwargs, PyGICallableCache *cache,
-                                     GCallback function_ptr, gpointer user_data);
+                                     gpointer user_data);
 PyObject *_wrap_g_callable_info_invoke (PyGIBaseInfo *self, PyObject *py_args,
                                         PyObject *kwargs);
 
diff --git a/gi/pygi-marshal-cleanup.c b/gi/pygi-marshal-cleanup.c
index 3d82601..169e149 100644
--- a/gi/pygi-marshal-cleanup.c
+++ b/gi/pygi-marshal-cleanup.c
@@ -136,7 +136,7 @@ pygi_marshal_cleanup_args_to_py_marshal_success (PyGIInvokeState   *state,
     while (cache_item) {
         PyGIArgCache *arg_cache = (PyGIArgCache *) cache_item->data;
         PyGIMarshalCleanupFunc cleanup_func = arg_cache->to_py_cleanup;
-        gpointer data = state->args[arg_cache->c_arg_index]->v_pointer;
+        gpointer data = state->arg_values[arg_cache->c_arg_index].v_pointer;
 
         if (cleanup_func != NULL && data != NULL)
             cleanup_func (state,
@@ -168,7 +168,7 @@ pygi_marshal_cleanup_args_from_py_parameter_fail (PyGIInvokeState   *state,
     for (i = 0; i < _pygi_callable_cache_args_len (cache)  && i <= failed_arg_index; i++) {
         PyGIArgCache *arg_cache = _pygi_callable_cache_get_arg (cache, i);
         PyGIMarshalCleanupFunc cleanup_func = arg_cache->from_py_cleanup;
-        gpointer data = state->args[i]->v_pointer;
+        gpointer data = state->arg_values[i].v_pointer;
         PyObject *py_arg = PyTuple_GET_ITEM (state->py_in_args,
                                              arg_cache->py_arg_index);
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]