Optimize single-handler va_marshaller case
authorAlexander Larsson <alexl@redhat.com>
Wed, 22 Feb 2012 18:36:05 +0000 (19:36 +0100)
committerAlexander Larsson <alexl@redhat.com>
Fri, 2 Mar 2012 16:13:03 +0000 (17:13 +0100)
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
gobject/gvaluecollector.h

index 5b5d301..54542c1 100644 (file)
 
 #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,
index 9bdf482..6d5190e 100644 (file)
@@ -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