[ABI] Vala Interface+Abstract class pair creates double indirection
@mpiechotka
Submitted by Maciej Marcin Piechotka Link to original bug (#706712)
Description
Vala creates double indirection when implementing interface by virtual methods. This was discussed on IRC and by blog post[1] however the original solution is incorrect - I put the discussion here to put the arguments etc. in one place (sorry for any disorganization it might cause but it is good to also have the invalid arguments documented).
The proposed solution was to update the both abstract method as well as interface in class but this changes both ABI and C API as the subclasses need to do the same. Let say we have interface I with method f, abstract class A implementing it and classes B and C (where C extends B, which extends A) where only B is compiled using the new method. it will override the I.f and A.f. C.f will override A.f but not I.f. Therefore calling method f on C will call B.f instead of C.f.
Possible solution would be to have a dynamic fix in abstract class. I.e. instead of adding simply:
static void gee_abstract_list_gee_list_interface_init (GeeListIface * iface) {
(...)
iface->set = (void (*)(GeeList*, gint, gconstpointer)) gee_abstract_list_set;
(...)
}
void gee_abstract_list_set (GeeAbstractList* self, gint index, gconstpointer item) {
g_return_if_fail (self != NULL);
GEE_ABSTRACT_LIST_GET_CLASS (self)->set (self, index, item);
}
Implement it in following way:
static void gee_abstract_list_gee_list_interface_init (GeeListIface * iface) {
(...)
iface->set = (void (*)(GeeList*, gint, gconstpointer)) gee_abstract_list_impl_set;
(...)
}
static void
gee_abstract_list_impl_set (GeeAbstractList* self, gint index, gconstpointer item) {
void (*real)(GeeList*, gint, gconstpointer);
real = GEE_ABSTRACT_LIST_GET_CLASS (self)->set;
GEE_LIST_GET_INTERFACE (self)->set = real;
real (self, index, item);
}
void gee_abstract_list_set (GeeAbstractList* self, gint index, gconstpointer item) {
g_return_if_fail (self != NULL);
GEE_ABSTRACT_LIST_GET_CLASS (self)->set (self, index, item);
}
This however cause other problem - it would be yet another violation of the strict C11 spec[2] for multi-thread access. While it should not cause problem on major architectures (x86, arm, ...) it might on architectures where word access is not atomic[3] (DEC Alpha for example). Using traditional atomics is too expensive and likely they would overcome any potential benefits due to cost of memory barrier. However the C11 primitives allows to avoid the barrier entirely:
// list.c
void gee_list_set (GeeList* self, gint index, gconstpointer item) {
void (*real)(GeeList*, gint, gconstpointer);
g_return_if_fail (self != NULL);
real = atomic_load_explicit (&GEE_LIST_GET_INTERFACE (self)->set, memory_order_relaxed);
&GEE_LIST_GET_INTERFACE (self)->set (self, index, item);
}
// abstractlist.c
static void gee_abstract_list_gee_list_interface_init (GeeListIface * iface) {
(...)
iface->set = ATOMIC_INIT ((void (*)(GeeList*, gint, gconstpointer)) gee_abstract_list_impl_set);
(...)
}
static void
gee_abstract_list_impl_set (GeeAbstractList* self, gint index, gconstpointer item) {
void (*real)(GeeList*, gint, gconstpointer);
real = GEE_ABSTRACT_LIST_GET_CLASS (self)->set;
atomic_store_explicit (GEE_LIST_GET_INTERFACE (self)->set = real, real, memory_order_relaxed);
real (self, index, item);
}
void gee_abstract_list_set (GeeAbstractList* self, gint index, gconstpointer item) {
g_return_if_fail (self != NULL);
GEE_ABSTRACT_LIST_GET_CLASS (self)->set (self, index, item);
}
This on the other hand requires changing the interface both in terms of implementation of gee_list_set as well as the C API and possibly ABI (depending on implementation of compiler). Furthermore the standard-compilant stdthread.h is not present on current systems and relevant optimizations have been introduced only in newest compilers.
The possible solution is to extend GObject. It would be similar to g_type_add_interface_check[4] only called after a abstract class implementation instead of interface.
[1] http://blog.piechotka.com.pl/2013/05/18/vala-abi-and-branch-prediction/
[2] Other problems include, for example, storing integer in pointer.
[3] Here atomic does not mean synchronized. It corresponds to relaxed memory ordering in C11 standard - i.e. it is guaranteed that the value read is one of values written (so there is no half-written words). It is often hard to find this information as usually 'atomic' refers to more old-fashioned
[4] https://developer.gnome.org/gobject/2.34/gobject-Type-Information.html#g-type-add-interface-check