Crazy memory usage with TableView :(
-
Interesting findings:
- ListView includes no QQmlJavaScriptExpression allocations (expected; my test doesn't really contain any javascript. TableView is rather full of it.)
- QQmlBinding instances are way down: 89,800 down to 3,600 (506kb, much more manageable)
- Highest memory use comes from QQuickTextNode, which is unsurprising given how many items of text are visible on my screen (and QQuickTextNode is a bit of a pig, that is a known issue)
- There's a few allocations from WTF::OSAllocator::reserveAndCommit that are at the top of the list this time (6.25mb total). I think this is used for the JS garbage collection heap, but I forget. Seems high.
Any other users don't really stand out, as things start to get "lost" in the crowd a bit...
-
So it sounds like what you are saying is that TableView has a lot of internal bindings and it could make sense to determine what those are and whether they are useful.
Any suggestions on how to go forward with that?
-
To me, it looks like the TableView implementation is suboptimal. I don't really know how to suggest fixing it, because I don't really do much work on or with controls - most of my work is focused on QtQuick itself. You could try opening a bug on it with the findings from here, but I'm not sure how much priority it will get (again, because I'm not involved with it, I don't know what the people working on it are busy with and what their workloads look like).
It could well also be that it isn't a simple fix: generally speaking there's a reason that the base components like ListView in Quick are implemented in C++ - developer efficiency on the base component set goes down (because writing good components is harder), but performance tends to be better because one can take advantage of a less declarative implementation in favor of better performance. This is pure speculation, however: I haven't really looked at the TableView implementation beyond a trivial glance.
Seperately from that, the text node stuff should be improved on. That's already a known issue, but I don't think there's a bug for that yet. I'll ask the person whom I think would know best.
-
Regarding the text node, the most comprehensive bug is https://bugreports.qt.io/browse/QTBUG-42853. The overhead for each element is easily in the 1k range on the render thread, that comes in addition to whatever the QQuickText object has on the GUI thread (which is a QQuickItem which is 300+bytes and sits on a QTextDocument with its own datastructures).
-
I have started pushing a few patches to make Text a bit smaller. Those may make it into 5.5, if not, 5.6 -- provided I'm not doing anything too terribly silly.
-
Robin,
It's funny that you used the "orthogonal" of TableView in your example. TableView is ListView of Repeaters, not the other way around. That also means that we may look even worse, but it's worth checking.
There are a few things that can be optimized in TableView, but the most flagrant is related to styling. Our abuse of Loaders is to blame for a lot of the performance issues we've been having in controls as a whole, and having one Loader per table cell doesn't help. I can't say where all those QQmlBinding instances come from, though.
-
Good catch Gabriel. I've been trying to submit an issue in the JIRA but there is a PEBKAC where I can't log in to my existing account. In the meantime, I'm going to see if disabling all table styling could help.
-
Certainly some pretty good ideas coming out of this thread. Setting custom delegates to simple placeholders should remove most of the overhead of style itself. I don't think using a Loader is really the main issue though.
The excessive binding use is most likely caused by the construction of lots of simple styleData objects passed to the delegate containing style information. At the moment these are QtObjects with a lot of bound properties. Sounds like they might benefit significantly from being replaced with simple C++ objects. This would at least reduce the amount of created javascript bindings to a fraction of what it is at now.
-
How could we test that styleData theory? Is there a way to hijack it or is it just a matter of hacking the source?
-
I've been working on doing things a bit more lazily on TableView. "Here's a WIP patch":https://codereview.qt-project.org/105966 that instantiates columns only as they get close the the actual visible portion of the table view (applicable only on the very latest dev branch). Unfortunately, we can't recycle per-column delegates because each column can have their own distinct delegate, at least not in a straightforward way. But I'm sure something can be done. So, the tradeoff is less memory consumption, and faster creation and vertical scrolling, against slower horizontal scrolling. And, for obvious reasons, smaller windows perform better than larger windows.
@Jens, it has been proved (and confirmed by Simon) that the way we inject the styleData object in the loaded component is one of the causes things are so slow. The main reason being the "styleData" name resolution going through the slowest path possible. Making it a plain object is not going to change the creation time by much. Also, since we're showing model data (therefore, potentially dynamic), we need to bind to something, so moving away from QObject seems a step backwards to me.
-
sohail. You would have to hack the source but that is actually pretty simple as it's just a matter of copying the contents of TableView.qml to a local project and directly fork it. You might get some warnings from the style construction, but that is all.
For instance, you can go through all of the "styleData" properties and comment out all bindings except the "value" one just to experiment on it's impact. I did a quick test and I'm afraid it wont make all that much difference regardless. They really don't seem like the bottleneck.
@gabriel Thats indeed interesting. Did Simon suggest any other ways of injecting the data or is the concept itself not fixable? Given how much we rely on these, it might make sense to modify the engine to be more accommodating for this scenario.
Regarding columns, I was actually just playing around with the same concept by simply setting columns !visible based on scrolling but seems like you are way ahead on that already. That should at least prevent the loaders from running. If we could also find a way to prevent setting up the hidden styleData objects too, there would be a pretty significant improvement on memory use. Did you test the patch on the example above already?