Revisiting QWidgets & data, refactoring and performance

published at 18.02.2016 11:10 by Jens Weller
Save to Instapaper Pocket

My CMS project has grown quite a bit, and there are a few places where I think I should refactor the code. One of the larger ones is that TreeItem::get<T> returns a pointer instead of a reference. Another one is related to how the Qt UI application is acting when opening a new panel in the TabControl. There used to be a noticeable delay...

C++ gives you a lot of performance out of the box, just by being close to the model and free from abstractions to enforce "safety" on the stupid programmer. My code usually is not heavily depending on performance, it runs not on a server or is an app with fancy animations, neither do I write games where after a few ms the current game loop should be finished. If your code does run in such an environment I'd like to recommend to you the Keynote at Meeting C++ 2015 from Chandler Carruth on Optimization and his talk from CppCon on benchmarking performance. Also, there are two excellent blog articles on benchmarking with micro benchmarking libraries: overview and an example.

In my case I did not need to benchmark or profile my code, it was fairly obvious which part of the code triggered it, and I needed to understand why, and fix it. I already knew the problem at CppCon, so I was really interested in trying out perf after Chandlers plenary talk, but Stephan T. Lavavej already gave me a hint in his talk on <functional>. My code uses std::function quite often, and in some cases I had decided to take such a parameter as a sink parameter (or was I just lazy there?), but this seems not to work as planned, at least not with the gcc version (4.8.x) I'm using.

Turning the sink parameters into const references improved the situation a lot. Avoiding a copy of a std::function object can also avoid heap allocation. std::function uses type erasure with an internal base class and inside the template derived class calling the actual callee through a virtual method. But, std::function is allowed to optimize this in some cases, when it can do a small object optimization. Then the data for the functor is stored internally, e.g. the argument is only a function pointer or an instance of std::reference_wrapper. So avoiding copies of std::function objects did improve the situation. Yet I'm not satisfied with the related code, there is still a lot of allocations, and at least when debugging under heavy system use (parallel video encoding e.g.), its slightly visible again.

Revisiting QWidgets and Data

I wrote last August, how I exchange the data into my model when its displayed inside a panel. Each Widget interacts with my model through a mechanism that transfers the data to the model after the widget looses focus. For this an EventFilter class is derived from QObject and a virtual function is overwritten. The EventFilter class takes an std::function object, and in the virtual method this functor object is invoked. In August that was a fitting solution, after all I'm writing a CMS not a QWidget framework. So I moved on, yet always when implementing a new panel class, I feld a little dirty for writing code like this:

ui->txt_pagename->installEventFilter(new EventFilter(filter,this));

ui->txt_pagealias->installEventFilter(new EventFilter(Filter<std::string>(std::bind([&]Page::setAlias,page,std::placeholders::_1),[&]getText<QLineEdit>),this));

ui->titleLineEdit->installEventFilter(new EventFilter(Filter<std::string>(std::bind([&]Page::setAlias,page,std::placeholders::_1),[&]getText<QLineEdit>),this));
...

Filter is a lambda, which you can see in my earlier blog post in its full beauty, Filter is a template, which takes again two std::function objects, one for retrieving the value, and the first for setting it. I do actually have now a little framework for exchanging values from QWidgets to my model. As extracting the value out of the widget has no common interface, there is a getINTERFACENAME method for the most common ones as a template. A big misconception in the EventFilter class is, that it has a 1:1 relation to the widget its used in. Each gets a nicely with new allocated instance, which then is parented to the currently constructed panel. So for each widget instance there are a few heap allocations, first EventFilter it self, and then a few hidden ones through std::function copies. Getting rid of the std::function copies has improved the situation, but I still don't like the code,  so the final step is to get rid of all the news creating a new EventFilter object. Because as it turns out, EventFilter could be refactored to simply look up what to call for a particular pointer, having one instance for each panel:

class EventFilter : public QObject

{

    Q_OBJECT

public:

    using eventfilter_sig = std::function<bool(QObject*,QEvent*)>;

    boost::container::flat_map<QObject*,eventfilter_sig> object2setter;

    explicit EventFilter(QObject* parent = 0);

    ~EventFilter() = default;

    void registerEventSource(QObject* obj, const eventfilter_sig[&] setter);

protected:

    bool eventFilter(QObject *obj, QEvent *event)override;

};

The change is simple, a map stores now which function objects should be called for with QObject pointer. Each widget now needs to be registered through registerEventSource instead of being a constructor parameter. In eventFilter is then a simple look up into the map, which makes it slightly more expensive at run time, but avoids a lot of allocations during construction. Which was the problem, that construction of a panel was taking way to long. This is now solved.

Improvements?

Writing the blog post has given me a few ideas what could be done to improve further. One option is to use the panel class as the event filter, its already derived from QObject and could simply override eventFilter on its own. Disadvantage: instead of overriding it once in this class, I have to override it in n classes. But it would allow for removing QObject out of this class and thus allow for making it a template, removing the dependency to Qt fully.

I also could improve the interface, by using overloading for the registration, I could write a method for each used QWidget (LineEdit, ComboBox etc.), then getText<QLineEdit> would be moved into the EventFilter class. This would improve usability, yet most code already using this interface would have to be refactored (again).

Join the Meeting C++ patreon community!
This and other posts on Meeting C++ are enabled by my supporters on patreon!