[gjs/wip/gcampax/70-arg-cache: 6/6] function: use the argument cache for the instance parameter too

[Philip Chimento <pchimento src gnome org>]

commit d05ab978727190b7bf2379bdf01c2c536d120437
Author: Giovanni Campagna <gcampagna src gnome org>
Date:   Mon Apr 22 19:21:10 2013 +0200

    function: use the argument cache for the instance parameter too
    
    This frees us of expensive allocations and switches in another
    hot path.
    
    Closes #70.
    
    [skip ci] - doesn't compile yet
    
    (Philip Chimento: Rebased and fixed coding style.)

 gi/arg-cache.cpp | 55 +++++++++++++++++++++++++++++++++++++++---------
 gi/arg-cache.h   | 11 ++++++++++
 gi/function.cpp  | 64 ++++++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 102 insertions(+), 28 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 0dd356d..7e8622f 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -42,6 +42,8 @@
 
 static bool gjs_arg_cache_build_normal_in_arg(GjsArgumentCache *self,
                                               GITypeTag         tag);
+static bool gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self,
+                                                 GIBaseInfo       *interface_info);
 
 /* The global entry point for any invocations of GDestroyNotify;
  * look up the callback through the user_data and then free it.
@@ -561,6 +563,34 @@ gjs_arg_cache_build_return (GjsArgumentCache *self,
     return true;
 }
 
+bool
+gjs_arg_cache_build_instance(GjsArgumentCache *self,
+                             GICallableInfo   *info)
+{
+    GIBaseInfo *interface_info = g_base_info_get_container(info);
+
+    self->arg_index = -2;
+    self->arg_name = "instance parameter";
+    /* XXX: This is actually wrong for some calls, like
+       g_dbus_method_invocation_return_value(), but the
+       information is not reported in the typelib
+    */
+    self->transfer = GI_TRANSFER_NOTHING;
+    /* Some calls accept null for the instance, but generally
+       in an object oriented language it's wrong to call a method
+       on null */
+    self->nullable = false;
+    self->skip_out = true;
+
+    /* Don't set marshal_out, it should not be called for the
+       instance parameter */
+    self->release = gjs_marshal_skipped_release;
+    bool ok = gjs_arg_cache_build_interface_in_arg(self, interface_info);
+
+    g_base_info_unref(interface_info);
+    return ok;
+}
+
 bool
 gjs_arg_cache_build_arg (GjsArgumentCache *self,
                          GjsArgumentCache *arguments,
@@ -1309,11 +1339,10 @@ gjs_arg_cache_build_flags_mask(GjsArgumentCache *self,
 }
 
 static bool
-gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self)
+gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self,
+                                     GIBaseInfo       *interface_info)
 {
-    GIBaseInfo *interface_info = g_type_info_get_interface(&self->type_info);
     GIInfoType interface_type = g_base_info_get_type(interface_info);
-    bool ok = true;
     GType gtype;
 
     /* We do some transfer magic later, so lets ensure we
@@ -1377,13 +1406,13 @@ gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self)
                 /* This is a smart marshaller, no release needed */
             } else {
                 /* Can't handle unions without a GType */
-                ok = false;
+                return false;
             }
         } else { /* generic boxed type */
             if (gtype == G_TYPE_NONE &&
                 self->transfer != GI_TRANSFER_NOTHING) {
                 /* Can't transfer ownership of a structure type not registered as a boxed */
-                ok = false;
+                return false;
             } else {
                 self->marshal_in = gjs_marshal_boxed_in_in;
                 /* This is a smart marshaller, no release needed */
@@ -1395,11 +1424,10 @@ gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self)
         /* Don't know how to handle this interface type
            (should not happen in practice, for typelibs emitted
            by g-ir-compiler) */
-        ok = false;
+        return false;
     }
 
-    g_base_info_unref(interface_info);
-    return ok;
+    return true;
 }
 
 static bool
