Qt Slots called too frequently

Question

I have a Worker Thread that copes with heavy and long computations (up to tenth of seconds). These computations produce several thousands of QLines, representing the edges of a dynamically-growing tree. These edges can be modified anytime, since they connect the nodes of the trees by checking the cost, represented by the distance. I would like a smooth update of the QGraphicsScene containing the edges. I tried with signal and slots:

• Worker thread emits a signal, so when the buffer is full this signal gets caught by the main thread, that will cope with the update/drawing of the line
• This signal gets still caught by the main thread, but it seems it gets emitted very often, so QGraphicsView gets choked with QLine to be added
• Changing the size of the buffer doesn't matter
• Is there an alternative approach to this?

The main slot is:

void MainWindow::update_scene(bufferType buffer)
{
  for (int i = 0; i < buffer.size(); ++i)
  {
      if (buffer[i].first < (g_edges.size() - 1))
      {
          delete g_edges[buffer[i].first];
          g_edges[buffer[i].first] = scene->addLine(buffer[i].second);
      }
      else
          g_edges.push_back(scene->addLine(buffer[i].second));
   }
}

Note that bufferType is of type QList<std::pair<int,QLine>>. Here is the heavy computing part

while (T.size() < max_nodes_number && !_stop)
{
    const cnode random_node = rand_conf ();
    const cnode nearest_node = T.nearest_node (random_node);
    cnode new_node = new_conf (nearest_node, random_node);

    if (obstacle_free(nearest_node, new_node))
    {
        QList<cnode*> X_near = T.neighbours (new_node, max_neighbour_radius);
        cnode lowest_cost_node = nearest_node;
        qreal c_min = nearest_node.cost() + T.distance (nearest_node, new_node);

        for (int j = 0; j < X_near.size(); ++j)
        {
            if (obstacle_free(*X_near[j], new_node) && ((X_near[j]->cost() + T.distance (*X_near[j], new_node)) < c_min))
            {
                c_min = X_near[j]->cost() + T.distance (*X_near[j], new_node);
                lowest_cost_node = *X_near[j];
            }
        }

        T.add_node (new_node, lowest_cost_node.id());
        queue (new_node.id(), QLine (new_node.x(), new_node.y(), lowest_cost_node.x(), lowest_cost_node.y()));

        for (int j = 0; j < X_near.size(); ++j)
        {
            if (obstacle_free(*X_near[j], new_node) && (new_node.cost() + T.distance (new_node, *X_near[j])) < X_near[j]->cost())
            {
                queue (X_near[j]->id(), QLine (new_node.x(), new_node.y(), X_near[j]->x(), X_near[j]->y()));

                T.update_parent (*X_near[j], new_node.id());
                T.rewire_tree (X_near[j]->id());
            }
        }
    }
}
emit finished();

Please note that T is a class representing a Tree. It is constituted by some methods allowing to add a node, searching for the nearest one, etc. It has a QList<cnode> as private member, storing the tree's nodes. cnode is a structure constituted of two coordinates, an id, a parent, a cost, a list of its children.

Answer 1

The solution is as usual - avoid frequent queued connections, as those are quite slow. Queued connections are a coarse grain construct and such be used as such.

Batch the work. In your scenario, you could aggregate the computed lines in a container, and only when it reaches a certain threshold, pass that container to the main thread to draw/update the lines. The threshold can be count, time or a combination of both, you don't want not updating if there are only a few results to update. You will need to expand on your design to split the while loop to run in the thread event loop instead of blocking so you can aggregate and pass updates periodically - something similar to this. This is always a good idea for workers that take time - you can monitor progress, cancel, pause and all sorts of handy stuff.

Those 2 lines look fishy:

    edges.removeAt(i);
    edges.insert (i, scene->addLine (l));

Then you remove and then insert - that's an invitation for potential costly reallocation, even without reallocation there is unnecessary copying involved. Instead of removing and inserting you can simply replace the element at that index.

In your case you might omit splitting the actual while loop. Just don't emit in the loop, instead do something like this (pseudocode):

while(...) {
    ...
    queue(new line)
    ...
    queue(update line)
    ...
    queue(final flush)
}

void queue(stuff) {
    stuffBuffer.append(stuff)
    if (stuffBuffer.size() > 50 || final_flush) {
        emit do_stuff(stuffBuffer) // pass by copy
        stuffBuffer.clear() // COW will only clear stuffBuffer but not the copy passed above
    } 
}

Or if it will make you feel better:

    copy = stuffBuffer
    stuffBuffer.clear()
    emit do_stuff(copy)

This way the two containers are detached from the shared data before the copy is emitted.

EDIT: After a long discussion I ended up proposing a number of changes to the design to improve performance (the queued connections were only one aspect of the performance problem):

• alleviate the graphics scene - find a compromise between "one item per line" and "one item for all lines" solution, where each item handles the drawing of the lines of its direct children, balancing between the CPU time for adding items to the scene and redrawing items on data changes.

• disable automatic scene updates, and instead control the scene update explicitly, this way the scene is not updated for each and every tiny change.

• aggregate view commands in batches and submit the work buffer at a fixed interval to avoid queued signals overhead.