[gjs] Don't use a property for the keep alive



commit 4260621e475859deb0e308f8108293bf049329a6
Author: Giovanni Campagna <gcampagna src gnome org>
Date:   Tue Apr 9 00:22:08 2013 +0200

    Don't use a property for the keep alive
    
    The keep alive is accessed so often that even using property IDs is slow.
    Introduce the concept of "global slots" instead, which are special
    hidden properties hanging off the global object. We can't use
    regular reserved slots, because the global objects uses some internally,
    so we emulate them with the private data and a custom tracer.
    While we're there, let's do the same for imports.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=697592

 gi/keep-alive.c  |   88 ++++++++---------------------------------------------
 gi/keep-alive.h  |    1 -
 gi/object.c      |    4 +-
 gi/repo.c        |   27 ++++-------------
 gjs/importer.c   |   71 +++++++++++++++++++++----------------------
 gjs/jsapi-util.c |   58 +++++++++++++++++++++++++++++++++--
 gjs/jsapi-util.h |   13 ++++++++
 7 files changed, 124 insertions(+), 138 deletions(-)
---
diff --git a/gi/keep-alive.c b/gi/keep-alive.c
index f008448..e933a43 100644
--- a/gi/keep-alive.c
+++ b/gi/keep-alive.c
@@ -322,62 +322,33 @@ gjs_keep_alive_remove_child(JSContext         *context,
 }
 
 static JSObject*
-gjs_keep_alive_get_from_parent(JSContext *context,
-                               JSObject  *parent)
-{
-    jsid keep_alive_name;
-    jsval value;
-
-    keep_alive_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
-                                                   GJS_STRING_KEEP_ALIVE_MARKER);
-    if (!JS_GetPropertyById(context, parent, keep_alive_name, &value))
-        return NULL;
-
-    if (JSVAL_IS_OBJECT(value))
-        return JSVAL_TO_OBJECT(value);
-    else
-        return NULL;
-}
-
-JSObject*
-gjs_keep_alive_get_global(JSContext *context)
-{
-    return gjs_keep_alive_get_from_parent(context,
-                                          JS_GetGlobalObject(context));
-}
-
-static JSObject*
-gjs_keep_alive_create_in_parent(JSContext *context,
-                                JSObject  *parent)
+gjs_keep_alive_create(JSContext *context)
 {
     JSObject *keep_alive;
-    jsid keep_alive_name;
 
     JS_BeginRequest(context);
 
     keep_alive = gjs_keep_alive_new(context);
+    if (!keep_alive)
+        gjs_fatal("could not create keep_alive on global object, no memory?");
 
-    keep_alive_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
-                                                   GJS_STRING_KEEP_ALIVE_MARKER);
-    if (!JS_DefinePropertyById(context, parent,
-                               keep_alive_name,
-                               OBJECT_TO_JSVAL(keep_alive),
-                               NULL, NULL,
-                               /* No ENUMERATE since this is a hidden
-                                * implementation detail kind of property
-                                */
-                               JSPROP_READONLY | JSPROP_PERMANENT))
-        gjs_fatal("no memory to define keep_alive property");
+    gjs_set_global_slot(context, GJS_GLOBAL_SLOT_KEEP_ALIVE, OBJECT_TO_JSVAL(keep_alive));
 
     JS_EndRequest(context);
     return keep_alive;
 }
 
-static JSObject*
-gjs_keep_alive_create_in_global(JSContext *context)
+JSObject*
+gjs_keep_alive_get_global(JSContext *context)
 {
-    return gjs_keep_alive_create_in_parent(context,
-                                           JS_GetGlobalObject(context));
+    jsval keep_alive;
+
+    keep_alive = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_KEEP_ALIVE);
+
+    if (G_LIKELY (JSVAL_IS_OBJECT(keep_alive)))
+        return JSVAL_TO_OBJECT(keep_alive);
+
+    return gjs_keep_alive_create(context);
 }
 
 void
