Proposed performance enhancement for QTreeView::dataChanged
-
I'm trying to improve the performance of my application and have recently updated to Qt6 from Qt5.
In making that change, I believe the changes to QAbstractItemView::dataChanged have made my situation worse than before.
This is because it now performs a lot of calculations for large changes. This was introduced at this time: https://codereview.qt-project.org/c/qt/qtbase/+/285280/17 Which I think is fine, but I wish to suggest some further enhancements that would assist in my case...Since that change, I also see a new addition that attempts to improve and mitigate some of the introduced overhead by using the new "updateThreshold". Presently, this updateThreshold will determine that this particular dataChanged event contains too many modifications, and decides to simply invalidate the whole widget- excellent.
Though in my case, I have 10000's of rows, and intermediate data models that do need to know when data has changed. Unfortunately, my data is changing in individual cells (that the intermediate models need to know about). But these cell changes are not occurring in bulk sequences, instead, they're scattered over the dataset (they're the result of async queries to a db returning and updating records where data is available).
Because I'm updating one row item per call to dataChanged at a time, but making 1000's of those calls, the optimization of checking the updateThreshold doesn't kick in.
I'm wondering if we could further enhance the QTreeView::dataChanged handler, to keep a record of the number of cells updated since the last paintEvent... It would do 3 things:
- Record the number of cells updated since the last paintEvent.
- If this event would push the number of cells updated over the updateThreshold - do a full update.
- If an event comes through and the number of cells updated is already above updateThreshold - then return - because a full update has already been queued (by the previous event).
I intend to make a derived version of QTreeView to make this change myself - but again, I'd appreciate any feedback on the idea.
Would this be an acceptable change for myself or someone else to put together to the qtbase repo?
I'd appreciate feedback from someone else familiar with this area of the code - perhaps @Christian-Ehrlicher ?Many thanks for your time and patience!
-
And what's with the other widgets or objects which might depend on dataChanged() signal?
set the threshold to 1 to get the old behavior. -
Thanks for taking a look.
Regarding your questions:
what about other objects that need the dataChanged event?
My proposal is only for a further enhancement of the View Widget code that is processing the dataChanged event. Specifically QTreeView. I expect all other objects connected to the event would still have their chance to do their work. As far as I understand (which is limited and perhaps you can point me to something that is outside my understanding) - As far as I understand, the QAbstractItemView and QTreeView widget's handling of dataChanged signals is simply determining the region of the widget to update and has no further responsibility. Is that a fair assessment?
Set update threshold(1) if you want the old behaviour.
I was hoping I might be able to squeeze better behaviour than before with this change.
I was hoping my approach would avoid unnecessary extra calls to update() when the widget has already been completely invalidated. From my reading, there is no fast path for the update call even if it has already been called in the current frame 1000's of times already. This suggested change could limit that overhead?
If possible, could you point out where I might have made some incorrect assumptions in my thinking?
Again, I appreciate your time and patience.
All the best, -
An update() does nothing - it just triggers a redraw. The redrawing is done when the os triggers a redraw so updates are accumulated.
-
Thanks for taking a look.
Regarding your questions:
what about other objects that need the dataChanged event?
My proposal is only for a further enhancement of the View Widget code that is processing the dataChanged event. Specifically QTreeView. I expect all other objects connected to the event would still have their chance to do their work. As far as I understand (which is limited and perhaps you can point me to something that is outside my understanding) - As far as I understand, the QAbstractItemView and QTreeView widget's handling of dataChanged signals is simply determining the region of the widget to update and has no further responsibility. Is that a fair assessment?
Set update threshold(1) if you want the old behaviour.
I was hoping I might be able to squeeze better behaviour than before with this change.
I was hoping my approach would avoid unnecessary extra calls to update() when the widget has already been completely invalidated. From my reading, there is no fast path for the update call even if it has already been called in the current frame 1000's of times already. This suggested change could limit that overhead?
If possible, could you point out where I might have made some incorrect assumptions in my thinking?
Again, I appreciate your time and patience.
All the best,@inbilla hi,
Are you using custom models ?
If so, are you also taking advantage of the last argument of the dataChanged signal ? This one limits the roles that the view should get data for in order to update itself.
