GitLab repository storage has been migrated to hashed layout. Please contact Infrastructure team if you notice any issues with repositories or hooks.

Commit 0cc23474 authored by Giovanni Campagna's avatar Giovanni Campagna Committed by Georges Basile Stavracas Neto

object: don't use toggle references unless necessary

Many GObjects (such as widgets and actors) don't have JS state, but
they're kept alive by C code. We can therefore save some memory
by GCing these objects, and creating new wrappers when needed.
If state is ever set, we transparently switch to toggle refs, so
no change should be visible at the JS level.

https://bugzilla.gnome.org/show_bug.cgi?id=679688
parent ead64625
......@@ -70,6 +70,11 @@ struct ObjectInstance {
unsigned js_object_finalized : 1;
unsigned g_object_finalized : 1;
/* True if this object has visible JS state, and thus its lifecycle is
* managed using toggle references. False if this object just keeps a
* hard ref on the underlying GObject, and may be finalized at will. */
bool uses_toggle_ref : 1;
};
static std::stack<JS::PersistentRootedObject> object_init_list;
......@@ -85,6 +90,7 @@ extern struct JSClass gjs_object_instance_class;
GJS_DEFINE_PRIV_FROM_JS(ObjectInstance, gjs_object_instance_class)
static void disassociate_js_gobject (GObject *gobj);
static void ensure_uses_toggle_ref(JSContext *cx, ObjectInstance *priv);
typedef enum {
SOME_ERROR_OCCURRED = false,
......@@ -430,6 +436,9 @@ set_g_param_from_prop(JSContext *context,
case SOME_ERROR_OCCURRED:
return false;
case NO_SUCH_G_PROPERTY:
/* We need to keep the wrapper alive in order not to lose custom
* "expando" properties */
ensure_uses_toggle_ref(context, priv);
return result.succeed();
case VALUE_WAS_SET:
default:
......@@ -496,14 +505,7 @@ object_instance_set_prop(JSContext *context,
bool ret = true;
bool g_param_was_set = false;
if (!gjs_get_string_id(context, id, &name))
return result.succeed(); /* not resolved, but no error */
priv = priv_from_js(context, obj);
gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
"Set prop '%s' hook obj %p priv %p",
name.get(), obj.get(), priv);
if (priv == nullptr)
/* see the comment in object_instance_get_prop() on this */
return result.succeed();
......@@ -521,6 +523,18 @@ object_instance_set_prop(JSContext *context,
return result.succeed();
}
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
* then a number or symbol property was probably set. */
ensure_uses_toggle_ref(context, priv);
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;
......@@ -1126,7 +1140,10 @@ static void
release_native_object (ObjectInstance *priv)
{
priv->keep_alive.reset();
g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, NULL);
if (priv->uses_toggle_ref)
g_object_remove_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, nullptr);
else
g_object_unref(priv->gobj);
priv->gobj = NULL;
}
......@@ -1256,16 +1273,28 @@ associate_js_gobject (JSContext *context,
ObjectInstance *priv;
priv = priv_from_js(context, object);
priv->uses_toggle_ref = false;
priv->gobj = gobj;
g_assert(!priv->keep_alive.rooted());
set_object_qdata(gobj, priv);
priv->keep_alive = object;
ensure_weak_pointer_callback(context);
wrapped_gobject_list.insert(priv);
g_object_weak_ref(gobj, wrapped_gobj_dispose_notify, priv);
}
static void
ensure_uses_toggle_ref(JSContext *cx,
ObjectInstance *priv)
{
if (priv->uses_toggle_ref)
return;
g_assert(!priv->keep_alive.rooted());
/* OK, here is where things get complicated. We want the
* wrapped gobj to keep the JSObject* wrapper alive, because
......@@ -1278,8 +1307,14 @@ associate_js_gobject (JSContext *context,
* the wrapper to be garbage collected (and thus unref the
* wrappee).
*/
priv->keep_alive.root(context, object, gobj_no_longer_kept_alive_func, priv);
g_object_add_toggle_ref(gobj, wrapped_gobj_toggle_notify, NULL);
priv->uses_toggle_ref = true;
priv->keep_alive.switch_to_rooted(cx, gobj_no_longer_kept_alive_func, priv);
g_object_add_toggle_ref(priv->gobj, wrapped_gobj_toggle_notify, nullptr);
/* We now have both a ref and a toggle ref, we only want the toggle ref.
* This may immediately remove the GC root we just added, since refcount
* may drop to 1. */
g_object_unref(priv->gobj);
}
static void
......@@ -1383,6 +1418,7 @@ G_GNUC_END_IGNORE_DEPRECATIONS
* we're not actually using it, so just let it get collected. Avoiding
* this would require a non-trivial amount of work.
* */
ensure_uses_toggle_ref(context, other_priv);
object.set(other_priv->keep_alive);
g_object_unref(gobj); /* We already own a reference */
gobj = NULL;
......@@ -1410,11 +1446,6 @@ G_GNUC_END_IGNORE_DEPRECATIONS
if (priv->gobj == NULL)
associate_js_gobject(context, object, gobj);
/* We now have both a ref and a toggle ref, we only want the
* toggle ref. This may immediately remove the GC root
* we just added, since refcount may drop to 1.
*/
g_object_unref(gobj);
gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "JSObject created with GObject %p (%s)",
priv->gobj, G_OBJECT_TYPE_NAME(priv->gobj));
......@@ -1683,6 +1714,8 @@ real_connect_func(JSContext *context,
return true;
}
ensure_uses_toggle_ref(context, priv);
if (argc != 2 || !argv[0].isString() || !JS::IsCallable(&argv[1].toObject())) {
gjs_throw(context, "connect() takes two args, the signal name and the callback");
return false;
......@@ -2123,9 +2156,6 @@ gjs_object_from_g_object(JSContext *context,
g_object_ref_sink(gobj);
associate_js_gobject(context, obj, gobj);
/* see the comment in init_object_instance() for this */
g_object_unref(gobj);
g_assert(priv->keep_alive == obj.get());
}
......@@ -2657,6 +2687,10 @@ gjs_object_custom_init(GTypeInstance *instance,
associate_js_gobject(context, object, G_OBJECT (instance));
/* Custom JS objects will most likely have visible state, so
* just do this from the start */
ensure_uses_toggle_ref(context, priv);
JS::RootedValue v(context);
if (!gjs_object_get_property(context, object,
GJS_STRING_INSTANCE_INIT, &v)) {
......@@ -3109,6 +3143,9 @@ gjs_object_associate_closure(JSContext *cx,
if (!priv)
return false;
if (priv->gobj)
ensure_uses_toggle_ref(cx, priv);
do_associate_closure(priv, closure);
return true;
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment