Code review comment for lp://qastaging/~uriboni/webbrowser-app/newtabview-listviews
Revision history for this message
| Ugo Riboni (uriboni) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
demo site
(Get the code!)
> I have profiled the solution I was suggesting in my first comment. With a
> database of 100000 bookmarks, instantiating the entire view takes 600ms, and
> it is instantly responsive.
>
> It works well when all sections are expanded, but it slows down considerably
> when all sections are collapsed (presumably because then there are 1000
> invisible delegates instantiated for each section, so if there are 10
> collapsed sections on screen that’s 10000 delegates instantiated at any given
> time).
All the solutions you proposed are not removing the main problem, which is that the performance is still linearly proportional to the number of bookmarks. By making the delegates leaner you are making the degrade in performance slower as the number of bookmarks grow, but the point is that we can't control the number of bookmarks, so the problem is never really solved.
Moreover you are testing with 1K bookmarks per folder, while you will see the app being completely unusable if you put 10K bookmarks in a folder and try to open it (or put them in the root folder, which is open when you open the view).
> On top of that the layout of the view is incorrect after expanding/
> a section, because the implementation of ListView computes an estimate of its
> contentHeight when using sections, instead of using the real content height.
Hiding delegates by making their height zero is essentially an hack, in my opinion: it works in some cases but it is not robust and/or scalable, and denies what is arguably the main benefit of using ListView in the first place: delegates are created and destroyed on-demand.
> Nested list views may not be such a bad idea after all…
How would you go about implementing nested ListViews ? A ListView to work needs to know the height of its delegates, and if each delegate is a ListView then they need to have height equal to contentHeight, which instantiates all the sub-list delegates. You can limit the height of each sub-ListView to a certain fixed amount, but then you have scrolling problems because either you scroll the main ListView or the sub-ListView.