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

[Philip Chimento <pchimento src gnome org>]

commit b6939aa2d2518d4a1ec862ea39e7a6f28c46225c
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  | 182 ++++++++++++++-----------------------------------------
 3 files changed, 88 insertions(+), 147 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 212858d6..43980bc4 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,
@@ -1284,11 +1312,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. */
@@ -1351,14 +1378,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 */
@@ -1383,11 +1410,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
@@ -1464,8 +1490,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 5ce1e8a3..664400ab 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 931c1192..48d5ea3b 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -555,127 +555,6 @@ gjs_callback_trampoline_new(JSContext       *context,
     return trampoline;
 }
 
-static bool
-gjs_fill_method_instance(JSContext       *context,
-                         JS::HandleObject obj,
-                         Function        *function,
-                         GIArgument      *out_arg,
-                         bool&            is_gobject)
-{
-    GIBaseInfo *container = g_base_info_get_container((GIBaseInfo *) function->info);
-    GIInfoType type = g_base_info_get_type(container);
-    GType gtype = g_registered_type_info_get_g_type ((GIRegisteredTypeInfo *)container);
-    GITransfer transfer = g_callable_info_get_instance_ownership_transfer (function->info);
-
-    is_gobject = false;
-
-    if (type == GI_INFO_TYPE_STRUCT || type == GI_INFO_TYPE_BOXED) {
-        /* GError must be special cased */
-        if (g_type_is_a(gtype, G_TYPE_ERROR)) {
-            if (!gjs_typecheck_gerror(context, obj, true))
-                return false;
-
-            out_arg->v_pointer = gjs_gerror_from_error(context, obj);
-            if (transfer == GI_TRANSFER_EVERYTHING)
-                out_arg->v_pointer = g_error_copy ((GError*) out_arg->v_pointer);
-        } else if (type == GI_INFO_TYPE_STRUCT &&
-                   g_struct_info_is_gtype_struct((GIStructInfo*) container)) {
-            /* And so do GType structures */
-            GType actual_gtype;
-            gpointer klass;
-
-            actual_gtype = gjs_gtype_get_actual_gtype(context, obj);
-
-            if (actual_gtype == G_TYPE_NONE) {
-                gjs_throw(context, "Invalid GType class passed for instance parameter");
-                return false;
-            }
-
-            /* We use peek here to simplify reference counting (we just ignore
-               transfer annotation, as GType classes are never really freed)
-               We know that the GType class is referenced at least once when
-               the JS constructor is initialized.
-            */
-
-            if (g_type_is_a(actual_gtype, G_TYPE_INTERFACE))
-                klass = g_type_default_interface_peek(actual_gtype);
-            else
-                klass = g_type_class_peek(actual_gtype);
-
-            out_arg->v_pointer = klass;
-        } else {
-            if (!gjs_typecheck_boxed(context, obj, container, gtype, true))
-                return false;
-
-            out_arg->v_pointer = gjs_c_struct_from_boxed(context, obj);
-            if (transfer == GI_TRANSFER_EVERYTHING) {
-                if (gtype != G_TYPE_NONE)
-                    out_arg->v_pointer = g_boxed_copy (gtype, out_arg->v_pointer);
-                else {
-                    gjs_throw (context, "Cannot transfer ownership of instance argument for non boxed 
structure");
-                    return false;
-                }
-            }
-        }
-
-    } else if (type == GI_INFO_TYPE_UNION) {
-        if (!gjs_typecheck_union(context, obj, container, gtype, true))
-            return false;
-
-        out_arg->v_pointer = gjs_c_union_from_union(context, obj);
-        if (transfer == GI_TRANSFER_EVERYTHING)
-            out_arg->v_pointer = g_boxed_copy (gtype, out_arg->v_pointer);
-
-    } else if (type == GI_INFO_TYPE_OBJECT || type == GI_INFO_TYPE_INTERFACE) {
-        if (g_type_is_a(gtype, G_TYPE_OBJECT)) {
-            if (!gjs_typecheck_object(context, obj, gtype, true))
-                return false;
-            out_arg->v_pointer = gjs_g_object_from_object(context, obj);
-            is_gobject = true;
-            if (transfer == GI_TRANSFER_EVERYTHING)
-                g_object_ref (out_arg->v_pointer);
-        } else if (g_type_is_a(gtype, G_TYPE_PARAM)) {
-            if (!gjs_typecheck_param(context, obj, G_TYPE_PARAM, true))
-                return false;
-            out_arg->v_pointer = gjs_g_param_from_param(context, obj);
-            if (transfer == GI_TRANSFER_EVERYTHING)
-                g_param_spec_ref ((GParamSpec*) out_arg->v_pointer);
-        } else if (G_TYPE_IS_INTERFACE(gtype)) {
-            if (gjs_typecheck_is_object(context, obj, false)) {
-                if (!gjs_typecheck_object(context, obj, gtype, true))
-                    return false;
-                out_arg->v_pointer = gjs_g_object_from_object(context, obj);
-                is_gobject = true;
-                if (transfer == GI_TRANSFER_EVERYTHING)
-                    g_object_ref (out_arg->v_pointer);
-            } else {
-                if (!gjs_typecheck_fundamental(context, obj, gtype, true))
-                    return false;
-                out_arg->v_pointer = gjs_g_fundamental_from_object(context, obj);
-                if (transfer == GI_TRANSFER_EVERYTHING)
-                    gjs_fundamental_ref (context, out_arg->v_pointer);
-            }
-        } else if (G_TYPE_IS_INSTANTIATABLE(gtype)) {
-            if (!gjs_typecheck_fundamental(context, obj, gtype, true))
-                return false;
-            out_arg->v_pointer = gjs_g_fundamental_from_object(context, obj);
-            if (transfer == GI_TRANSFER_EVERYTHING)
-                gjs_fundamental_ref (context, out_arg->v_pointer);
-        } else {
-            gjs_throw_custom(context, JSProto_TypeError, nullptr,
-                             "%s.%s is not an object instance neither a fundamental instance of a supported 
type",
-                             g_base_info_get_namespace(container),
-                             g_base_info_get_name(container));
-            return false;
-        }
-
-    } else {
-        g_assert_not_reached();
-    }
-
-    return true;
-}
-
 /* Intended for error messages. Return value must be freed */
 static char *
 format_function_name(Function *function,
@@ -721,7 +600,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,10 +684,14 @@ 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_pointers[ffi_arg_pos] = in_value;
         ++ffi_arg_pos;
     }
 
@@ -931,11 +813,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 +840,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 +893,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 +1089,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,