Next iteration of thread and class architecture
These are some of my thoughts about the current state of the thread and class architecture in Kaidan. I'm sharing them with you
1 What has changed recently / our new tools
- 1.1 QXmpp offers async APIs with QFutures
-
1.2 Signals have been replaced by QFutures in many places in QXmpp
- Kaidan could also start to use QFutures to replace some signals
-
1.3 We can easily execute lambdas on another thread with
runOnThread()
or the cross-threadawait()
- 1.4 We started to use the watcher idiom
2 Issues with the current architecture
-
2.1 We can't handle signals from objects from the XMPP thread (MessageHandler, RosterManager, etc.) from QML.
- In QML
Connections
does not work on objects from different threads - => we need additional wrapper classes on the main thread
- => there are many classes for one purpose: e.g. QXmppRosterManager, RosterManager, RosterController, RosterModels (if we want to have multiple independent views)
- In QML
-
2.2 We need to create
*Requested()
signals to access the XMPP thread objects (avoidable in C++, required for QML) -
2.3 We need many additonal thread-safe "caches" classes (e.g. TransferCache or ServerFeaturesCache)
- some could be part of an existing controller class
- it would be nice to avoid having to deal with thread-safety at all (e.g. no mutexes)
-
2.4 Too many thread interactions
- in some places the logic is complicated just because of the thread interaction
- sometimes values need to be cached somewhere on the main thread and need to be updated via signals just because the Manager class runs on another thread
- sometimes the code is not "linear" because of jumps with
*Requested()
signals back and forth between threads, as a result the code is hard to read
-
2.5 MessageModel and RosterModel act as controllers not just as data models
- it's not possible to create two message/roster models to view two different chats
- not well structured: especially the message model does many things unrelated to just viewing the messages, e.g. it also does chat markers, chat states and OMEMO management => the code is not encapsulated => large maintenance effort
-
2.6 In some places there still are manual
Connections
in QML where a watcher idiom would be much better
Issues I don't know how to solve them yet:
-
2.7 the database/model situation is very complicated
- insert/delete/update is "duplicated" in the models and in the db
- we had issues because code was executed in the model before it was in the database (or something like that), see !822 (merged)
- it's much faster to use cached data, but as a result of that there is no single place of truth
3 Ideas for improvements
-
3.1 Replace Kaidan managers (RosterManager, MessageHandler, …) on the XMPP thread with controllers on the main thread
- the QXmppClient and its extensions stay on the XMPP thread
- the controllers use
runOnThread
, cross-threadawait
and signal connections to communicate with the QXmpp client extensions - QML never accesses objects from another thread (so no additional wrapping classes are needed) (solves 2.1)
- all of the controllers/managers/models/db classes live on the same thread (solves 2.4)
- only the QXmpp client extensions live on the XMPP thread
- note: the DB classes internally run lambda functions on the database thread, but live on the main thread
- thread safety is no issue anymore (solves 2.3)
- no mutexes are needed when communicating with the QXmpp client extensions (because of QFutures and signals)
- no thread-safe "caches" are required anymore, the existing (or additional) controllers can be used directly
- finally no
*Requested()
signals anymore (solves 2.2)- all models and controllers run on the same thread
- QXmpp client extension communication is done by using await, runOnThread and "normal" signal connections
-
3.2 Splitting up RosterModel and MessageModel (solves 2.5)
- RosterController and MessageController should handle everything that is not related to presenting the data via QAbstractListModel
- the models should be creatable from QML
- the models should attach to the controllers to receive updates
- 3.3 About 2.6: Just use new watchers and replace signals, e.g. for interaction with the TransferCache
4 Questions
-
4.1 How exactly do the controllers handle multiple accounts (and so multiple clients and managers)?
- definitely solvable
- 4.2 How exactly do we communicate the updates from the Roster/MessageController between multiple models?
- 4.3 Are there issues/disadvantages with 3 (and especially 3.1)?
-
4.4 How can we do a smooth transition without touching too much code?
- Yes, we can do 3.1 in small(er) steps:
- We can start with adjusting parts of the code, e.g. replacing the RegistrationManager, the UploadManager or the OmemoManager with controllers
- MessageHandler and RosterManager are bigger tasks, but they still can be done independently
- Yes, we can do 3.1 in small(er) steps:
- …
Edited by Linus Jahn