GNOME.org
 

[gjs: 3/9] js: Don't convert to UTF-8 just for debug logging

commit 3b9e93a496b78ef895b58cf834bc033f8fadf83d
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Nov 19 16:53:39 2017 -0800

    js: Don't convert to UTF-8 just for debug logging
    
    In most resolve handlers we converted the passed-in jsid to UTF-8 early
    in the function so that we could emit a debug log message first thing.
    Instead, use the new gjs_debug_id() function to log the message without
    doing a UTF-8 conversion, so we can move the relatively expensive UTF-8
    conversion after all the early returns in the resolve handler.

 gi/boxed.cpp       | 87 +++++++++++++++++++++++++-----------------------------
 gi/fundamental.cpp | 16 +++++-----
 gi/interface.cpp   | 12 ++++----
 gi/ns.cpp          | 16 +++++-----
 gi/object.cpp      | 50 +++++++++++++++----------------
 gi/param.cpp       | 13 ++++----
 gi/repo.cpp        | 16 +++++-----
 gi/union.cpp       | 20 ++++++-------
 gjs/importer.cpp   | 17 ++++++-----
 9 files changed, 118 insertions(+), 129 deletions(-)
---
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index 231bd0f4..cd020e44 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -118,67 +118,62 @@ boxed_resolve(JSContext       *context,
               JS::HandleId     id,
               bool            *resolved)
 {
-    Boxed *priv;
-    GjsAutoJSChar name;
-
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true;
-    }
-
-    priv = priv_from_js(context, obj);
-    gjs_debug_jsprop(GJS_DEBUG_GBOXED, "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+    Boxed *priv = priv_from_js(context, obj);
+    gjs_debug_jsprop(GJS_DEBUG_GBOXED, "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_obj(obj).c_str(), priv);
 
     if (priv == nullptr)
         return false; /* wrong class */
 
-    if (priv->gboxed == NULL) {
-        /* We are the prototype, so look for methods and other class properties */
-        GIFunctionInfo *method_info;
-
-        method_info = g_struct_info_find_method((GIStructInfo*) priv->info,
-                                                name);
+    if (priv->gboxed) {
+        /* We are an instance, not a prototype, so look for
+         * per-instance props that we want to define on the
+         * JSObject. Generally we do not want to cache these in JS, we
+         * want to always pull them from the C object, or JS would not
+         * see any changes made from C. So we use the get/set prop
+         * hooks, not this resolve hook.
+         */
+        *resolved = false;
+        return true;
+    }
 
-        if (method_info != NULL) {
-            const char *method_name;
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;
+    }
 
+    /* We are the prototype, so look for methods and other class properties */
+    GIFunctionInfo *method_info = g_struct_info_find_method(priv->info, name);
+    if (!method_info) {
+        *resolved = false;
+        return true;
+    }
 #if GJS_VERBOSE_ENABLE_GI_USAGE
-            _gjs_log_info_usage((GIBaseInfo*) method_info);
+    _gjs_log_info_usage(method_info);
 #endif
-            if (g_function_info_get_flags (method_info) & GI_FUNCTION_IS_METHOD) {
-                method_name = g_base_info_get_name( (GIBaseInfo*) method_info);
-
-                gjs_debug(GJS_DEBUG_GBOXED,
-                          "Defining method %s in prototype for %s.%s",
-                          method_name,
-                          g_base_info_get_namespace( (GIBaseInfo*) priv->info),
-                          g_base_info_get_name( (GIBaseInfo*) priv->info));
-
-                /* obj is the Boxed prototype */
-                if (gjs_define_function(context, obj, priv->gtype,
-                                        (GICallableInfo *)method_info) == NULL) {
-                    g_base_info_unref( (GIBaseInfo*) method_info);
-                    return false;
-                }
 
-                *resolved = true;
-            } else {
-                *resolved = false;
-            }
+    if (g_function_info_get_flags(method_info) & GI_FUNCTION_IS_METHOD) {
+        const char *method_name = g_base_info_get_name(method_info);
+
+        gjs_debug(GJS_DEBUG_GBOXED,
+                  "Defining method %s in prototype for %s.%s",
+                  method_name,
+                  g_base_info_get_namespace( (GIBaseInfo*) priv->info),
+                  g_base_info_get_name( (GIBaseInfo*) priv->info));
 
+        /* obj is the Boxed prototype */
+        if (!gjs_define_function(context, obj, priv->gtype, method_info)) {
             g_base_info_unref( (GIBaseInfo*) method_info);
+            return false;
         }
+
+        *resolved = true;
     } else {
-        /* We are an instance, not a prototype, so look for
-         * per-instance props that we want to define on the
-         * JSObject. Generally we do not want to cache these in JS, we
-         * want to always pull them from the C object, or JS would not
-         * see any changes made from C. So we use the get/set prop
-         * hooks, not this resolve hook.
-         */
         *resolved = false;
     }
+
+    g_base_info_unref(method_info);
     return true;
 }
 
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 282394bc..44502897 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -301,17 +301,11 @@ fundamental_instance_resolve(JSContext       *context,
                              bool            *resolved)
 {
     FundamentalInstance *priv;
-    GjsAutoJSChar name;
-
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true; /* not resolved, but no error */
-    }
 
     priv = priv_from_js(context, obj);
     gjs_debug_jsprop(GJS_DEBUG_GFUNDAMENTAL,
-                     "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+                     "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
 
     if (priv == nullptr)
         return false; /* wrong class */
@@ -328,6 +322,12 @@ fundamental_instance_resolve(JSContext       *context,
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true; /* not resolved, but no error */
+    }
+
     /* We are the prototype, so look for methods and other class properties */
     Fundamental *proto_priv = (Fundamental *) priv;
     GIFunctionInfo *method_info;
diff --git a/gi/interface.cpp b/gi/interface.cpp
index 6394f952..9ce08482 100644
--- a/gi/interface.cpp
+++ b/gi/interface.cpp
@@ -113,14 +113,8 @@ interface_resolve(JSContext       *context,
                   bool            *resolved)
 {
     Interface *priv;
-    GjsAutoJSChar name;
     GIFunctionInfo *method_info;
 
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true;
-    }
-
     priv = priv_from_js(context, obj);
 
     if (priv == nullptr)
@@ -134,6 +128,12 @@ interface_resolve(JSContext       *context,
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;
+    }
+
     method_info = g_interface_info_find_method((GIInterfaceInfo*) priv->info, name);
 
     if (method_info != NULL) {
diff --git a/gi/ns.cpp b/gi/ns.cpp
index 99ee4eea..f80862f7 100644
--- a/gi/ns.cpp
+++ b/gi/ns.cpp
@@ -69,22 +69,22 @@ ns_resolve(JSContext       *context,
         return true;
     }
 
-    GjsAutoJSChar name;
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true;  /* not resolved, but no error */
-    }
-
     priv = priv_from_js(context, obj);
     gjs_debug_jsprop(GJS_DEBUG_GNAMESPACE,
-                     "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+                     "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
 
     if (priv == NULL) {
         *resolved = false;  /* we are the prototype, or have the wrong class */
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;  /* not resolved, but no error */
+    }
+
     repo = g_irepository_get_default();
 
     info = g_irepository_find_by_name(repo, priv->gi_namespace, name);
diff --git a/gi/object.cpp b/gi/object.cpp
index 6d4e1bfe..81660742 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -438,16 +438,10 @@ object_instance_get_prop(JSContext              *context,
                          JS::HandleId            id,
                          JS::MutableHandleValue  value_p)
 {
-    ObjectInstance *priv;
-    GjsAutoJSChar name;
-
-    if (!gjs_get_string_id(context, id, &name))
-        return true; /* not resolved, but no error */
-
-    priv = priv_from_js(context, obj);
+    ObjectInstance *priv = priv_from_js(context, obj);
     gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
-                     "Get prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+                     "Get prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
 
     if (priv == nullptr)
         /* If we reach this point, either object_instance_new_resolve
@@ -468,6 +462,10 @@ object_instance_get_prop(JSContext              *context,
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name))
+        return true; /* not resolved, but no error */
+
     if (!get_prop_from_g_param(context, obj, priv, name, value_p))
         return false;
 
@@ -562,11 +560,15 @@ object_instance_set_prop(JSContext              *context,
                          JS::ObjectOpResult&     result)
 {
     ObjectInstance *priv;
-    GjsAutoJSChar name;
     bool ret = true;
     bool g_param_was_set = false;
 
     priv = priv_from_js(context, obj);
+
+    gjs_debug_jsprop(GJS_DEBUG_GOBJECT, "Set prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(),
+                     priv);
+
     if (priv == nullptr)
         /* see the comment in object_instance_get_prop() on this */
         return result.succeed();
@@ -584,6 +586,7 @@ object_instance_set_prop(JSContext              *context,
         return result.succeed();
     }
 
+    GjsAutoJSChar name;
     if (!gjs_get_string_id(context, id, &name)) {
         /* We need to keep the wrapper alive in order not to lose custom
          * "expando" properties. In this case if gjs_get_string_id() is false
@@ -592,10 +595,6 @@ object_instance_set_prop(JSContext              *context,
         return result.succeed();  /* not resolved, but no error */
     }
 
-    gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
-                     "Set prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
-
     ret = set_g_param_from_prop(context, priv, name, g_param_was_set, value_p, result);
     if (g_param_was_set || !ret)
         return ret;
@@ -792,20 +791,12 @@ object_instance_resolve(JSContext       *context,
                         bool            *resolved)
 {
     GIFunctionInfo *method_info;
-    ObjectInstance *priv;
-    GjsAutoJSChar name;
-
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true; /* not resolved, but no error */
-    }
-
-    priv = priv_from_js(context, obj);
+    ObjectInstance *priv = priv_from_js(context, obj);
 
     gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
-                     "Resolve prop '%s' hook obj %p priv %p (%s.%s) gobj %p %s",
-                     name.get(),
-                     obj.get(),
+                     "Resolve prop '%s' hook, obj %s, priv %p (%s.%s), gobj %p %s",
+                     gjs_debug_id(id).c_str(),
+                     gjs_debug_object(obj).c_str(),
                      priv,
                      priv && priv->info ? g_base_info_get_namespace (priv->info) : "",
                      priv && priv->info ? g_base_info_get_name (priv->info) : "",
@@ -837,11 +828,16 @@ object_instance_resolve(JSContext       *context,
                    priv->info ? g_base_info_get_name( (GIBaseInfo*) priv->info) : g_type_name(priv->gtype),
                    priv->gobj);
         gjs_dumpstack();
-
         *resolved = false;
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;  /* not resolved, but no error */
+    }
+
     /* If we have no GIRepository information (we're a JS GObject subclass),
      * we need to look at exposing interfaces. Look up our interfaces through
      * GType data, and then hope that *those* are introspectable. */
diff --git a/gi/param.cpp b/gi/param.cpp
index 8f3f31ed..92a319a4 100644
--- a/gi/param.cpp
+++ b/gi/param.cpp
@@ -58,22 +58,21 @@ param_resolve(JSContext       *context,
     GIObjectInfo *info = NULL;
     GIFunctionInfo *method_info;
     Param *priv;
-    GjsAutoJSChar name;
     bool ret = false;
 
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true; /* not resolved, but no error */
-    }
-
     priv = priv_from_js(context, obj);
-
     if (priv != NULL) {
         /* instance, not prototype */
         *resolved = false;
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true; /* not resolved, but no error */
+    }
+
     info = (GIObjectInfo*)g_irepository_find_by_gtype(g_irepository_get_default(), G_TYPE_PARAM);
     method_info = g_object_info_find_method(info, name);
 
diff --git a/gi/repo.cpp b/gi/repo.cpp
index 4714f62a..d819dcaa 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -175,15 +175,9 @@ repo_resolve(JSContext       *context,
         return true;
     }
 
-    GjsAutoJSChar name;
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true;
-    }
-
     priv = priv_from_js(context, obj);
-    gjs_debug_jsprop(GJS_DEBUG_GREPO, "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+    gjs_debug_jsprop(GJS_DEBUG_GREPO, "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
 
     if (priv == NULL) {
         /* we are the prototype, or have the wrong class */
@@ -191,6 +185,12 @@ repo_resolve(JSContext       *context,
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;
+    }
+
     if (!resolve_namespace_object(context, obj, id, name)) {
         return false;
     }
diff --git a/gi/union.cpp b/gi/union.cpp
index 9896cc2f..9ed83015 100644
--- a/gi/union.cpp
+++ b/gi/union.cpp
@@ -60,17 +60,9 @@ union_resolve(JSContext       *context,
               JS::HandleId     id,
               bool            *resolved)
 {
-    Union *priv;
-    GjsAutoJSChar name;
-
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true; /* not resolved, but no error */
-    }
-
-    priv = priv_from_js(context, obj);
-    gjs_debug_jsprop(GJS_DEBUG_GBOXED, "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+    Union *priv = priv_from_js(context, obj);
+    gjs_debug_jsprop(GJS_DEBUG_GBOXED, "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
 
     if (priv == nullptr)
         return false; /* wrong class */
@@ -87,6 +79,12 @@ union_resolve(JSContext       *context,
         return true;
     }
 
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true; /* not resolved, but no error */
+    }
+
     /* We are the prototype, so look for methods and other class properties */
     GIFunctionInfo *method_info;
 
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index f3dc9ca1..7b0fbae7 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -731,17 +731,11 @@ importer_resolve(JSContext        *context,
         return true;
     }
 
-    GjsAutoJSChar name;
-    if (!gjs_get_string_id(context, id, &name)) {
-        *resolved = false;
-        return true;
-    }
-
     priv = priv_from_js(context, obj);
 
     gjs_debug_jsprop(GJS_DEBUG_IMPORTER,
-                     "Resolve prop '%s' hook obj %p priv %p",
-                     name.get(), obj.get(), priv);
+                     "Resolve prop '%s' hook, obj %s, priv %p",
+                     gjs_debug_id(id).c_str(), gjs_debug_object(obj).c_str(), priv);
     if (!priv) {
         /* we are the prototype, or have the wrong class */
         *resolved = false;
@@ -749,6 +743,13 @@ importer_resolve(JSContext        *context,
     }
 
     JSAutoRequest ar(context);
+
+    GjsAutoJSChar name;
+    if (!gjs_get_string_id(context, id, &name)) {
+        *resolved = false;
+        return true;
+    }
+
     if (!do_import(context, obj, priv, id, name))
         return false;
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]