Created attachment 181093 [details] [review]
vala: use g_signal_emit when possible

Ok I was intrigued by this, so I started working on this...

I found that instead of using global guint values (then to be exported as external in other files), it was also possible use type-struct values to keep everything in the same place... Plus, this behavior can't be used always, since the signal_id can't be always exported and external bindings doesn't use it.

However, attached you can find my work, please test it with your benchmark (als if I've already did and it seems fine).

-------

As experienced in some benchmarks, the usage of g_signal_emit_by_name can cause
some performance loss. When possible emitting a signal in a vala class / interface
should be performed via g_signal_emit using the signal_id returned by g_signal_new.

However this is possible only when the used interface / class is compiled in the
same scope of the vala file that is emitting the signal.
In fact, to perform this some *_signal_id guid variables are added (avoiding name
clash with other default signals func pointers) to the type definition struct of
the class / interface and they are assigned to signals ID returned by g_signal_new.
Then when a signal should be emitted, if the signal has saved its signal_id,
g_signal_emit is called, using the reltive *_signal_id function.

This always works in all the cases I've tested, when emitting the signal in a
different file than the one where the class / interface is defined, their type
structs could be redefined in the *.c file.

Of course, when using *.vapi files and emitting signals on external objects the
standard g_signal_emit_by_name method is used.

I confirm performance gains with this patch.

To be more specific:

It gives significant speedup, on my computer it's like 1.025 to 0.86.

As I can see in the patch description, there are significant limitations. If the class comes from external vapi, old style (calling signal by name) will be used. Maybe some attributes can be used to control this? So for example:

- each signal ID is in the object's Type structure, and has some name, unless some attribute is used,
- CCode has_signal_id=false can be set in vapis on bindings to external libraries to indicate, that the particular signal's ID is not stored in the Type,
- maybe some CCode signal_id_name="foo" can be used to control how is the ID named in the Type struct?
- maybe even something like CCode signal_id_path="Namespace.[int|lass.static_member]" to be able to store signal ID anywhere you like?

(In reply to comment #3)
correction of a typo:
> - maybe even something like CCode
> signal_id_path="Namespace.[int|Class.static_member]" to be able to store signal
> ID anywhere you like?

(In reply to comment #1)
> In fact, to perform this some *_signal_id guid variables are added (avoiding
> name
> clash with other default signals func pointers) to the type definition struct

Unfortunately we can't do that as it would break ABI, so accessing the ids directly seems impossible. I happened to stumble on a blog comment from ebassi on this matter.

http://arunchaganty.wordpress.com/2008/05/18/i-am-a-gobject/#comment-104

So it seems the proper way to do this would be:
 - for each class/iface emit an enum holding the signals and a *_LAST_SIGNAL value
 - for each class/iface emit a static array of size *_LAST_SIGNAL and use it for signal creation/emission

Also, if bug 681356 was fixed, it would be possible to generate emitters that make use of the signal ids.

Created attachment 337462 [details] [review]
Use g_signal_emit where possible

This includes basically the above, namely:

- An enumeration is generated, holding capitalized signal names.
- Signal ids are stored in a static array.
- When emitting a signal in the same file the signal was declared, g_signal_emit is invoked using the stored ids.
- Everywhere else g_signal_emit_by_name or the emitter is used as before.

The signal enum generation is broken for inherited signals from interfaces.


public interface Bar : GLib.Object {
	//[HasEmitter]
	public signal int bar (int i);
}

public class Foo : GLib.Object, Bar {
}

void main () {
	var f = new Foo ();

	f.bar.connect (i => i + 12);

	var res = f.bar (30);
	assert (res == 42);
}

Created attachment 337530 [details] [review]
Use g_signal_emit where possible

For interfaces, the LAST_SIGNAL value was generated before visiting the children, so the signal array had size zero. Also, there was no test case for interfaces with signals that could have caught this.

commit 3bbf223347d9280accb608c90672312d4bd30a05
Author: Simon Werbeck <simon.werbeck@gmail.com>
Date:   Wed Oct 5 19:01:44 2016 +0200

    codegen: Use g_signal_emit where possible