[gjs/wip/verdre/cache-gtype] Revert "arg-cache: Save space by not caching GType"




commit d526bf8980c5ab9e779ce6e6e74ecaf88c559950
Author: Jonas Dreßler <verdre v0yd nl>
Date:   Thu Oct 22 18:04:03 2020 +0200

    Revert "arg-cache: Save space by not caching GType"
    
    As found in https://gitlab.gnome.org/GNOME/gjs/-/issues/361, getting the
    GType using g_registered_type_info_get_g_type() is very expensive and
    can make up for a significant part of the overhead when calling into a C
    function.
    
    Since the argument cache seems to be fairly small in a typical
    gnome-shell session (about 1300 entries), the total increased memory
    usage of about 10kB seems very reasonable given the benefits of the
    caching.
    
    This reverts commit adfb7dc3ba79b17b0cefa47f5249e9da917217ad.

 gi/arg-cache.cpp | 25 +++++++++++++------------
 gi/arg-cache.h   |  8 ++++++--
 gi/function.cpp  | 10 +++-------
 3 files changed, 22 insertions(+), 21 deletions(-)
---
diff --git a/gi/arg-cache.cpp b/gi/arg-cache.cpp
index 90d8d0ce..fca5836a 100644
--- a/gi/arg-cache.cpp
+++ b/gi/arg-cache.cpp
@@ -10,7 +10,6 @@
 
 #include <ffi.h>
 #include <girepository.h>
-#include <glib-object.h>
 #include <glib.h>
 
 #include <js/Conversions.h>
@@ -579,7 +578,7 @@ static bool gjs_marshal_boxed_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
 
     if (!value.isObject())
         return report_gtype_mismatch(cx, self->arg_name, value, gtype);
@@ -592,7 +591,7 @@ static bool gjs_marshal_boxed_in_in(JSContext* cx, GjsArgumentCache* self,
 
     return BoxedBase::transfer_to_gi_argument(cx, object, arg, GI_DIRECTION_IN,
                                               self->transfer, gtype,
-                                              self->contents.info);
+                                              self->contents.object.info);
 }
 
 // Unions include ClutterEvent and GdkEvent, which occur fairly often in an
@@ -605,7 +604,7 @@ static bool gjs_marshal_union_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
     if (!value.isObject())
@@ -614,7 +613,7 @@ static bool gjs_marshal_union_in_in(JSContext* cx, GjsArgumentCache* self,
     JS::RootedObject object(cx, &value.toObject());
     return UnionBase::transfer_to_gi_argument(cx, object, arg, GI_DIRECTION_IN,
                                               self->transfer, gtype,
-                                              self->contents.info);
+                                              self->contents.object.info);
 }
 
 GJS_JSAPI_RETURN_CONVENTION
@@ -657,7 +656,7 @@ static bool gjs_marshal_gbytes_in_in(JSContext* cx, GjsArgumentCache* self,
     // ownership, so we need to do the same here.
     return BoxedBase::transfer_to_gi_argument(
         cx, object, arg, GI_DIRECTION_IN, GI_TRANSFER_EVERYTHING, G_TYPE_BYTES,
-        self->contents.info);
+        self->contents.object.info);
 }
 
 GJS_JSAPI_RETURN_CONVENTION
@@ -667,7 +666,7 @@ static bool gjs_marshal_interface_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
     if (!value.isObject())
@@ -696,7 +695,7 @@ static bool gjs_marshal_object_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
     if (!value.isObject())
@@ -715,7 +714,7 @@ static bool gjs_marshal_fundamental_in_in(JSContext* cx, GjsArgumentCache* self,
     if (value.isNull())
         return self->handle_nullable(cx, arg);
 
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
     g_assert(gtype != G_TYPE_NONE);
 
     if (!value.isObject())
@@ -970,7 +969,8 @@ static bool gjs_marshal_boxed_in_release(JSContext*, GjsArgumentCache* self,
                                          GjsFunctionCallState*,
                                          GIArgument* in_arg,
                                          GIArgument* out_arg [[maybe_unused]]) {
-    GType gtype = g_registered_type_info_get_g_type(self->contents.info);
+    GType gtype = self->contents.object.gtype;
+
     g_assert(g_type_is_a(gtype, G_TYPE_BOXED));
 
     if (!gjs_arg_get<void*>(in_arg))
@@ -993,7 +993,7 @@ static bool gjs_marshal_gvalue_in_maybe_release(JSContext* cx,
 }
 
 static void gjs_arg_cache_interface_free(GjsArgumentCache* self) {
-    g_clear_pointer(&self->contents.info, g_base_info_unref);
+    g_clear_pointer(&self->contents.object.info, g_base_info_unref);
 }
 
 static const GjsArgumentMarshallers skip_all_marshallers = {
@@ -1383,7 +1383,8 @@ static bool gjs_arg_cache_build_interface_in_arg(JSContext* cx,
         case GI_INFO_TYPE_INTERFACE:
         case GI_INFO_TYPE_UNION: {
             GType gtype = g_registered_type_info_get_g_type(interface_info);
-            self->contents.info = g_base_info_ref(interface_info);
+            self->contents.object.gtype = gtype;
+            self->contents.object.info = g_base_info_ref(interface_info);
 
             // Transfer handling is a bit complex here, because some of our _in
             // marshallers know not to copy stuff if we don't need to.
diff --git a/gi/arg-cache.h b/gi/arg-cache.h
index d6e293df..058d686a 100644
--- a/gi/arg-cache.h
+++ b/gi/arg-cache.h
@@ -11,6 +11,7 @@
 #include <stdint.h>
 
 #include <girepository.h>
+#include <glib-object.h>
 #include <glib.h>  // for g_assert
 
 #include <js/RootingAPI.h>
@@ -64,7 +65,10 @@ struct GjsArgumentCache {
         } number;
 
         // boxed / union / GObject
-        GIRegisteredTypeInfo* info;
+        struct {
+            GType gtype;
+            GIBaseInfo* info;
+        } object;
 
         // foreign structures
         GIStructInfo* tmp_foreign_info;
@@ -144,7 +148,7 @@ struct GjsArgumentCache {
 // if sizeof(GjsArgumentCache) is increased.
 // Note that this check is not applicable for clang-cl builds, as Windows is
 // an LLP64 system
-static_assert(sizeof(GjsArgumentCache) <= 104,
+static_assert(sizeof(GjsArgumentCache) <= 112,
               "Think very hard before increasing the size of GjsArgumentCache. "
               "One is allocated for every argument to every introspected "
               "function.");
diff --git a/gi/function.cpp b/gi/function.cpp
index 5b74fde0..4cc950a1 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -799,13 +799,9 @@ static bool gjs_invoke_c_function(JSContext* context, Function* function,
 
         // Callback lifetimes will be attached to the instance object if it is
         // a GObject or GInterface
-        if (cache->contents.info) {
-            GType gtype =
-                g_registered_type_info_get_g_type(cache->contents.info);
-            if (g_type_is_a(gtype, G_TYPE_OBJECT) ||
-                g_type_is_a(gtype, G_TYPE_INTERFACE))
-                state.instance_object = obj;
-        }
+        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;
     }
 
     unsigned processed_c_args = ffi_arg_pos;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]