@@ -392,12 +363,6 @@ gjs_keep_alive_add_global_child(JSContext         *context,
 
     keep_alive = gjs_keep_alive_get_global(context);
 
-    if (!keep_alive)
-        keep_alive = gjs_keep_alive_create_in_global(context);
-
-    if (!keep_alive)
-        gjs_fatal("could not create keep_alive on global object, no memory?");
-
     gjs_keep_alive_add_child(context,
                              keep_alive,
                              notify, child, data);
@@ -427,28 +392,3 @@ gjs_keep_alive_remove_global_child(JSContext         *context,
 
     JS_EndRequest(context);
 }
-
-JSObject*
-gjs_keep_alive_get_for_import_global(JSContext *context)
-{
-    JSObject *global;
-    JSObject *keep_alive;
-
-    global = gjs_get_import_global(context);
-
-    g_assert(global != NULL);
-
-    JS_BeginRequest(context);
-
-    keep_alive = gjs_keep_alive_get_from_parent(context, global);
-
-    if (!keep_alive)
-        keep_alive = gjs_keep_alive_create_in_parent(context, global);
-
-    if (!keep_alive)
-        gjs_fatal("could not create keep_alive on global object, no memory?");
-
-    JS_EndRequest(context);
-
-    return keep_alive;
-}
diff --git a/gi/keep-alive.h b/gi/keep-alive.h
index fb197ff..a5880c1 100644
--- a/gi/keep-alive.h
+++ b/gi/keep-alive.h
@@ -73,7 +73,6 @@ void      gjs_keep_alive_remove_global_child       (JSContext         *context,
                                                     GjsUnrootedFunc  notify,
                                                     JSObject          *child,
                                                     void              *data);
-JSObject* gjs_keep_alive_get_for_import_global    (JSContext         *context);
 
 G_END_DECLS
 
diff --git a/gi/object.c b/gi/object.c
index a76579a..ca8924a 100644
--- a/gi/object.c
+++ b/gi/object.c
@@ -927,7 +927,7 @@ handle_toggle_up(JSContext *context,
      */
     if (priv->keep_alive == NULL) {
         gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "Adding object to keep alive");
-        priv->keep_alive = gjs_keep_alive_get_for_import_global(context);
+        priv->keep_alive = gjs_keep_alive_get_global(context);
         gjs_keep_alive_add_child(context, priv->keep_alive,
                                  gobj_no_longer_kept_alive_func,
                                  obj,
@@ -1175,7 +1175,7 @@ associate_js_gobject (JSContext      *context,
      * the wrapper to be garbage collected (and thus unref the
      * wrappee).
      */
-    priv->keep_alive = gjs_keep_alive_get_for_import_global(context);
+    priv->keep_alive = gjs_keep_alive_get_global(context);
     gjs_keep_alive_add_child(context,
                              priv->keep_alive,
                              gobj_no_longer_kept_alive_func,
diff --git a/gi/repo.c b/gi/repo.c
index 65f4d29..6db129b 100644
--- a/gi/repo.c
+++ b/gi/repo.c
@@ -559,22 +559,16 @@ static JSObject*
 lookup_override_function(JSContext  *context,
                          jsid        ns_name)
 {
-    JSObject *global;
     jsval importer;
     jsval overridespkg;
     jsval module;
     jsval function;
-    jsid imports_name, overrides_name, object_init_name;
+    jsid overrides_name, object_init_name;
 
     JS_BeginRequest(context);
-    global = gjs_get_import_global(context);
 
-    importer = JSVAL_VOID;
-    imports_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
-                                                GJS_STRING_IMPORTS);
-    if (!gjs_object_require_property(context, global, "global object", imports_name, &importer) ||
-        !JSVAL_IS_OBJECT(importer))
-        goto fail;
+    importer = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_IMPORTS);
+    g_assert(JSVAL_IS_OBJECT(importer));
 
     overridespkg = JSVAL_VOID;
     overrides_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
@@ -609,25 +603,16 @@ JSObject*
 gjs_lookup_namespace_object_by_name(JSContext      *context,
                                     jsid            ns_name)
 {
-    JSObject *global;
     JSObject *repo_obj;
     jsval importer;
     jsval girepository;
     jsval ns_obj;
-    jsid imports_name, gi_name;
+    jsid gi_name;
 
     JS_BeginRequest(context);
-    global = gjs_get_import_global(context);
 
-    importer = JSVAL_VOID;
-    imports_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
-                                                GJS_STRING_IMPORTS);
-    if (!gjs_object_require_property(context, global, "global object", imports_name, &importer) ||
-        !JSVAL_IS_OBJECT(importer)) {
-        gjs_log_exception(context, NULL);
-        gjs_throw(context, "No imports property in global object");
-        goto fail;
-    }
+    importer = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_IMPORTS);
+    g_assert(JSVAL_IS_OBJECT(importer));
 
     girepository = JSVAL_VOID;
     gi_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
diff --git a/gjs/importer.c b/gjs/importer.c
index b51756f..bf0d2e9 100644
--- a/gjs/importer.c
+++ b/gjs/importer.c
@@ -62,7 +62,7 @@ define_meta_properties(JSContext  *context,
     /* We define both __moduleName__ and __parentModule__ to null
      * on the root importer
      */
-    parent_is_module = JS_InstanceOf(context, parent, &gjs_importer_class, NULL);
+    parent_is_module = parent && JS_InstanceOf(context, parent, &gjs_importer_class, NULL);
 
     gjs_debug(GJS_DEBUG_IMPORTER, "Defining parent %p of %p '%s' is mod %d",
               parent, module_obj, module_name ? module_name : "<root>", parent_is_module);
@@ -1126,12 +1126,12 @@ gjs_get_search_path(void)
     return (G_CONST_RETURN char * G_CONST_RETURN *)search_path;
 }
 
-JSObject*
-gjs_define_importer(JSContext    *context,
-                    JSObject     *in_object,
+static JSObject*
+gjs_create_importer(JSContext    *context,
                     const char   *importer_name,
                     const char  **initial_search_path,
-                    gboolean      add_standard_search_path)
+                    gboolean      add_standard_search_path,
+                    JSObject     *in_object)
 {
     JSObject *importer;
     char **paths[2] = {0};
@@ -1149,9 +1149,9 @@ gjs_define_importer(JSContext    *context,
 
     /* API users can replace this property from JS, is the idea */
     if (!gjs_define_string_array(context, importer,
-                                    "searchPath", -1, (const char **)search_path,
-                                    /* settable (no READONLY) but not deleteable (PERMANENT) */
-                                    JSPROP_PERMANENT | JSPROP_ENUMERATE))
+                                 "searchPath", -1, (const char **)search_path,
+                                 /* settable (no READONLY) but not deleteable (PERMANENT) */
+                                 JSPROP_PERMANENT | JSPROP_ENUMERATE))
         gjs_fatal("no memory to define importer search path prop");
 
     g_strfreev(search_path);
@@ -1159,6 +1159,21 @@ gjs_define_importer(JSContext    *context,
     if (!define_meta_properties(context, importer, NULL, importer_name, in_object))
         gjs_fatal("failed to define meta properties on importer");
 
+    return importer;
+}
+
+JSObject*
+gjs_define_importer(JSContext    *context,
+                    JSObject     *in_object,
+                    const char   *importer_name,
+                    const char  **initial_search_path,
+                    gboolean      add_standard_search_path)
+
+{
+    JSObject *importer;
+
+    importer = gjs_create_importer(context, importer_name, initial_search_path, add_standard_search_path, 
in_object);
+
     if (!JS_DefineProperty(context, in_object,
                            importer_name, OBJECT_TO_JSVAL(importer),
                            NULL, NULL,
@@ -1180,35 +1195,26 @@ gjs_create_root_importer(JSContext   *context,
                          const char **initial_search_path,
                          gboolean     add_standard_search_path)
 {
-    JSObject *global;
-    jsid imports_name;
+    jsval importer;
     JSBool found;
 
-    global = gjs_get_import_global(context);
-
     JS_BeginRequest(context);
 
-    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;
-    }
+    importer = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_IMPORTS);
 
-    if (!found) {
-        if (gjs_define_importer(context, global,
-                                "imports",
-                                initial_search_path, add_standard_search_path) == NULL) {
-            JS_EndRequest(context);
-            return JS_FALSE;
-        }
-    } else {
+    if (G_UNLIKELY (!JSVAL_IS_VOID(importer))) {
         gjs_debug(GJS_DEBUG_IMPORTER,
                   "Someone else already created root importer, ignoring second request");
+
         JS_EndRequest(context);
         return JS_TRUE;
     }
 
+    importer = OBJECT_TO_JSVAL(gjs_create_importer(context, "imports",
+                                                   initial_search_path,
+                                                   add_standard_search_path, NULL));
+    gjs_set_global_slot(context, GJS_GLOBAL_SLOT_IMPORTS, importer);
+
     JS_EndRequest(context);
     return JS_TRUE;
 }
@@ -1219,7 +1225,7 @@ gjs_define_root_importer(JSContext   *context,
                          const char  *importer_name)
 {
     JSObject *global;
-    jsval value;
+    jsval importer;
     JSBool success;
     jsid imports_name;
 
@@ -1227,18 +1233,11 @@ gjs_define_root_importer(JSContext   *context,
     global = gjs_get_import_global(context);
     JS_BeginRequest(context);
 
+    importer = gjs_get_global_slot(context, GJS_GLOBAL_SLOT_IMPORTS);
     imports_name = gjs_runtime_get_const_string(JS_GetRuntime(context),
                                                 GJS_STRING_IMPORTS);
-    if (!gjs_object_require_property(context,
-                                     global, "global object",
-                                     imports_name, &value) ||
-        !JSVAL_IS_OBJECT(value)) {
-        gjs_debug(GJS_DEBUG_IMPORTER, "Root importer did not exist, couldn't get from load context; must 
create it");
-        goto fail;
-    }
-
     if (!JS_DefineProperty(context, in_object,
-                           importer_name, value,
+                           importer_name, importer,
                            NULL, NULL,
                            GJS_MODULE_PROP_FLAGS)) {
         gjs_debug(GJS_DEBUG_IMPORTER, "DefineProperty %s on %p failed",
diff --git a/gjs/jsapi-util.c b/gjs/jsapi-util.c
index 8891bb3..cfadd4f 100644
--- a/gjs/jsapi-util.c
+++ b/gjs/jsapi-util.c
@@ -44,7 +44,6 @@ gjs_util_error_quark (void)
     return g_quark_from_static_string ("gjs-util-error-quark");
 }
 
-
 /**
  * gjs_get_import_global:
  * @context: a #JSContext
@@ -71,15 +70,32 @@ gjs_get_import_global(JSContext *context)
 }
 
 static void
-finalize_stub(JSFreeOp *fop, JSObject *global)
+global_finalize(JSFreeOp *fop, JSObject *global)
 {
+    g_free(JS_GetPrivate(global));
+    JS_SetPrivate(global, NULL);
+}
+
+static void
+global_trace(JSTracer *trc, JSObject *global)
+{
+    jsval *slots;
+    int i;
+
+    slots = JS_GetPrivate(global);
+
+    for (i = 0; i < GJS_GLOBAL_SLOT_LAST; i++) {
+        if (!JSVAL_IS_VOID(slots[i]))
+            JS_CALL_VALUE_TRACER(trc, slots[i], "global slot");
+    }
 }
 
 static JSClass global_class = {
     "GjsGlobal", JSCLASS_GLOBAL_FLAGS,
     JS_PropertyStub, JS_PropertyStub, JS_PropertyStub, JS_StrictPropertyStub,
-    JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, finalize_stub,
-    JSCLASS_NO_OPTIONAL_MEMBERS
+    JS_EnumerateStub, JS_ResolveStub, JS_ConvertStub, global_finalize,
+    NULL /* checkAccess */, NULL /* call */, NULL /* hasInstance */, NULL /* construct */, global_trace,
+    { NULL }
 };
 
 /**
@@ -95,14 +111,48 @@ gboolean
 gjs_init_context_standard (JSContext       *context)
 {
     JSObject *global;
+    jsval *slots;
+    int i;
+
     global = JS_NewGlobalObject(context, &global_class, NULL);
     if (global == NULL)
         return FALSE;
     if (!JS_InitStandardClasses(context, global))
         return FALSE;
+
+    slots = g_new(jsval, GJS_GLOBAL_SLOT_LAST);
+    for (i = 0; i < GJS_GLOBAL_SLOT_LAST; i++)
+        slots[i] = JSVAL_VOID;
+
+    JS_SetPrivate(global, slots);
     return TRUE;
 }
 
+void
+gjs_set_global_slot (JSContext     *context,
+                     GjsGlobalSlot  slot,
+                     jsval          value)
+{
+    JSObject *global;
+    jsval *slots;
+
+    global = JS_GetGlobalObject(context);
+    slots = JS_GetPrivate(global);
+    slots[slot] = value;
+}
+
+jsval
+gjs_get_global_slot (JSContext     *context,
+                     GjsGlobalSlot  slot)
+{
+    JSObject *global;
+    jsval *slots;
+
+    global = JS_GetGlobalObject(context);
+    slots = JS_GetPrivate(global);
+    return slots[slot];
+}
+
 /* 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,".
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index fff9db8..210f5ee 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -43,6 +43,12 @@ enum {
   GJS_UTIL_ERROR_ARGUMENT_TYPE_MISMATCH
 };
 
+typedef enum {
+    GJS_GLOBAL_SLOT_IMPORTS,
+    GJS_GLOBAL_SLOT_KEEP_ALIVE,
+    GJS_GLOBAL_SLOT_LAST,
+} GjsGlobalSlot;
+
 typedef struct GjsRootedArray GjsRootedArray;
 
 /* Flags that should be set on properties exported from native code modules.
@@ -182,6 +188,13 @@ 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);
+
+jsval       gjs_get_global_slot              (JSContext       *context,
+                                              GjsGlobalSlot    slot);
+void        gjs_set_global_slot              (JSContext       *context,
+                                              GjsGlobalSlot    slot,
+                                              jsval            value);
+
 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]