From f02ec2f2de3c9863da36be951662ddf4080adfaa Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Wed, 22 Feb 2012 19:36:05 +0100 Subject: [PATCH] Optimize single-handler va_marshaller case When there is only one closure handling a signal emission and it doesn't have a bunch of complicated features enabled we can short circuit the va_args collection into GValues and call the callback via the va_marshaller directly. https://bugzilla.gnome.org/show_bug.cgi?id=661140 --- gobject/gsignal.c | 344 ++++++++++++++++++++++++++++++++-------------- gobject/gvaluecollector.h | 30 ++++ 2 files changed, 274 insertions(+), 100 deletions(-) diff --git a/gobject/gsignal.c b/gobject/gsignal.c index 5b5d301..54542c1 100644 --- a/gobject/gsignal.c +++ b/gobject/gsignal.c @@ -106,7 +106,9 @@ #define REPORT_BUG "please report occurrence circumstances to gtk-devel-list@gnome.org" #ifdef G_ENABLE_DEBUG -#define IF_DEBUG(debug_type, cond) if ((_g_type_debug_flags & G_TYPE_DEBUG_ ## debug_type) || cond) +#define COND_DEBUG(debug_type, cond) ((_g_type_debug_flags & G_TYPE_DEBUG_ ## debug_type) || (cond)) +#define IF_DEBUG(debug_type, cond) if (COND_DEBUG(debug_type, cond)) + static volatile gpointer g_trace_instance_signals = NULL; static volatile gpointer g_trap_instance_signals = NULL; #endif /* G_ENABLE_DEBUG */ @@ -181,6 +183,7 @@ static gboolean signal_emit_unlocked_R (SignalNode *node, const GValue *instance_and_params); static const gchar * type_debug_name (GType type); static void node_check_deprecated (const SignalNode *node); +static void node_update_single_va_closure (SignalNode *node); /* --- structures --- */ @@ -205,9 +208,10 @@ struct _SignalNode guint destroyed : 1; /* reinitializable portion */ - guint test_class_offset : 12; guint flags : 9; guint n_params : 8; + guint single_va_closure_is_valid : 1; + guint single_va_closure_is_after : 1; GType *param_types; /* mangled with G_SIGNAL_TYPE_STATIC_SCOPE flag */ GType return_type; /* mangled with G_SIGNAL_TYPE_STATIC_SCOPE flag */ GBSearchArray *class_closure_bsa; @@ -215,9 +219,11 @@ struct _SignalNode GSignalCMarshaller c_marshaller; GSignalCVaMarshaller va_marshaller; GHookList *emission_hooks; + + GClosure *single_va_closure; }; -#define MAX_TEST_CLASS_OFFSET (4096) /* 2^12, 12 bits for test_class_offset */ -#define TEST_CLASS_MAGIC (1) /* indicates NULL class closure, candidate for NOP optimization */ + +#define SINGLE_VA_CLOSURE_EMPTY_MAGIC GINT_TO_POINTER(1) /* indicates single_va_closure is valid but empty */ struct _SignalKey { @@ -687,6 +693,47 @@ handler_insert (guint signal_id, hlist->tail_after = handler; } +static void +node_update_single_va_closure (SignalNode *node) +{ + GClosure *closure = NULL; + gboolean is_after = FALSE; + + /* Fast path single-handler without boxing the arguments in GValues */ + if (G_TYPE_IS_OBJECT (node->itype) && + (node->flags & (G_SIGNAL_NO_RECURSE|G_SIGNAL_MUST_COLLECT)) == 0 && + (node->emission_hooks == NULL || node->emission_hooks->hooks == NULL)) + { + GSignalFlags run_type; + ClassClosure * cc; + GBSearchArray *bsa = node->class_closure_bsa; + + if (bsa == NULL || bsa->n_nodes == 0) + closure = SINGLE_VA_CLOSURE_EMPTY_MAGIC; + else if (bsa->n_nodes == 1) + { + /* Look for default class closure (can't support non-default as it + chains up using GValues */ + cc = g_bsearch_array_get_nth (bsa, &g_class_closure_bconfig, 0); + if (cc->instance_type == 0) + { + run_type = node->flags & (G_SIGNAL_RUN_FIRST|G_SIGNAL_RUN_LAST|G_SIGNAL_RUN_CLEANUP); + /* Only support *one* of run-first or run-last, not multiple or cleanup */ + if (run_type == G_SIGNAL_RUN_FIRST || + run_type == G_SIGNAL_RUN_LAST) + { + closure = cc->closure; + is_after = (run_type == G_SIGNAL_RUN_LAST); + } + } + } + } + + node->single_va_closure_is_valid = TRUE; + node->single_va_closure = closure; + node->single_va_closure_is_after = is_after; +} + static inline void emission_push (Emission **emission_list_p, Emission *emission) @@ -943,6 +990,7 @@ g_signal_add_emission_hook (guint signal_id, node->emission_hooks->seq_id = seq_hook_id; g_hook_append (node->emission_hooks, hook); seq_hook_id = node->emission_hooks->seq_id; + SIGNAL_UNLOCK (); return hook->hook_id; @@ -971,6 +1019,9 @@ g_signal_remove_emission_hook (guint signal_id, g_warning ("%s: invalid signal id `%u'", G_STRLOC, signal_id); else if (!node->emission_hooks || !g_hook_destroy (node->emission_hooks, hook_id)) g_warning ("%s: signal \"%s\" had no hook (%lu) to remove", G_STRLOC, node->name, hook_id); + + node->single_va_closure_is_valid = FALSE; + SIGNAL_UNLOCK (); } @@ -1346,19 +1397,6 @@ g_signal_new (const gchar *signal_name, va_end (args); - /* optimize NOP emissions with NULL class handlers */ - if (signal_id && G_TYPE_IS_INSTANTIATABLE (itype) && return_type == G_TYPE_NONE && - class_offset && class_offset < MAX_TEST_CLASS_OFFSET && - ~signal_flags & G_SIGNAL_MUST_COLLECT) - { - SignalNode *node; - - SIGNAL_LOCK (); - node = LOOKUP_SIGNAL_NODE (signal_id); - node->test_class_offset = class_offset; - SIGNAL_UNLOCK (); - } - return signal_id; } @@ -1481,8 +1519,7 @@ signal_add_class_closure (SignalNode *node, { ClassClosure key; - /* can't optimize NOP emissions with overridden class closures */ - node->test_class_offset = 0; + node->single_va_closure_is_valid = FALSE; if (!node->class_closure_bsa) node->class_closure_bsa = g_bsearch_array_create (&g_class_closure_bconfig); @@ -1636,9 +1673,9 @@ g_signal_newv (const gchar *signal_name, TRACE(GOBJECT_SIGNAL_NEW(signal_id, name, itype)); } node->destroyed = FALSE; - node->test_class_offset = 0; /* setup reinitializable portion */ + node->single_va_closure_is_valid = FALSE; node->flags = signal_flags & G_SIGNAL_FLAGS_MASK; node->n_params = n_params; node->param_types = g_memdup (param_types, sizeof (GType) * n_params); @@ -1708,13 +1745,7 @@ g_signal_newv (const gchar *signal_name, node->emission_hooks = NULL; if (class_closure) signal_add_class_closure (node, 0, class_closure); - else if (G_TYPE_IS_INSTANTIATABLE (itype) && - return_type == G_TYPE_NONE && - ~signal_flags & G_SIGNAL_MUST_COLLECT) - { - /* optimize NOP emissions */ - node->test_class_offset = TEST_CLASS_MAGIC; - } + SIGNAL_UNLOCK (); g_free (name); @@ -1744,6 +1775,9 @@ g_signal_set_va_marshaller (guint signal_id, _g_closure_set_va_marshal (cc->closure, va_marshaller); } } + + node->single_va_closure_is_valid = FALSE; + SIGNAL_UNLOCK (); } @@ -1817,7 +1851,7 @@ signal_destroy_R (SignalNode *signal_node) signal_node->destroyed = TRUE; /* reentrancy caution, zero out real contents first */ - signal_node->test_class_offset = 0; + signal_node->single_va_closure_is_valid = FALSE; signal_node->n_params = 0; signal_node->param_types = NULL; signal_node->return_type = 0; @@ -2887,50 +2921,6 @@ g_signal_has_handler_pending (gpointer instance, return has_pending; } -static inline gboolean -signal_check_skip_emission (SignalNode *node, - gpointer instance, - GQuark detail) -{ - HandlerList *hlist; - - /* are we able to check for NULL class handlers? */ - if (!node->test_class_offset) - return FALSE; - - /* are there emission hooks pending? */ - if (node->emission_hooks && node->emission_hooks->hooks) - return FALSE; - - /* is there a non-NULL class handler? */ - if (node->test_class_offset != TEST_CLASS_MAGIC) - { - GTypeClass *class = G_TYPE_INSTANCE_GET_CLASS (instance, G_TYPE_FROM_INSTANCE (instance), GTypeClass); - - if (G_STRUCT_MEMBER (gpointer, class, node->test_class_offset)) - return FALSE; - } - - /* are signals being debugged? */ -#ifdef G_ENABLE_DEBUG - IF_DEBUG (SIGNALS, g_trace_instance_signals || g_trap_instance_signals) - return FALSE; -#endif /* G_ENABLE_DEBUG */ - - /* is this a no-recurse signal already in emission? */ - if (node->flags & G_SIGNAL_NO_RECURSE && - emission_find (g_restart_emissions, node->signal_id, detail, instance)) - return FALSE; - - /* do we have pending handlers? */ - hlist = handler_list_lookup (node->signal_id, instance); - if (hlist && hlist->handlers) - return FALSE; - - /* none of the above, no emission required */ - return TRUE; -} - /** * g_signal_emitv: * @instance_and_params: (array): argument list for the signal emission. @@ -3021,18 +3011,49 @@ g_signal_emitv (const GValue *instance_and_params, #endif /* G_ENABLE_DEBUG */ /* optimize NOP emissions */ - if (signal_check_skip_emission (node, instance, detail)) + if (!node->single_va_closure_is_valid) + node_update_single_va_closure (node); + + if (node->single_va_closure != NULL && + (node->single_va_closure == SINGLE_VA_CLOSURE_EMPTY_MAGIC || + _g_closure_is_void (node->single_va_closure, instance)) +#ifdef G_ENABLE_DEBUG + && !COND_DEBUG (SIGNALS, g_trace_instance_signals != instance && + g_trap_instance_signals == instance) +#endif /* G_ENABLE_DEBUG */ + ) { - /* nothing to do to emit this signal */ - SIGNAL_UNLOCK (); - /* g_printerr ("omitting emission of \"%s\"\n", node->name); */ - return; + HandlerList* hlist = handler_list_lookup (node->signal_id, instance); + if (hlist == NULL || hlist->handlers == NULL) + { + /* nothing to do to emit this signal */ + SIGNAL_UNLOCK (); + /* g_printerr ("omitting emission of \"%s\"\n", node->name); */ + return; + } } SIGNAL_UNLOCK (); signal_emit_unlocked_R (node, detail, instance, return_value, instance_and_params); } +static inline gboolean +accumulate (GSignalInvocationHint *ihint, + GValue *return_accu, + GValue *handler_return, + SignalAccumulator *accumulator) +{ + gboolean continue_emission; + + if (!accumulator) + return TRUE; + + continue_emission = accumulator->func (ihint, return_accu, handler_return, accumulator->data); + g_value_reset (handler_return); + + return continue_emission; +} + /** * g_signal_emit_valist: * @instance: the instance the signal is being emitted on. @@ -3079,13 +3100,153 @@ g_signal_emit_valist (gpointer instance, } #endif /* !G_DISABLE_CHECKS */ - /* optimize NOP emissions */ - if (signal_check_skip_emission (node, instance, detail)) + if (!node->single_va_closure_is_valid) + node_update_single_va_closure (node); + + if (node->single_va_closure != NULL +#ifdef G_ENABLE_DEBUG + && !COND_DEBUG (SIGNALS, g_trace_instance_signals != instance && + g_trap_instance_signals == instance) +#endif /* G_ENABLE_DEBUG */ + ) { - /* nothing to do to emit this signal */ - SIGNAL_UNLOCK (); - /* g_printerr ("omitting emission of \"%s\"\n", node->name); */ - return; + HandlerList* hlist = handler_list_lookup (node->signal_id, instance); + Handler *l; + GClosure *closure = NULL; + gboolean fastpath = TRUE; + GSignalFlags run_type = G_SIGNAL_RUN_FIRST; + + if (node->single_va_closure != SINGLE_VA_CLOSURE_EMPTY_MAGIC && + !_g_closure_is_void (node->single_va_closure, instance)) + { + if (_g_closure_supports_invoke_va (node->single_va_closure)) + { + closure = node->single_va_closure; + if (node->single_va_closure_is_after) + run_type = G_SIGNAL_RUN_LAST; + else + run_type = G_SIGNAL_RUN_FIRST; + } + else + fastpath = FALSE; + } + + for (l = hlist ? hlist->handlers : NULL; fastpath && l != NULL; l = l->next) + { + if (!l->block_count && + (!l->detail || l->detail == detail)) + { + if (closure != NULL || !_g_closure_supports_invoke_va (l->closure)) + { + fastpath = FALSE; + break; + } + else + { + closure = l->closure; + if (l->after) + run_type = G_SIGNAL_RUN_LAST; + else + run_type = G_SIGNAL_RUN_FIRST; + } + } + } + + if (fastpath && closure == NULL && node->return_type == G_TYPE_NONE) + { + SIGNAL_UNLOCK (); + return; + } + + if (fastpath) + { + SignalAccumulator *accumulator; + Emission emission; + GValue *return_accu, accu = G_VALUE_INIT; + guint signal_id; + GValue emission_return = G_VALUE_INIT; + GType rtype = node->return_type & ~G_SIGNAL_TYPE_STATIC_SCOPE; + gboolean static_scope = node->return_type & G_SIGNAL_TYPE_STATIC_SCOPE; + + signal_id = node->signal_id; + accumulator = node->accumulator; + if (rtype == G_TYPE_NONE) + return_accu = NULL; + else if (accumulator) + return_accu = &accu; + else + return_accu = &emission_return; + + emission.instance = instance; + emission.ihint.signal_id = node->signal_id; + emission.ihint.detail = detail; + emission.ihint.run_type = run_type; + emission.state = EMISSION_RUN; + emission.chain_type = G_TYPE_FROM_INSTANCE (instance); + emission_push (&g_recursive_emissions, &emission); + + SIGNAL_UNLOCK (); + + TRACE(GOBJECT_SIGNAL_EMIT(signal_id, detail, instance, G_TYPE_FROM_INSTANCE (instance))); + + if (rtype != G_TYPE_NONE) + g_value_init (&emission_return, rtype); + + if (accumulator) + g_value_init (&accu, rtype); + + if (closure != NULL) + { + g_object_ref (instance); + _g_closure_invoke_va (closure, + return_accu, + instance, + var_args, + node->n_params, + node->param_types); + accumulate (&emission.ihint, &emission_return, &accu, accumulator); + g_object_unref (instance); + } + + SIGNAL_LOCK (); + + emission.chain_type = G_TYPE_NONE; + emission_pop (&g_recursive_emissions, &emission); + + SIGNAL_UNLOCK (); + + if (accumulator) + g_value_unset (&accu); + + if (rtype != G_TYPE_NONE) + { + gchar *error = NULL; + for (i = 0; i < node->n_params; i++) + { + GType ptype = node->param_types[i] & ~G_SIGNAL_TYPE_STATIC_SCOPE; + G_VALUE_COLLECT_SKIP (ptype, var_args); + } + + G_VALUE_LCOPY (&emission_return, + var_args, + static_scope ? G_VALUE_NOCOPY_CONTENTS : 0, + &error); + if (!error) + g_value_unset (&emission_return); + else + { + g_warning ("%s: %s", G_STRLOC, error); + g_free (error); + /* we purposely leak the value here, it might not be + * in a sane state if an error condition occurred + */ + } + } + + TRACE(GOBJECT_SIGNAL_EMIT_END(signal_id, detail, instance, G_TYPE_FROM_INSTANCE (instance))); + + return; + } } n_params = node->n_params; @@ -3227,23 +3388,6 @@ g_signal_emit_by_name (gpointer instance, g_warning ("%s: signal name `%s' is invalid for instance `%p'", G_STRLOC, detailed_signal, instance); } -static inline gboolean -accumulate (GSignalInvocationHint *ihint, - GValue *return_accu, - GValue *handler_return, - SignalAccumulator *accumulator) -{ - gboolean continue_emission; - - if (!accumulator) - return TRUE; - - continue_emission = accumulator->func (ihint, return_accu, handler_return, accumulator->data); - g_value_reset (handler_return); - - return continue_emission; -} - static gboolean signal_emit_unlocked_R (SignalNode *node, GQuark detail, diff --git a/gobject/gvaluecollector.h b/gobject/gvaluecollector.h index 9bdf482..6d5190e 100644 --- a/gobject/gvaluecollector.h +++ b/gobject/gvaluecollector.h @@ -158,6 +158,36 @@ G_STMT_START { \ G_VALUE_COLLECT_INIT(value, _value_type, var_args, flags, __error); \ } G_STMT_END +#define G_VALUE_COLLECT_SKIP(_value_type, var_args) \ +G_STMT_START { \ + GTypeValueTable *_vtable = g_type_value_table_peek (_value_type); \ + gchar *_collect_format = _vtable->collect_format; \ + \ + while (*_collect_format) \ + { \ + switch (*_collect_format++) \ + { \ + case G_VALUE_COLLECT_INT: \ + va_arg ((var_args), gint); \ + break; \ + case G_VALUE_COLLECT_LONG: \ + va_arg ((var_args), glong); \ + break; \ + case G_VALUE_COLLECT_INT64: \ + va_arg ((var_args), gint64); \ + break; \ + case G_VALUE_COLLECT_DOUBLE: \ + va_arg ((var_args), gdouble); \ + break; \ + case G_VALUE_COLLECT_POINTER: \ + va_arg ((var_args), gpointer); \ + break; \ + default: \ + g_assert_not_reached (); \ + } \ + } \ +} G_STMT_END + /** * G_VALUE_LCOPY: * @value: a #GValue return location. @value is supposed to be initialized -- 2.7.4