Complex signal arguments are passed by value and needlessly copied in both stub and proxy

[martin-ejdestig]

Perhaps not that bad for a string, but e.g. have a signal that results in something like this in:

*_proxy.h:

    sigc::signal<void, Glib::DBusObjectPathString, std::map<Glib::ustring,Glib::VariantBase>> Foo_signal;

*_proxy.cpp:Proxy::handle_signal():

        if (parameters.get_n_children() != 2) return;
        Glib::Variant<Glib::DBusObjectPathString> base_arg1;
        parameters.get_child(base_arg1, 0);
        Glib::DBusObjectPathString p_arg1;
        p_arg1 = base_arg1.get();

        if (parameters.get_n_children() != 2) return;
        Glib::Variant<std::map<Glib::ustring,Glib::VariantBase>> base_arg2;
        parameters.get_child(base_arg2, 1);
        std::map<Glib::ustring,Glib::VariantBase> p_arg2;
        p_arg2 = base_properties.get();

        Foo_signal.emit((p_arg1), (p_arg2));

*_stub.cpp:

    void Foo_emitter(Glib::DBusObjectPathString, std::map<Glib::ustring,Glib::VariantBase>);
    sigc::signal<void, Glib::DBusObjectPathString, std::map<Glib::ustring,Glib::VariantBase>> Foo_signal;

void Stub::Foo_emitter(Glib::DBusObjectPathString arg1, std::map<Glib::ustring,Glib::VariantBase> arg2)
{
    std::vector<Glib::VariantBase> paramsList;

    paramsList.push_back(Glib::Variant<Glib::DBusObjectPathString>::create((arg1)));;
    paramsList.push_back(Glib::Variant<std::map<Glib::ustring,Glib::VariantBase>>::create((arg2)));;
    ...
}

Should perhaps add const & or && and std::move for non simple types where applicable.

[martin-ejdestig]

The std::map could potentially be very large. Perhaps it is not a problem in practice due to IPC overhead overshadowing cost of copying. I think Glib::Variant* is reference counted so perhaps copying is negligible (GVariant definetely is, but not sure if Glibmm wrapped class does a deep copy of the underlying GVariant or not).

@mardy @kursatkobya @jonte Any experience with using signals with complex types and large values in the generator? Pondering if this is worth looking into or not.

[martin-ejdestig]

Built Glibmm locally and Glib::VariantBase does this:

VariantBase::VariantBase(const VariantBase& src)
:
  gobject_ ((src.gobject_) ? g_variant_ref_sink(src.gobject_) : nullptr)
{}

VariantBase& VariantBase::operator=(const VariantBase& src)
{
  const auto new_gobject = (src.gobject_) ? g_variant_ref_sink(src.gobject_) : nullptr;

  if(gobject_)
    g_variant_unref(gobject_);

  gobject_ = new_gobject;

  return *this;
}

So cost of copying complex container types is reference counting on all elements. And in this particular case, copies of std::string (backing used for Glib::ustring) for the keys.

[mardy]

I'm always for optimizing :-)

But in this case I'm not sure we can: I don't see much room to improve the sub side, because it needs anyway to copy those values into the VariantBase objects. But making the parameters const references does makes sense in any case.

As for the proxy side, yes, I think we can save a copy in the emission of the local signal if we mark its parameters to be const references.