Commit 5798f94b authored by Simon Feltman's avatar Simon Feltman

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
parent ad680ae9
......@@ -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;
}
......
......@@ -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);
......
......@@ -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
......
......@@ -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;
}
......
......@@ -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;
}
}
......
......@@ -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
......
......@@ -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];