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

[Philip Chimento <pchimento src gnome org>]

commit a1fada5ae4b822cf3599606fa319a222d78770b2
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.
    
    (Philip Chimento: Rebased and fixed coding style and bugs.)

 gi/arg-cache.cpp |  95 +++++++++++++++++++++++++++++++++
 gi/arg-cache.h   |   4 ++
 gi/function.cpp  | 156 +++++++++++--------------------------------------------
 3 files changed, 130 insertions(+), 125 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index d2eb4d45..cacf8a34 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -48,6 +48,7 @@
 #include "gi/gerror.h"
 #include "gi/gtype.h"
 #include "gi/object.h"
+#include "gi/param.h"
 #include "gi/union.h"
 #include "gi/value.h"
 #include "gjs/byteArray.h"
@@ -706,6 +707,58 @@ static bool gjs_marshal_object_in_in(JSContext* cx, GjsArgumentCache* self,
                                                self->transfer, gtype);
 }
 
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_marshal_gtype_struct_instance_in(JSContext* cx,
+                                                 GjsArgumentCache* self,
+                                                 GjsFunctionCallState*,
+                                                 GIArgument* arg,
+                                                 JS::HandleValue value) {
+    // Instance parameter is never nullable
+    if (!value.isObject())
+        return report_primitive_type_mismatch(cx, self, value, JSTYPE_OBJECT);
+
+    JS::RootedObject obj(cx, &value.toObject());
+    GType actual_gtype;
+    if (!gjs_gtype_get_actual_gtype(cx, obj, &actual_gtype))
+        return false;
+
+    if (actual_gtype == G_TYPE_NONE) {
+        gjs_throw(cx, "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))
+        arg->v_pointer = g_type_default_interface_peek(actual_gtype);
+    else
+        arg->v_pointer = g_type_class_peek(actual_gtype);
+
+    return true;
+}
+
+GJS_JSAPI_RETURN_CONVENTION
+static bool gjs_marshal_param_instance_in(JSContext* cx, GjsArgumentCache* self,
+                                          GjsFunctionCallState*,
+                                          GIArgument* arg,
+                                          JS::HandleValue value) {
+    // Instance parameter is never nullable
+    if (!value.isObject())
+        return report_primitive_type_mismatch(cx, self, value, JSTYPE_OBJECT);
+
+    JS::RootedObject obj(cx, &value.toObject());
+    if (!gjs_typecheck_param(cx, obj, G_TYPE_PARAM, true))
+        return false;
+    arg->v_pointer = gjs_g_param_from_param(cx, obj);
+
+    if (self->transfer == GI_TRANSFER_EVERYTHING)
+        g_param_spec_ref(static_cast<GParamSpec*>(arg->v_pointer));
+
+    return true;
+}
+
 GJS_JSAPI_RETURN_CONVENTION
 static bool gjs_marshal_skipped_out(JSContext*, GjsArgumentCache*,
                                     GjsFunctionCallState*, GIArgument*,
@@ -1240,6 +1293,48 @@ static bool gjs_arg_cache_build_normal_in_arg(JSContext* cx,
     return true;
 }
 
+bool gjs_arg_cache_build_instance(JSContext* cx, GjsArgumentCache* self,
+                                  GICallableInfo* callable) {
+    GIBaseInfo* interface_info = g_base_info_get_container(callable);  // !owned
+
+    self->arg_pos = -2;
+    self->arg_name = "instance parameter";
+    self->transfer = g_callable_info_get_instance_ownership_transfer(callable);
+    // 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;
+
+    // These cases could be covered by the generic marshaller, except that
+    // there's no way to get GITypeInfo for a method's instance parameter.
+    // Instead, special-case the arguments here that would otherwise go through
+    // the generic marshaller.
+    // See: https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/334
+    GIInfoType info_type = g_base_info_get_type(interface_info);
+    if (info_type == GI_INFO_TYPE_STRUCT &&
+        g_struct_info_is_gtype_struct(interface_info)) {
+        self->marshal_in = gjs_marshal_gtype_struct_instance_in;
+        self->release = gjs_marshal_skipped_release;
+        return true;
+    }
+    if (info_type == GI_INFO_TYPE_OBJECT) {
+        GType gtype = g_registered_type_info_get_g_type(interface_info);
+
+        if (g_type_is_a(gtype, G_TYPE_PARAM)) {
+            self->marshal_in = gjs_marshal_param_instance_in;
+            self->release = gjs_marshal_skipped_release;
+            return true;
+        }
+    }
+
+    bool ok = gjs_arg_cache_build_interface_in_arg(cx, self, callable,
+                                                   interface_info);
+    // Don't set marshal_out, it should not be called for the instance
+    // parameter
+    self->release = gjs_marshal_skipped_release;
+    return ok;
+}
+
 bool gjs_arg_cache_build_arg(JSContext* cx, GjsArgumentCache* self,
                              GjsArgumentCache* arguments, int gi_index,
                              GIDirection direction, GIArgInfo* arg,
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index 25629fd4..b4807302 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -115,4 +115,8 @@ bool gjs_arg_cache_build_return(JSContext* cx, GjsArgumentCache* self,
                                 GICallableInfo* callable,
                                 bool* inc_counter_out);
 
+GJS_JSAPI_RETURN_CONVENTION
+bool gjs_arg_cache_build_instance(JSContext* cx, GjsArgumentCache* self,
+                                  GICallableInfo* callable);
+
 #endif  // GI_ARG_CACHE_H_
diff --git a/gi/function.cpp b/gi/function.cpp
index d7ee152c..3e59d48c 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -44,21 +44,16 @@
 #include <js/Realm.h>  // for GetRealmFunctionPrototype
 #include <js/RootingAPI.h>
 #include <js/TypeDecls.h>
+#include <js/Value.h>
 #include <js/Warnings.h>
 #include <jsapi.h>        // for HandleValueArray, JS_GetElement
-#include <jspubtd.h>      // for JSProto_TypeError, JSTYPE_FUNCTION
 
 #include "gi/arg-cache.h"
 #include "gi/arg.h"
-#include "gi/boxed.h"
 #include "gi/closure.h"
 #include "gi/function.h"
-#include "gi/fundamental.h"
 #include "gi/gerror.h"
-#include "gi/gtype.h"
 #include "gi/object.h"
-#include "gi/param.h"
-#include "gi/union.h"
 #include "gjs/context-private.h"
 #include "gjs/context.h"
 #include "gjs/jsapi-class.h"
@@ -597,110 +592,6 @@ GjsCallbackTrampoline* gjs_callback_trampoline_new(
     return trampoline;
 }
 
-GJS_JSAPI_RETURN_CONVENTION
-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 (!ErrorBase::transfer_to_gi_argument(context, obj, out_arg,
-                                                    GI_DIRECTION_OUT, transfer))
-                return false;
-        } 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;
-
-            if (!gjs_gtype_get_actual_gtype(context, obj, &actual_gtype))
-                return false;
-
-            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 (!BoxedBase::transfer_to_gi_argument(context, obj, out_arg,
-                                                    GI_DIRECTION_OUT, transfer,
-                                                    gtype, container))
-                return false;
-        }
-
-    } else if (type == GI_INFO_TYPE_UNION) {
-        if (!UnionBase::transfer_to_gi_argument(context, obj, out_arg,
-                                                GI_DIRECTION_OUT, transfer,
-                                                gtype, container))
-            return false;
-
-    } else if (type == GI_INFO_TYPE_OBJECT || type == GI_INFO_TYPE_INTERFACE) {
-        if (g_type_is_a(gtype, G_TYPE_OBJECT)) {
-            if (!ObjectBase::transfer_to_gi_argument(
-                    context, obj, out_arg, GI_DIRECTION_OUT, transfer, gtype))
-                return false;
-            is_gobject = true;
-        } 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 (ObjectBase::check_jsclass(context, obj)) {
-                if (!ObjectBase::transfer_to_gi_argument(context, obj, out_arg,
-                                                         GI_DIRECTION_OUT,
-                                                         transfer, gtype))
-                    return false;
-                is_gobject = true;
-            } else {
-                if (!FundamentalBase::transfer_to_gi_argument(
-                        context, obj, out_arg, GI_DIRECTION_OUT, transfer,
-                        gtype))
-                    return false;
-            }
-        } else if (G_TYPE_IS_INSTANTIATABLE(gtype)) {
-            if (!FundamentalBase::transfer_to_gi_argument(
-                    context, obj, out_arg, GI_DIRECTION_OUT, transfer, gtype))
-                return false;
-        } 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 */
 GJS_USE
 static char* format_function_name(Function* function) {
@@ -750,7 +641,6 @@ static bool gjs_invoke_c_function(JSContext* context, Function* function,
     bool failed, postinvoke_release_failed;
 
     bool is_method;
-    bool is_object_method = false;
     GITypeTag return_tag;
     JS::RootedValueVector return_values(context);
 
@@ -833,15 +723,20 @@ static bool gjs_invoke_c_function(JSContext* context, Function* function,
         return false;
 
     if (is_method) {
+        GjsArgumentCache* cache = &function->arguments[-2];
         GIArgument* in_value = &state.in_cvalues[-2];
-        if (!gjs_fill_method_instance(context, obj, function, in_value,
-                                      is_object_method))
+        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[ffi_arg_pos] = in_value;
         ++ffi_arg_pos;
 
-        if (is_object_method)
+        // Callback lifetimes will be attached to the instance object if it is
+        // a GObject or GInterface
+        if (g_type_is_a(cache->contents.object.gtype, G_TYPE_OBJECT) ||
+            g_type_is_a(cache->contents.object.gtype, G_TYPE_INTERFACE))
             state.instance_object = obj;
     }
 
@@ -986,12 +881,14 @@ 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);
+    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];
@@ -1022,7 +919,7 @@ release:
     if (postinvoke_release_failed)
         failed = true;
 
-    g_assert(ffi_arg_pos == processed_c_args + 1);
+    g_assert(ffi_arg_pos == processed_c_args + (is_method ? 2 : 1));
 
     if (!r_value && function->js_out_argc > 0 && (!failed && !did_throw_gerror)) {
         // If we have one return value or out arg, return that item on its
@@ -1081,15 +978,16 @@ uninit_cached_function_data (Function *function)
 {
     g_assert(function->info && "Don't know how to free cache without GI info");
 
-    // Careful! function->arguments is one inside an array
+    // Careful! function->arguments is one/two inside an array
     if (function->arguments) {
+        int start_index = g_callable_info_is_method(function->info) ? -2 : -1;
         int gi_argc = g_callable_info_get_n_args(function->info);
-        for (int ix = -1; ix < gi_argc; ix++) {
+        for (int ix = start_index; ix < gi_argc; ix++) {
             if (function->arguments[ix].free)
                 function->arguments[ix].free(&function->arguments[ix]);
         }
 
-        g_free(&function->arguments[-1]);
+        g_free(&function->arguments[start_index]);
         function->arguments = nullptr;
     }
 
@@ -1247,11 +1145,19 @@ 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
+    size_t offset = is_method ? 2 : 1;
+    GjsArgumentCache* arguments =
+        g_new0(GjsArgumentCache, n_args + offset) + offset;
+
+    if (is_method &&
+        !gjs_arg_cache_build_instance(context, &arguments[-2], info))
+        return false;
 
     bool inc_counter;
     if (!gjs_arg_cache_build_return(context, &arguments[-1], arguments, info,