Moving QAbstractItemModel Subclass Instance from Main GUI Thread to Worker Thread When Used in a QTreeView

Crag_Hack

What kinda issues in the long run? No race conditions right?

Crag_Hack

Forgot to mention I am signal/slotting pointers to the models not objects themselves... race condition possible? Do I need to mutex the model? Does this approach work well? Does this make more sense were you guys under the interpretation that I was signal/slotting objects? Thanks

SGaist

Can you explain your model design ?

You just explained that you are moving pointer to objects around one or more models (it was only one until now but you used plural in your last message) that you move from one thread to another.

Crag_Hack

@SGaist Thanks I am using 5 different tree models and 5 different QTreeViews, one model per view, thus the original statement about only one instance. I dusted off my Computer Systems book from college and reread the concurrency chapter; I'm pretty sure race conditions aren't and issue but I'm by no means an expert. I posted some code below. The Worker object lives in a worker thread for long-lived operations. The Program object is the main program object living in the main GUI thread to handle all GUI display. Any input would be much appreciated.

void Worker::createModels()
{
    TreeModel *treeFilesModel = new TreeModel();
    //other 4 models created after this

    //code here to add items to the models

    treeFilesModel->moveToThread(QApplication::instance()->thread());
    //other 4 models moved to main GUI thread
    emit passModels(treeFilesModel,...other 4 models); //signal to move models to main GUI thread for display, target slot is passModelsAndSetupTree
}

void Program::passModelsAndSetupTree(TreeModel *treeFilesModel,...other 4 models)
{
    treeFilesView = new TreeView();
    treeFilesView->setModel(treeFilesModel);
    //other 4 views set up
    //after everybody is set up user can interact with views to choose actions on model items displayed in views
}

void Program::traverseTrees()
{
    traverseTree(treeFilesModel->getFirstItem());
    //other 4 models traversed

    //should I delete the treeviews here?
    //should I move models to worker thread here using moveToThread()?

    emit process(treeFilesModel,...other 4 models); //signal to move models to worker thread for processing
}

SGaist

I insist, stop moving your models around. There should be no need for that.

The name of your model variables makes it look like you are implementing your own QFileSystemModel, is that the case ?

Crag_Hack

@SGaist said in Moving QAbstractItemModel Subclass Instance from Main GUI Thread to Worker Thread When Used in a QTreeView:

I insist, stop moving your models around. There should be no need for that.

So get rid of moveToThread() calls but leave signal/slots (obviously) right? If so we should update the other thread since I was told to use those calls there.

The name of your model variables makes it look like you are implementing your own QFileSystemModel, is that the case ?

I made the models from scratch borrowing code from this book. I never looked at how QFileSystemModel was constructed to influence my own models.

SGaist

Indeed, keep the signals and slots and stop moving your model around. Note that you can make use of threads within your model. Just that it does not make sense to move your model itself back and forth between threads.

That's a very good book.

Crag_Hack

Thanks, sounds like I'm doing things in a acceptable fashion other than the moveToThread() calls right?

Are moveToThread() calls only relevant for objects with event processing? Do they serve any other purpose?

An alternative way of doing things would be to use threads within the model like QFileSystemModel does correct? I'll take a peak at the source.

Before passing the models back to the worker thread for processing should I delete the associated QTreeViews or does it matter?

SGaist

I am currently only discussing the model move between different threads. For the rest I do not know what you do with it.

The goal is to change thread affinity and thus call to any of the object function would then happen in the new thread.

It's not really an alternate rather the correct way.

There's no need to delete any view. If anything, set a null model.

Crag_Hack

I checked out the source code and can't find any thread usage in the model; I must be missing something the model looks almost the same as mine with the same virtual function overrides I use. Can you point me to the relevant code? Thanks. I have a design pattern book that I could dust off (ha another old dusty book) and reread the MVC chapter.... Also what threw me off in the past looking at Qt source was the non-descript class member variable names like d and q... why use names like that???

SGaist

There's no explicit QThread used, but if you look for just thread, you'll find more information.

As for the d and q pointers, long story short, it because Qt uses the PIMPL idiom.

For all the details, see this Wiki page about that matter and why Qt uses it. The short version is to allow the forward and backward compatibility within a major release that you can enjoy using Qt i.e. you can just switch the libraries without having to recompile your application (provided that the version you switch to has all the symbols you are using)

Crag_Hack

There's no explicit QThread used, but if you look for just thread, you'll find more information.

I took a peek... if there's no code for offloading to a worker thread how does one adopt the same approach in their own models? Do you have to inherit from QFileSystemModel?

