GNOME.org
 

[gjs] Get rid of gjs_object_get_property and gjs_object_has_property

commit 4601f36e9cb4530b51bc16a93692590ba810db3e
Author: Giovanni Campagna <gcampagna src gnome org>
Date:   Mon Apr 8 22:13:03 2013 +0200

    Get rid of gjs_object_get_property and gjs_object_has_property
    
    They needlessly save and restore the exception state, which is
    expensive. Callers that need it should implement it themselves,
    but they don't really, so just have them call JS_GetProperty
    directly.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=697592

 gi/arg.c               |   25 ++++++++++++++-----
 gi/boxed.c             |    4 ++-
 gi/enumeration.c       |    4 ++-
 gi/function.c          |    7 +++--
 gi/gerror.c            |    4 ++-
 gi/interface.c         |    3 +-
 gi/keep-alive.c        |    7 +++--
 gi/ns.c                |    7 +++--
 gi/object.c            |    7 ++++-
 gi/param.c             |    3 +-
 gi/repo.c              |   25 +++++++++++--------
 gi/union.c             |    4 ++-
 gi/value.c             |   10 +++++--
 gjs/importer.c         |   29 ++++++++++++++--------
 gjs/jsapi-util-error.c |    5 ++-
 gjs/jsapi-util.c       |   60 +++++++----------------------------------------
 gjs/jsapi-util.h       |   12 ++-------
 17 files changed, 106 insertions(+), 110 deletions(-)
---
diff --git a/gi/arg.c b/gi/arg.c
index 90a0bee..dd6e85e 100644
--- a/gi/arg.c
+++ b/gi/arg.c
@@ -35,6 +35,7 @@
 #include "gjs/byteArray.h"
 #include <gjs/gjs-module.h>
 #include <gjs/compat.h>
+#include <gjs/runtime.h>
 
 #include <util/log.h>
 
