[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 f60e5771814bc6df03469a0951b6603290f93a01
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 | 50 +++++++++++++++++++++++++++++++++++---------
 gi/arg-cache.h   |  3 +++
 gi/function.cpp  | 63 +++++++++++++++++++++++++++++++++++++++++---------------
 3 files changed, 89 insertions(+), 27 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 3610e27..64a31fd 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -43,6 +43,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.
@@ -550,6 +552,32 @@ 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_pos = -2;
+    self->arg_name = "instance parameter";
+    /* FIXME: 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,
@@ -1289,11 +1317,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;
 
     /* We do some transfer magic later, so let's ensure we don't mess up.
      * Should not happen in practice. */
@@ -1356,14 +1383,14 @@ 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 */
@@ -1388,11 +1415,10 @@ gjs_arg_cache_build_interface_in_arg(GjsArgumentCache *self)
     default:
         /* 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
@@ -1469,8 +1495,12 @@ 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 = g_type_info_get_interface(&self->type_info);
+        bool 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 5ce1e8a..664400a 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -115,6 +115,9 @@ bool gjs_arg_cache_build_return(GjsArgumentCache *self,
                                 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 b341abe..9040cb3 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,11 +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_cvalues[-2], is_object_method))
+        GjsArgumentCache *cache = &function->arguments[-2];
+        GIArgument *in_value = &state.in_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_cvalues[-2];
-        ++ffi_arg_pos;
+
+        ffi_arg_pointers[ffi_arg_pos] = in_value;
+        ffi_arg_pos++;
     }
 
     processed_c_args = ffi_arg_pos;
@@ -931,11 +934,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_cvalues[gi_arg_pos];
         GIArgument *out_value = &state.out_cvalues[gi_arg_pos];
@@ -953,7 +961,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));
 
     /* Return a GIArgument for the first return value, if requested */
     if (r_value && function->js_out_argc > 0 && (!failed && !did_throw_gerror))
@@ -1006,12 +1014,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);
 }
@@ -1196,11 +1210,26 @@ init_cached_function_data (JSContext      *context,
         }
     }
 
+    bool is_method = g_callable_info_is_method(info);
     n_args = g_callable_info_get_n_args((GICallableInfo *) 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,