Unless my own model doesn't work properly I'd like to continue using it the way I am... it works great as far as I can tell and there's no critical flaws in the approach right? Are there any caveats to constructing and processing the model in the worker thread the way I am now? I only care that it works and is 'good enough'.. not seeking absolute perfection for now.

When you say "Suffice, you'll have to check but moving back and forth your model at any moment will likely create issues in the long run." do you just mean I have to see if my tree view works properly empirically and determine sufficiency from that?

Hopefully this wraps up this thread, thanks for the help.

SGaist

@Crag_Hack said in Moving QAbstractItemModel Subclass Instance from Main GUI Thread to Worker Thread When Used in a QTreeView:

I took a peek... if there's no code for offloading to a worker thread how does one adopt the same approach in their own models? Do you have to inherit from QFileSystemModel?

No you don't. As for the integration, it depends on what you do in your thread. The QThread documentation shows the two possible techniques (worker object and QThread subclass).

@Crag_Hack said in Moving QAbstractItemModel Subclass Instance from Main GUI Thread to Worker Thread When Used in a QTreeView:

When you say "Suffice, you'll have to check but moving back and forth your model at any moment will likely create issues in the long run." do you just mean I have to see if my tree view works properly empirically and determine sufficiency from that?

I would recommend running dedicated tests rather than just checking if it's working on a workstation.

Crag_Hack

The QThread documentation shows the two possible techniques (worker object and QThread subclass).

That's what I've been doing all along! Lol. I use a worker object but I said I used a worker thread sorry if I didn't make that clear.

SGaist

From your description your worker object seems to be the model itself, isn't it ?

Crag_Hack

Nope I have a worker object initialized in the constructor of the program object as below. Then I create the models in the worker thread as in the previously posted code and signal/slot them back and forth between the Worker thread and main GUI thread as needed. Given this should I still avoid those moveToThread() calls? Also is this a proper way of doing things?

workerThread = new QThread(this);
workerThread->start();
worker = new Worker;
worker->moveToThread(workerThread);

SGaist

That's what makes it "unclean". The worker object should do the heavy processing and then populate the model through signals and slots, not create it.

Out of curiosity, what kind of heavy lifting must be done ?

Crag_Hack

That's what makes it "unclean". The worker object should do the heavy processing and then populate the model through signals and slots, not create it.

How does one populate through signals/slots? I am dealing with potentially multiple tens of thousands of items in the trees so it doesn't make sense to signal/slot each item does it?

Also what's so bad about this approach that makes things 'unclean'. Doesn't it work fine? Are there any critical flaws?

Out of curiosity, what kind of heavy lifting must be done ?

This particular tree displays file/directory structures and allows the user to manipulate them.

SGaist

You don't have to signal each and every change, that's when you make your code smart and do batch updates for example.

Also, performance wise, there's the question of the interest of having tens of thousands of items loaded at the same time in memory. Are they worth being all loaded or would a subset make more sense ? For example, would it make sense to load all the files from the root of your disk ? Also, do your users really need to have all the data pre-loaded or would "just in time" details be enough ?

Then, why is there a need to remove the model from the view when there's an action to be done with its data ? Here is the potential code smell. There should be no need for that in the first place.

Crag_Hack

You don't have to signal each and every change, that's when you make your code smart and do batch updates for example.

Can you show me some brief code/pseudocode or elaborate? I still don't know how to populate in this regard.

Also, performance wise, there's the question of the interest of having tens of thousands of items loaded at the same time in memory. Are they worth being all loaded or would a subset make more sense ? For example, would it make sense to load all the files from the root of your disk ? Also, do your users really need to have all the data pre-loaded or would "just in time" details be enough ?

I just did a benchmark and loading a tree of 50,000 items several levels deep only increased memory consumption from 12 MB to 51 MB. Also the tree loaded in 10 seconds or so. So maybe down the road I can change things but for now I'm just looking forward to completion sooner rather than later.

Then, why is there a need to remove the model from the view when there's an action to be done with its data ? Here is the potential code smell. There should be no need for that in the first place.

The items have state in member variables for processing and also once the user has completed interaction with the view the model gets sent to the worker thread for processing. The view is no longer needed at this point; hence my original question about if I need to do anything to the views before I send the model off for processing.

Here is the potential code smell.

Is this the 'unclean' part? Is the rest a proper approach? This is the most important question for me for now.

Also I was hoping this thread would serve for other Googlers out there looking for a quick and easy simple solution to this scenario; this scenario must be pretty common.

Thanks for your time :)