@@ -1081,6 +1082,8 @@ gjs_array_to_explicit_array_internal(JSContext       *context,
 {
     JSBool ret = JS_FALSE;
     GITypeInfo *param_info;
+    jsid length_name;
+    JSBool found_length;
 
     param_info = g_type_info_get_param_type(type_info, 0);
 
@@ -1090,6 +1093,9 @@ gjs_array_to_explicit_array_internal(JSContext       *context,
         goto out;
     }
 
+    length_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
+                                               GJS_STRING_LENGTH);
+
     if (JSVAL_IS_NULL(value)) {
         *contents = NULL;
         *length_p = 0;
@@ -1098,9 +1104,8 @@ gjs_array_to_explicit_array_internal(JSContext       *context,
         if (!gjs_string_to_intarray(context, value, param_info,
                                     contents, length_p))
             goto out;
-    } else if (gjs_object_has_property(context,
-                                       JSVAL_TO_OBJECT(value),
-                                       "length")) {
+    } else if (JS_HasPropertyById(context, JSVAL_TO_OBJECT(value), length_name, &found_length) &&
+               found_length) {
         jsval length_value;
         guint32 length;
 
@@ -1567,16 +1572,21 @@ gjs_value_to_g_argument(JSContext      *context,
         break;
 
     case GI_TYPE_TAG_GLIST:
-    case GI_TYPE_TAG_GSLIST:
+    case GI_TYPE_TAG_GSLIST: {
+        jsid length_name;
+        JSBool found_length;
+
+        length_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
+                                                   GJS_STRING_LENGTH);
+
         /* nullable_type=FALSE; while a list can be NULL in C, that
          * means empty array in JavaScript, it doesn't mean null in
          * JavaScript.
          */
         if (!JSVAL_IS_NULL(value) &&
             JSVAL_IS_OBJECT(value) &&
-            gjs_object_has_property(context,
-                                    JSVAL_TO_OBJECT(value),
-                                    "length")) {
+            JS_HasPropertyById(context, JSVAL_TO_OBJECT(value), length_name, &found_length) &&
+            found_length) {
             jsval length_value;
             guint32 length;
 
@@ -1620,6 +1630,7 @@ gjs_value_to_g_argument(JSContext      *context,
             report_type_mismatch = TRUE;
         }
         break;
+    }
 
     case GI_TYPE_TAG_GHASH:
         if (JSVAL_IS_NULL(value)) {
diff --git a/gi/boxed.c b/gi/boxed.c
index 5ce62e9..7c5280d 100644
--- a/gi/boxed.c
+++ b/gi/boxed.c
@@ -1186,7 +1186,9 @@ gjs_define_boxed_class(JSContext    *context,
 
     constructor_name = g_base_info_get_name( (GIBaseInfo*) info);
 
-    if (gjs_object_get_property(context, in_object, constructor_name, &value)) {
+    if (!JS_GetProperty(context, in_object, constructor_name, &value))
+        return JS_FALSE;
+    if (!JSVAL_IS_VOID(value)) {
         JSObject *constructor;
         jsid prototype_name;
 
diff --git a/gi/enumeration.c b/gi/enumeration.c
index 67c81db..76de0be 100644
--- a/gi/enumeration.c
+++ b/gi/enumeration.c
@@ -157,7 +157,9 @@ gjs_define_enumeration(JSContext    *context,
 
     enum_name = g_base_info_get_name( (GIBaseInfo*) info);
 
-    if (gjs_object_get_property(context, in_object, enum_name, &value)) {
+    if (!JS_GetProperty(context, in_object, enum_name, &value))
+        return JS_FALSE;
+    if (!JSVAL_IS_VOID(value)) {
         if (!JSVAL_IS_OBJECT(value)) {
             gjs_throw(context, "Existing property '%s' does not look like an enum object",
                       enum_name);
diff --git a/gi/function.c b/gi/function.c
index cd6e628..67a815e 100644
--- a/gi/function.c
+++ b/gi/function.c
@@ -1542,11 +1542,14 @@ function_new(JSContext      *context,
     JSObject *function;
     JSObject *global;
     Function *priv;
+    JSBool found;
 
     /* put constructor for GIRepositoryFunction() in the global namespace */
     global = gjs_get_import_global(context);
 
-    if (!gjs_object_has_property(context, global, gjs_function_class.name)) {
+    if (!JS_HasProperty(context, global, gjs_function_class.name, &found))
+        return NULL;
+    if (!found) {
         JSObject *prototype;
         JSObject *parent_proto;
         jsval native_function;
@@ -1580,8 +1583,6 @@ function_new(JSContext      *context,
         if (prototype == NULL)
             gjs_fatal("Can't init class %s", gjs_function_class.name);
 
-        g_assert(gjs_object_has_property(context, global, gjs_function_class.name));
-
         gjs_debug(GJS_DEBUG_GFUNCTION, "Initialized class %s prototype %p",
                   gjs_function_class.name, prototype);
     }
diff --git a/gi/gerror.c b/gi/gerror.c
index e8fda70..2187d3d 100644
--- a/gi/gerror.c
+++ b/gi/gerror.c
@@ -416,7 +416,9 @@ gjs_define_error_class(JSContext    *context,
 
     constructor_name = g_base_info_get_name( (GIBaseInfo*) info);
 
-    if (gjs_object_get_property(context, in_object, constructor_name, &value)) {
+    if (!JS_GetProperty(context, in_object, constructor_name, &value))
+        return JS_FALSE;
+    if (!JSVAL_IS_VOID(value)) {
         JSObject *constructor;
         jsid prototype_name;
 
diff --git a/gi/interface.c b/gi/interface.c
index a63962a..7188999 100644
--- a/gi/interface.c
+++ b/gi/interface.c
@@ -181,7 +181,8 @@ gjs_define_interface_class(JSContext       *context,
 
     constructor_name = g_base_info_get_name((GIBaseInfo*)info);
 
-    gjs_object_get_property(context, in_object, constructor_name, &value);
+    if (!JS_GetProperty(context, in_object, constructor_name, &value))
+        return JS_FALSE;
     if (!JSVAL_IS_VOID(value)) {
         JSObject *constructor;
         jsid prototype_name;
diff --git a/gi/keep-alive.c b/gi/keep-alive.c
index 05c7ff1..f008448 100644
--- a/gi/keep-alive.c
+++ b/gi/keep-alive.c
@@ -181,6 +181,7 @@ gjs_keep_alive_new(JSContext *context)
     KeepAlive *priv;
     JSObject *keep_alive;
     JSObject *global;
+    JSBool found;
 
     /* This function creates an unattached KeepAlive object; following our
      * general strategy, we have a single KeepAlive class with a constructor
@@ -196,7 +197,9 @@ gjs_keep_alive_new(JSContext *context)
 
     g_assert(global != NULL);
 
-    if (!gjs_object_has_property(context, global, gjs_keep_alive_class.name)) {
+    if (!JS_HasProperty(context, global, gjs_keep_alive_class.name, &found))
+        return JS_FALSE;
+    if (!found) {
         JSObject *prototype;
 
         gjs_debug(GJS_DEBUG_KEEP_ALIVE,
@@ -228,8 +231,6 @@ gjs_keep_alive_new(JSContext *context)
         if (prototype == NULL)
             gjs_fatal("Can't init class %s", gjs_keep_alive_class.name);
 
-        g_assert(gjs_object_has_property(context, global, gjs_keep_alive_class.name));
-
         gjs_debug(GJS_DEBUG_KEEP_ALIVE, "Initialized class %s prototype %p",
                   gjs_keep_alive_class.name, prototype);
     }
diff --git a/gi/ns.c b/gi/ns.c
index cfa9413..3ec7e5a 100644
--- a/gi/ns.c
+++ b/gi/ns.c
@@ -184,11 +184,14 @@ ns_new(JSContext    *context,
     JSObject *ns;
     JSObject *global;
     Ns *priv;
+    JSBool found;
 
     /* put constructor in the global namespace */
     global = gjs_get_import_global(context);
 
-    if (!gjs_object_has_property(context, global, gjs_ns_class.name)) {
+    if (!JS_HasProperty(context, global, gjs_ns_class.name, &found))
+        return JS_FALSE;
+    if (!found) {
         JSObject *prototype;
         prototype = JS_InitClass(context, global,
                                  /* parent prototype JSObject* for
@@ -215,8 +218,6 @@ ns_new(JSContext    *context,
         if (prototype == NULL)
             gjs_fatal("Can't init class %s", gjs_ns_class.name);
 
-        g_assert(gjs_object_has_property(context, global, gjs_ns_class.name));
-
         gjs_debug(GJS_DEBUG_GNAMESPACE, "Initialized class %s prototype %p",
                   gjs_ns_class.name, prototype);
     }
diff --git a/gi/object.c b/gi/object.c
index 608236f..f510a5c 100644
--- a/gi/object.c
+++ b/gi/object.c
@@ -1919,7 +1919,12 @@ gjs_define_object_class(JSContext     *context,
         constructor_name = g_base_info_get_name((GIBaseInfo*) info);
     }
 
-    if (gjs_object_get_property(context, in_object, constructor_name, &value)) {
+    if (!JS_GetProperty(context, in_object, constructor_name, &value)) {
+        if (info)
+            g_base_info_unref(info);
+        return JS_FALSE;
+    }
+    if (!JSVAL_IS_VOID(value)) {
         jsid prototype_name;
 
         if (!JSVAL_IS_OBJECT(value)) {
diff --git a/gi/param.c b/gi/param.c
index 5034900..7e0b8bc 100644
--- a/gi/param.c
+++ b/gi/param.c
@@ -485,7 +485,8 @@ gjs_define_param_class(JSContext    *context,
 
     constructor_name = "ParamSpec";
 
-    gjs_object_get_property(context, in_object, constructor_name, &value);
+    if (!JS_GetProperty(context, in_object, constructor_name, &value))
+        return JS_FALSE;
     if (!JSVAL_IS_VOID(value)) {
         jsid prototype_name;
 
diff --git a/gi/repo.c b/gi/repo.c
index ec7d713..8aefe86 100644
--- a/gi/repo.c
+++ b/gi/repo.c
@@ -37,6 +37,7 @@
 #include "gerror.h"
 
 #include <gjs/compat.h>
+#include <gjs/runtime.h>
 
 #include <util/log.h>
 #include <util/misc.h>
@@ -246,10 +247,14 @@ repo_new(JSContext *context)
     JSObject *repo;
     JSObject *global;
     JSObject *versions;
+    JSBool found;
+    jsid versions_name;
 
     global = gjs_get_import_global(context);
 
-    if (!gjs_object_has_property(context, global, gjs_repo_class.name)) {
+    if (!JS_HasProperty(context, global, gjs_repo_class.name, &found))
+        return NULL;
+    if (!found) {
         JSObject *prototype;
         prototype = JS_InitClass(context, global,
                                  /* parent prototype JSObject* for
@@ -276,8 +281,6 @@ repo_new(JSContext *context)
         if (prototype == NULL)
             gjs_fatal("Can't init class %s", gjs_repo_class.name);
 
-        g_assert(gjs_object_has_property(context, global, gjs_repo_class.name));
-
         gjs_debug(GJS_DEBUG_GREPO, "Initialized class %s prototype %p",
                   gjs_repo_class.name, prototype);
     }
@@ -299,14 +302,14 @@ repo_new(JSContext *context)
                         "repo constructor, obj %p priv %p", repo, priv);
 
     versions = JS_NewObject(context, NULL, NULL, global);
+    versions_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
+                                                 GJS_STRING_GI_VERSIONS);
 
-    JS_DefineProperty(context, repo,
-                      "versions",
-                      OBJECT_TO_JSVAL(versions),
-                      NULL, NULL,
-                      JSPROP_PERMANENT);
-
-    g_assert(gjs_object_has_property(context, repo, "versions"));
+    JS_DefinePropertyById(context, repo,
+                          versions_name,
+                          OBJECT_TO_JSVAL(versions),
+                          NULL, NULL,
+                          JSPROP_PERMANENT);
 
     JS_DefineObject(context, repo, DUMPBIN, NULL, NULL, JSPROP_PERMANENT);
 
diff --git a/gi/union.c b/gi/union.c
index 7472c99..cfa7e59 100644
--- a/gi/union.c
+++ b/gi/union.c
@@ -413,7 +413,9 @@ gjs_define_union_class(JSContext    *context,
 
     constructor_name = g_base_info_get_name( (GIBaseInfo*) info);
 
-    if (gjs_object_get_property(context, in_object, constructor_name, &value)) {
+    if (!JS_GetProperty(context, in_object, constructor_name, &value))
+        return JS_FALSE;
+    if (!JSVAL_IS_VOID(value)) {
         jsid prototype_name;
 
         if (!JSVAL_IS_OBJECT(value)) {
diff --git a/gi/value.c b/gi/value.c
index 9a33398..ece1db3 100644
--- a/gi/value.c
+++ b/gi/value.c
@@ -353,11 +353,15 @@ gjs_value_to_g_value_internal(JSContext    *context,
 
         g_value_set_object(gvalue, gobj);
     } else if (gtype == G_TYPE_STRV) {
+        jsid length_name;
+        JSBool found_length;
+
+        length_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
+                                                   GJS_STRING_LENGTH);
         if (JSVAL_IS_NULL(value)) {
             /* do nothing */
-        } else if (gjs_object_has_property(context,
-                                           JSVAL_TO_OBJECT(value),
-                                           "length")) {
+        } else if (JS_HasPropertyById(context, JSVAL_TO_OBJECT(value), length_name, &found_length) &&
+                   found_length) {
             jsval length_value;
             guint32 length;
 
diff --git a/gjs/importer.c b/gjs/importer.c
index 577609b..0b03d02 100644
--- a/gjs/importer.c
+++ b/gjs/importer.c
@@ -582,10 +582,10 @@ do_import(JSContext  *context,
         if (module_obj != NULL) {
             jsval obj_val;
 
-            if (gjs_object_get_property(context,
-                                        module_obj,
-                                        name,
-                                        &obj_val)) {
+            if (JS_GetProperty(context,
+                               module_obj,
+                               name,
+                               &obj_val)) {
                 if (!JSVAL_IS_VOID(obj_val) &&
                     JS_DefineProperty(context, obj,
                                       name, obj_val,
@@ -1012,11 +1012,13 @@ importer_new(JSContext    *context)
     JSObject *importer;
     Importer *priv;
     JSObject *global;
-    (void) priv;
+    JSBool found;
 
     global = gjs_get_import_global(context);
 
-    if (!gjs_object_has_property(context, global, gjs_importer_class.name)) {
+    if (!JS_HasProperty(context, global, gjs_importer_class.name, &found))
+        gjs_fatal("HasProperty call failed creating importer class");
+    if (!found) {
         JSObject *prototype;
         prototype = JS_InitClass(context, global,
                                  /* parent prototype JSObject* for
@@ -1043,8 +1045,6 @@ importer_new(JSContext    *context)
         if (prototype == NULL)
             gjs_fatal("Can't init class %s", gjs_importer_class.name);
 
-        g_assert(gjs_object_has_property(context, global, gjs_importer_class.name));
-
         gjs_debug(GJS_DEBUG_IMPORTER, "Initialized class %s prototype %p",
                   gjs_importer_class.name, prototype);
     }
@@ -1175,14 +1175,21 @@ gjs_create_root_importer(JSContext   *context,
                          gboolean     add_standard_search_path)
 {
     JSObject *global;
+    jsid imports_name;
+    JSBool found;
 
     global = gjs_get_import_global(context);
 
     JS_BeginRequest(context);
 
-    if (!gjs_object_has_property(context,
-                                 global,
-                                 "imports")) {
+    imports_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
+                                                GJS_STRING_IMPORTS);
+    if (!JS_HasPropertyById(context, global, imports_name, &found)) {
+        JS_EndRequest(context);
+        return JS_FALSE;
+    }
+
+    if (!found) {
         if (gjs_define_importer(context, global,
                                 "imports",
                                 initial_search_path, add_standard_search_path) == NULL) {
diff --git a/gjs/jsapi-util-error.c b/gjs/jsapi-util-error.c
index cb8dc67..9813515 100644
--- a/gjs/jsapi-util-error.c
+++ b/gjs/jsapi-util-error.c
@@ -82,8 +82,9 @@ gjs_throw_valist(JSContext       *context,
         goto out;
     }
 
-    if (!gjs_object_get_property(context, JS_GetGlobalObject(context),
-                                 error_class, &v_constructor)) {
+    if (!JS_GetProperty(context, JS_GetGlobalObject(context),
+                        error_class, &v_constructor) ||
+        !JSVAL_IS_OBJECT(v_constructor)) {
         JS_ReportError(context, "??? Missing Error constructor in global object?");
         goto out;
     }
diff --git a/gjs/jsapi-util.c b/gjs/jsapi-util.c
index fe1083c..da08aa2 100644
--- a/gjs/jsapi-util.c
+++ b/gjs/jsapi-util.c
@@ -103,49 +103,6 @@ gjs_init_context_standard (JSContext       *context)
     return TRUE;
 }
 
-
-/* Checks whether an object has a property; unlike JS_GetProperty(),
- * never sets an exception. Treats a property with a value of JSVAL_VOID
- * the same as an absent property and returns false in both cases.
- */
-gboolean
-gjs_object_has_property(JSContext  *context,
-                        JSObject   *obj,
-                        const char *property_name)
-{
-    return gjs_object_get_property(context, obj, property_name, NULL);
-}
-
-/* Checks whether an object has a property; unlike JS_GetProperty(),
- * never sets an exception. Treats a property with a value of JSVAL_VOID
- * the same as an absent property and returns false in both cases.
- * Always initializes *value_p, if only to JSVAL_VOID, even if it
- * returns FALSE.
- */
-gboolean
-gjs_object_get_property(JSContext  *context,
-                        JSObject   *obj,
-                        const char *property_name,
-                        jsval      *value_p)
-{
-    jsval value;
-    JSExceptionState *state;
-
-    JS_BeginRequest(context);
-
-    value = JSVAL_VOID;
-    state = JS_SaveExceptionState(context);
-    JS_GetProperty(context, obj, property_name, &value);
-    JS_RestoreExceptionState(context, state);
-
-    if (value_p)
-        *value_p = value;
-
-    JS_EndRequest(context);
-
-    return !JSVAL_IS_VOID(value);
-}
-
 /* Returns whether the object had the property; if the object did
  * not have the property, always sets an exception. Treats
  * "the property's value is JSVAL_VOID" the same as "no such property,"
@@ -413,13 +370,13 @@ gjs_log_object_props(JSContext      *context,
 
     while (!JSID_IS_VOID(prop_id)) {
         jsval propval;
-        char *name;
         char *debugstr;
+        char *name;
 
-        if (!gjs_get_string_id(context, prop_id, &name))
+        if (!JS_GetPropertyById(context, obj, prop_id, &propval))
             goto next;
 
-        if (!gjs_object_get_property(context, obj, name, &propval))
+        if (!gjs_get_string_id(context, prop_id, &name))
             goto next;
 
         debugstr = gjs_value_debug_string(context, propval);
@@ -502,7 +459,8 @@ log_one_exception_property(JSContext  *context,
     jsval v;
     char *debugstr;
 
-    gjs_object_get_property(context, object, name, &v);
+    if (!JS_GetProperty(context, object, name, &v))
+        return;
 
     debugstr = gjs_value_debug_string(context, v);
     gjs_debug(GJS_DEBUG_ERROR, "  %s = '%s'", name, debugstr);
@@ -641,12 +599,12 @@ try_to_chain_stack_trace(JSContext *src_context, JSContext *dst_context,
     JS_ClearPendingException(dst_context);
 
     /* get stack trace for src_exc and chained */
-    if (!(gjs_object_get_property(dst_context, JSVAL_TO_OBJECT(chained),
-                                  "stack", &dst_stack) &&
+    if (!(JS_GetProperty(dst_context, JSVAL_TO_OBJECT(chained),
+                         "stack", &dst_stack) &&
           JSVAL_IS_STRING(dst_stack)))
         goto out; // couldn't get chained stack
-    if (!(gjs_object_get_property(src_context, JSVAL_TO_OBJECT(src_exc),
-                                  "stack", &src_stack) &&
+    if (!(JS_GetProperty(src_context, JSVAL_TO_OBJECT(src_exc),
+                         "stack", &src_stack) &&
           JSVAL_IS_STRING(src_stack)))
         goto out; // couldn't get source stack
 
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 840958e..c154a74 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -151,8 +151,9 @@ jsval gjs_##cname##_create_proto(JSContext *context, JSObject *module, const cha
 { \
     jsval rval; \
     JSObject *global = gjs_get_import_global(context); \
-    if (!gjs_object_get_property(context, global, \
-                                 gjs_##cname##_class.name, &rval)) {  \
+    if (!JS_GetProperty(context, global, gjs_##cname##_class.name, &rval)) \
+        return JSVAL_NULL; \
+    if (JSVAL_IS_VOID(rval)) { \
         JSObject *prototype = JS_InitClass(context, global, \
                                  parent, \
                                  &gjs_##cname##_class, \
@@ -180,13 +181,6 @@ jsval gjs_##cname##_create_proto(JSContext *context, JSObject *module, const cha
 gboolean    gjs_init_context_standard        (JSContext       *context);
 
 JSObject*   gjs_get_import_global            (JSContext       *context);
-gboolean    gjs_object_has_property          (JSContext       *context,
-                                              JSObject        *obj,
-                                              const char      *property_name);
-gboolean    gjs_object_get_property          (JSContext       *context,
-                                              JSObject        *obj,
-                                              const char      *property_name,
-                                              jsval           *value_p);
 gboolean    gjs_object_require_property      (JSContext       *context,
                                               JSObject        *obj,
                                               const char      *obj_description,
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]