From 47c12389a08ef04274c02e4d19159874634e2aee Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Fri, 20 May 2022 20:49:05 -0400 Subject: [PATCH 1/3] gobject: Speed up property lookup When the param specs are provided as an array with g_object_class_install_properties, keep a copy of that array around and use it for looking up properties without the param spec pool. Note that this is an opportunistic optimization - currently, it only works for properties of the class itself, not for parent classes, and it only works if the property names are identical string literals (we're at the mercy of the linker for that). If we don't get lucky, we fall back to using the pspec pool as usual. --- gobject/gobject.c | 175 ++++++++++++++++++++++++++++++++++------------ gobject/gobject.h | 6 +- 2 files changed, 135 insertions(+), 46 deletions(-) diff --git a/gobject/gobject.c b/gobject/gobject.c index 2ee0db79c..1a6a21e19 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -485,6 +485,8 @@ g_object_base_class_init (GObjectClass *class) class->n_construct_properties = g_slist_length (class->construct_properties); class->get_property = NULL; class->set_property = NULL; + class->pspecs = NULL; + class->n_pspecs = 0; } static void @@ -704,6 +706,75 @@ g_object_class_install_property (GObjectClass *class, pspec); } +typedef struct { + const char *name; + GParamSpec *pspec; +} PspecEntry; + +static int +compare_pspec_entry (const void *a, + const void *b) +{ + const PspecEntry *ae = a; + const PspecEntry *be = b; + + return ae->name < be->name ? -1 : (ae->name > be->name ? 1 : 0); +} + +/* This uses pointer comparisons with @property_name, so + * will only work with string literals. */ +static inline GParamSpec * +find_pspec (GObjectClass *class, + const char *property_name) +{ + const PspecEntry *pspecs = (const PspecEntry *)class->pspecs; + gsize n_pspecs = class->n_pspecs; + + g_assert (n_pspecs <= G_MAXSSIZE); + + /* The limit for choosing between linear and binary search is + * fairly arbitrary. + * + * Both searches use pointer comparisons against @property_name. + * If this function is called with a non-static @property_name, + * it will fall through to the g_param_spec_pool_lookup() case. + * That’s OK; this is an opportunistic optimisation which relies + * on the fact that *most* (but not all) property lookups use + * static property names. + */ + if (n_pspecs < 10) + { + for (gsize i = 0; i < n_pspecs; i++) + { + if (pspecs[i].name == property_name) + return pspecs[i].pspec; + } + } + else + { + gssize lower = 0; + gssize upper = (int)class->n_pspecs - 1; + gssize mid; + + while (lower <= upper) + { + mid = (lower + upper) / 2; + + if (property_name < pspecs[mid].name) + upper = mid - 1; + else if (property_name > pspecs[mid].name) + lower = mid + 1; + else + return pspecs[mid].pspec; + } + } + + return g_param_spec_pool_lookup (pspec_pool, + property_name, + ((GTypeClass *)class)->g_type, + TRUE); +} + /** * g_object_class_install_properties: * @oclass: a #GObjectClass @@ -810,6 +881,32 @@ g_object_class_install_properties (GObjectClass *oclass, break; } } + + /* Save a copy of the pspec array inside the class struct. This + * makes it faster to look up pspecs for the class in future when + * acting on those properties. + * + * If a pspec is not in this cache array, calling code will fall + * back to using g_param_spec_pool_lookup(), so a pspec not being + * in this array is a (potential) performance problem but not a + * correctness problem. */ + if (oclass->pspecs == NULL) + { + PspecEntry *entries; + + entries = g_new (PspecEntry, n_pspecs - 1); + + for (i = 1; i < n_pspecs; i++) + { + entries[i - 1].name = pspecs[i]->name; + entries[i - 1].pspec = pspecs[i]; + } + + qsort (entries, n_pspecs - 1, sizeof (PspecEntry), compare_pspec_entry); + + oclass->pspecs = entries; + oclass->n_pspecs = n_pspecs - 1; + } } /** @@ -872,25 +969,16 @@ g_object_class_find_property (GObjectClass *class, const gchar *property_name) { GParamSpec *pspec; - GParamSpec *redirect; - + g_return_val_if_fail (G_IS_OBJECT_CLASS (class), NULL); g_return_val_if_fail (property_name != NULL, NULL); - - pspec = g_param_spec_pool_lookup (pspec_pool, - property_name, - G_OBJECT_CLASS_TYPE (class), - TRUE); - if (pspec) - { - redirect = g_param_spec_get_redirect_target (pspec); - if (redirect) - return redirect; - else - return pspec; - } - else - return NULL; + + pspec = find_pspec (class, property_name); + + if (pspec && ((GTypeInstance *)pspec)->g_class->g_type == G_TYPE_PARAM_OVERRIDE) + pspec = ((GParamSpecOverride *)pspec)->overridden; + + return pspec; } /** @@ -2217,8 +2305,8 @@ g_object_new_with_properties (GType object_type, params = g_newa (GObjectConstructParam, n_properties); for (i = 0; i < n_properties; i++) { - GParamSpec *pspec; - pspec = g_param_spec_pool_lookup (pspec_pool, names[i], object_type, TRUE); + GParamSpec *pspec = find_pspec (class, names[i]); + if (!g_object_new_is_valid_property (object_type, pspec, names[i], params, count)) continue; params[count].pspec = pspec; @@ -2283,9 +2371,8 @@ g_object_newv (GType object_type, for (i = 0; i < n_parameters; i++) { - GParamSpec *pspec; + GParamSpec *pspec = find_pspec (class, parameters[i].name); - pspec = g_param_spec_pool_lookup (pspec_pool, parameters[i].name, object_type, TRUE); if (!g_object_new_is_valid_property (object_type, pspec, parameters[i].name, cparams, j)) continue; @@ -2356,9 +2443,7 @@ g_object_new_valist (GType object_type, do { gchar *error = NULL; - GParamSpec *pspec; - - pspec = g_param_spec_pool_lookup (pspec_pool, name, object_type, TRUE); + GParamSpec *pspec = find_pspec (class, name); if (!g_object_new_is_valid_property (object_type, pspec, name, params, n_params)) break; @@ -2524,7 +2609,7 @@ g_object_setv (GObject *object, guint i; GObjectNotifyQueue *nqueue = NULL; GParamSpec *pspec; - GType obj_type; + GObjectClass *class; g_return_if_fail (G_IS_OBJECT (object)); @@ -2532,14 +2617,15 @@ g_object_setv (GObject *object, return; g_object_ref (object); - obj_type = G_OBJECT_TYPE (object); + + class = G_OBJECT_GET_CLASS (object); if (_g_object_has_notify_handler (object)) nqueue = g_object_notify_queue_freeze (object, FALSE); for (i = 0; i < n_properties; i++) { - pspec = g_param_spec_pool_lookup (pspec_pool, names[i], obj_type, TRUE); + pspec = find_pspec (class, names[i]); if (!g_object_set_is_valid_property (object, pspec, names[i])) break; @@ -2569,6 +2655,7 @@ g_object_set_valist (GObject *object, { GObjectNotifyQueue *nqueue = NULL; const gchar *name; + GObjectClass *class; g_return_if_fail (G_IS_OBJECT (object)); @@ -2577,6 +2664,8 @@ g_object_set_valist (GObject *object, if (_g_object_has_notify_handler (object)) nqueue = g_object_notify_queue_freeze (object, FALSE); + class = G_OBJECT_GET_CLASS (object); + name = first_property_name; while (name) { @@ -2585,10 +2674,7 @@ g_object_set_valist (GObject *object, gchar *error = NULL; GTypeValueTable *vtab; - pspec = g_param_spec_pool_lookup (pspec_pool, - name, - G_OBJECT_TYPE (object), - TRUE); + pspec = find_pspec (class, name); if (!g_object_set_is_valid_property (object, pspec, name)) break; @@ -2661,7 +2747,7 @@ g_object_getv (GObject *object, { guint i; GParamSpec *pspec; - GType obj_type; + GObjectClass *class; g_return_if_fail (G_IS_OBJECT (object)); @@ -2670,12 +2756,14 @@ g_object_getv (GObject *object, g_object_ref (object); + class = G_OBJECT_GET_CLASS (object); + memset (values, 0, n_properties * sizeof (GValue)); - obj_type = G_OBJECT_TYPE (object); for (i = 0; i < n_properties; i++) { - pspec = g_param_spec_pool_lookup (pspec_pool, names[i], obj_type, TRUE); + pspec = find_pspec (class, names[i]); + if (!g_object_get_is_valid_property (object, pspec, names[i])) break; g_value_init (&values[i], pspec->value_type); @@ -2705,23 +2793,23 @@ g_object_get_valist (GObject *object, va_list var_args) { const gchar *name; + GObjectClass *class; g_return_if_fail (G_IS_OBJECT (object)); g_object_ref (object); - + + class = G_OBJECT_GET_CLASS (object); + name = first_property_name; - + while (name) { GValue value = G_VALUE_INIT; GParamSpec *pspec; gchar *error; - - pspec = g_param_spec_pool_lookup (pspec_pool, - name, - G_OBJECT_TYPE (object), - TRUE); + + pspec = find_pspec (class, name); if (!g_object_get_is_valid_property (object, pspec, name)) break; @@ -2881,10 +2969,7 @@ g_object_get_property (GObject *object, g_object_ref (object); - pspec = g_param_spec_pool_lookup (pspec_pool, - property_name, - G_OBJECT_TYPE (object), - TRUE); + pspec = find_pspec (G_OBJECT_GET_CLASS (object), property_name); if (g_object_get_is_valid_property (object, pspec, property_name)) { diff --git a/gobject/gobject.h b/gobject/gobject.h index c67794ae5..04796c00a 100644 --- a/gobject/gobject.h +++ b/gobject/gobject.h @@ -370,8 +370,12 @@ struct _GObjectClass gsize flags; gsize n_construct_properties; + + gpointer pspecs; + gsize n_pspecs; + /* padding */ - gpointer pdummy[5]; + gpointer pdummy[3]; }; /** From df43536a6807063dcd8212041f4aad02193d5e08 Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 7 Jun 2022 08:52:00 -0400 Subject: [PATCH 2/3] Consistently use param_spec_follow_override --- gobject/gobject.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/gobject/gobject.c b/gobject/gobject.c index 1a6a21e19..f19c40180 100644 --- a/gobject/gobject.c +++ b/gobject/gobject.c @@ -954,6 +954,14 @@ g_object_interface_install_property (gpointer g_iface, (void) install_property_internal (iface_class->g_type, 0, pspec); } +/* Inlined version of g_param_spec_get_redirect_target(), for speed */ +static inline void +param_spec_follow_override (GParamSpec **pspec) +{ + if (((GTypeInstance *) (*pspec))->g_class->g_type == G_TYPE_PARAM_OVERRIDE) + *pspec = ((GParamSpecOverride *) (*pspec))->overridden; +} + /** * g_object_class_find_property: * @oclass: a #GObjectClass @@ -975,8 +983,8 @@ g_object_class_find_property (GObjectClass *class, pspec = find_pspec (class, property_name); - if (pspec && ((GTypeInstance *)pspec)->g_class->g_type == G_TYPE_PARAM_OVERRIDE) - pspec = ((GParamSpecOverride *)pspec)->overridden; + if (pspec) + param_spec_follow_override (&pspec); return pspec; } @@ -1432,14 +1440,6 @@ g_object_freeze_notify (GObject *object) g_object_unref (object); } -/* Inlined version of g_param_spec_get_redirect_target(), for speed */ -static inline void -param_spec_follow_override (GParamSpec **pspec) -{ - if (((GTypeInstance *) (*pspec))->g_class->g_type == G_TYPE_PARAM_OVERRIDE) - *pspec = ((GParamSpecOverride *) (*pspec))->overridden; -} - static inline void g_object_notify_by_spec_internal (GObject *object, GParamSpec *pspec) From 426724d51ca46cec30400f504706c947de2ab03c Mon Sep 17 00:00:00 2001 From: Matthias Clasen Date: Tue, 7 Jun 2022 12:02:58 -0400 Subject: [PATCH 3/3] Add some tests around properties Install the properties with a mixture of g_object_class_install_properties and g_object_class_install_properties, and verify that finding them still works, regardless of whether we use string literals or not. --- gobject/tests/properties.c | 138 +++++++++++++++++++++++++++++++++++-- 1 file changed, 132 insertions(+), 6 deletions(-) diff --git a/gobject/tests/properties.c b/gobject/tests/properties.c index 3695ee123..447a7cf04 100644 --- a/gobject/tests/properties.c +++ b/gobject/tests/properties.c @@ -173,22 +173,29 @@ test_object_class_init (TestObjectClass *klass) properties[PROP_FOO] = g_param_spec_int ("foo", "Foo", "Foo", -1, G_MAXINT, 0, - G_PARAM_READWRITE); + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); properties[PROP_BAR] = g_param_spec_boolean ("bar", "Bar", "Bar", FALSE, - G_PARAM_READWRITE); + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); properties[PROP_BAZ] = g_param_spec_string ("baz", "Baz", "Baz", NULL, G_PARAM_READWRITE); - properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux", - NULL, - G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY); gobject_class->set_property = test_object_set_property; gobject_class->get_property = test_object_get_property; gobject_class->finalize = test_object_finalize; - g_object_class_install_properties (gobject_class, N_PROPERTIES, properties); + g_object_class_install_properties (gobject_class, N_PROPERTIES - 1, properties); + + /* We intentionally install this property separately, to test + * that that works, and that property lookup works regardless + * how the property was installed. + */ + properties[PROP_QUUX] = g_param_spec_string ("quux", "quux", "quux", + NULL, + G_PARAM_READWRITE | G_PARAM_EXPLICIT_NOTIFY); + + g_object_class_install_property (gobject_class, PROP_QUUX, properties[PROP_QUUX]); } static void @@ -205,12 +212,130 @@ properties_install (void) { TestObject *obj = g_object_new (test_object_get_type (), NULL); GParamSpec *pspec; + char *name; g_assert (properties[PROP_FOO] != NULL); pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "foo"); g_assert (properties[PROP_FOO] == pspec); + name = g_strdup ("bar"); + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), name); + g_assert (properties[PROP_BAR] == pspec); + g_free (name); + + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "baz"); + g_assert (properties[PROP_BAZ] == pspec); + + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "quux"); + g_assert (properties[PROP_QUUX] == pspec); + + g_object_unref (obj); +} + +typedef struct { + GObject parent_instance; + int value[16]; +} ManyProps; + +typedef GObjectClass ManyPropsClass; + +static GParamSpec *props[16]; + +GType many_props_get_type (void) G_GNUC_CONST; + +G_DEFINE_TYPE(ManyProps, many_props, G_TYPE_OBJECT) + +static void +many_props_init (ManyProps *self) +{ +} + +static void +get_prop (GObject *object, + guint prop_id, + GValue *value, + GParamSpec *pspec) +{ + ManyProps *mp = (ManyProps *) object; + + if (prop_id > 0 && prop_id < 13) + g_value_set_int (value, mp->value[prop_id]); + else + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); +} + +static void +set_prop (GObject *object, + guint prop_id, + const GValue *value, + GParamSpec *pspec) +{ + ManyProps *mp = (ManyProps *) object; + + if (prop_id > 0 && prop_id < 13) + mp->value[prop_id] = g_value_get_int (value); + else + G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); +} + +static void +many_props_class_init (ManyPropsClass *class) +{ + G_OBJECT_CLASS (class)->get_property = get_prop; + G_OBJECT_CLASS (class)->set_property = set_prop; + + props[1] = g_param_spec_int ("one", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[2] = g_param_spec_int ("two", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[3] = g_param_spec_int ("three", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[4] = g_param_spec_int ("four", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[5] = g_param_spec_int ("five", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[6] = g_param_spec_int ("six", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[7] = g_param_spec_int ("seven", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[8] = g_param_spec_int ("eight", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[9] = g_param_spec_int ("nine", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[10] = g_param_spec_int ("ten", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[11] = g_param_spec_int ("eleven", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + props[12] = g_param_spec_int ("twelve", NULL, NULL, + 0, G_MAXINT, 0, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + g_object_class_install_properties (G_OBJECT_CLASS (class), 12, props); +} + +static void +properties_install_many (void) +{ + ManyProps *obj = g_object_new (many_props_get_type (), NULL); + GParamSpec *pspec; + + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "one"); + g_assert (props[1] == pspec); + + pspec = g_object_class_find_property (G_OBJECT_GET_CLASS (obj), "ten"); + g_assert (props[10] == pspec); + g_object_unref (obj); } @@ -639,6 +764,7 @@ main (int argc, char *argv[]) g_test_init (&argc, &argv, NULL); g_test_add_func ("/properties/install", properties_install); + g_test_add_func ("/properties/install-many", properties_install_many); g_test_add_func ("/properties/notify", properties_notify); g_test_add_func ("/properties/notify-queue", properties_notify_queue); g_test_add_func ("/properties/construct", properties_construct);