@@ -1478,8 +1506,15 @@ gjs_arg_cache_build_normal_in_arg(GjsArgumentCache *self,
         self->contents.string_is_filename = false;
         break;
 
-    case GI_TYPE_TAG_INTERFACE:
-        return gjs_arg_cache_build_interface_in_arg(self);
+    case GI_TYPE_TAG_INTERFACE: {
+        GIBaseInfo *interface_info;
+        JSBool ok;
+
+        interface_info = g_type_info_get_interface(&self->type_info);
+        ok = gjs_arg_cache_build_interface_in_arg(self, interface_info);
+        g_base_info_unref(interface_info);
+        return ok;
+    }
 
     case GI_TYPE_TAG_ARRAY:
     case GI_TYPE_TAG_GLIST:
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index b390e5a..7b2048a 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -110,11 +110,22 @@ bool gjs_arg_cache_build_arg(GjsArgumentCache *self,
                              GICallableInfo   *callable,
                              bool             *inc_counter);
 
+bool gjs_arg_cache_build_return(GjsArgumentCache *self,
+                                GjsArgumentCache *arguments,
+                                int               gi_index,
+                                GIDirection       direction,
+                                GIArgInfo        *arg,
+                                GICallableInfo   *callable,
+                                bool             *inc_counter);
+
 bool gjs_arg_cache_build_return(GjsArgumentCache *self,
                                 GjsArgumentCache *arguments,
                                 GICallableInfo   *info,
                                 bool             *inc_counter);
 
+bool gjs_arg_cache_build_instance(GjsArgumentCache *self,
+                                  GICallableInfo   *info);
+
 static inline bool
 gjs_arg_cache_is_skip_in(GjsArgumentCache *cache)
 {
diff --git a/gi/function.cpp b/gi/function.cpp
index d3486e1..bfd1995 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -721,7 +721,6 @@ gjs_invoke_c_function(JSContext                             *context,
     bool failed, postinvoke_release_failed;
 
     bool is_method;
-    bool is_object_method = FALSE;
     GITypeTag return_tag;
     GSList *iter;
 
@@ -806,12 +805,15 @@ gjs_invoke_c_function(JSContext                             *context,
     js_arg_pos = 0; /* index into argv */
 
     if (is_method) {
-        if (!gjs_fill_method_instance(context, obj, function,
-                                      &state.in_arg_cvalues[-2],
-                                      is_object_method))
+        GjsArgumentCache *cache = &function->arguments[-2];
+        GIArgument *in_value = &state.in_arg_cvalues[-2];
+        JS::RootedValue in_js_value(context, JS::ObjectValue(*obj));
+
+        if (!cache->marshal_in(context, cache, &state, in_value, in_js_value))
             return false;
-        ffi_arg_pointers[0] = &state.in_arg_cvalues[-2];
-        ++ffi_arg_pos;
+
+        ffi_arg_pointers[ffi_arg_pos] = in_value;
+        ffi_arg_pos++;
     }
 
     processed_c_args = ffi_arg_pos;
@@ -923,11 +925,16 @@ gjs_invoke_c_function(JSContext                             *context,
 release:
     /* In this loop we use ffi_arg_pos just to ensure we don't release stuff
      * we haven't allocated yet, if we failed in type conversion above.
-     * Because we start from -1 (the return value), we need to process 1 more
-     * than processed_c_args */
+     * If we start from -1 (the return value), we need to process 1 more than
+     * processed_c_args.
+     * If we start from -2 (the instance parameter), we need to process 2 more
+    */
     ffi_arg_pos = is_method ? 1 : 0;
     postinvoke_release_failed = false;
-    for (gi_arg_pos = -1; gi_arg_pos < gi_argc && ffi_arg_pos < (processed_c_args + 1); gi_arg_pos++, 
ffi_arg_pos++) {
+    for (gi_arg_pos = is_method ? -2 : -1;
+         gi_arg_pos < gi_argc &&
+             ffi_arg_pos < (processed_c_args + (is_method ? 2 : 1));
+         gi_arg_pos++, ffi_arg_pos++) {
         GjsArgumentCache *cache = &function->arguments[gi_arg_pos];
         GIArgument *in_value = &state.in_arg_cvalues[gi_arg_pos];
         GIArgument *out_value = &state.out_arg_cvalues[gi_arg_pos];
@@ -945,7 +952,7 @@ release:
     if (postinvoke_release_failed)
         failed = true;
 
-    g_assert_cmpuint(ffi_arg_pos, ==, processed_c_args + 1);
+    g_assert_cmpuint(ffi_arg_pos, ==, processed_c_args + (is_method ? 2 : 1));
 
     if (!failed && did_throw_gerror) {
         gjs_throw_g_error(context, local_error);
@@ -994,12 +1001,18 @@ GJS_NATIVE_CONSTRUCTOR_DEFINE_ABSTRACT(function)
 static void
 uninit_cached_function_data (Function *function)
 {
-    g_clear_pointer(&function->info, g_base_info_unref);
+    /* Careful! function->arguments is one/two inside an array */
+    if (function->arguments) {
+        bool is_method = g_callable_info_is_method(function->info);
 
-    /* Careful! function->arguments is one inside an array */
-    if (function->arguments)
-        g_free(&function->arguments[-1]);
-    function->arguments = nullptr;
+        if (is_method)
+            g_free(&function->arguments[-2]);
+        else
+            g_free(&function->arguments[-1]);
+        function->arguments = nullptr;
+    }
+
+    g_clear_pointer(&function->info, g_base_info_unref);
 
     g_function_invoker_destroy(&function->invoker);
 }
@@ -1184,11 +1197,26 @@ init_cached_function_data (JSContext      *context,
         }
     }
 
+    bool is_method = g_callable_info_is_method(info);
     n_args = g_callable_info_get_n_args(info);
 
-    /* arguments is one inside an array of n_args + 1, so
-     * arguments[-1] is the return value (if any) */
-    GjsArgumentCache *arguments = g_new0(GjsArgumentCache, n_args + 1) + 1;
+    /* arguments is one or two inside an array of n_args + 2, so
+     * arguments[-1] is the return value (which can be skipped if void)
+     * arguments[-2] is the instance parameter */
+    GjsArgumentCache *arguments;
+    if (is_method)
+        arguments = g_new0(GjsArgumentCache, n_args + 2) + 2;
+    else
+        arguments = g_new0(GjsArgumentCache, n_args + 1) + 1;
+
+    if (is_method) {
+        if (!gjs_arg_cache_build_instance(&arguments[-2], info)) {
+            gjs_throw(context, "Function %s.%s cannot be called: the instance parameter is not 
introspectable.",
+                      g_base_info_get_namespace((GIBaseInfo*) info),
+                      g_base_info_get_name((GIBaseInfo*) info));
+            return false;
+        }
+    }
 
     bool inc_counter;
     if (!gjs_arg_cache_build_return(&arguments[-1], arguments,