[HaikuDepot] switching between Featured and All packages tabs is slow

[diver]

hrev52462.

Just updated Haiku to see new tabbed HaikuDepot and noticed its being slow at switching between the tabs.

[diver]

This was introduced in hrev52449 (e00a489b80a7219bf80658d157548eb0aa4a97d0) Reverting this commit fixes extensive slowdown in switching between Featured view and all packages.

[diver]

BTW, now that I switched to the old look I find it much better than tabbed view.

[waddlesplash]

That change should not have been any more or less performant than previously; it just changed the layout from a BCardView to a BTabView. The problem is not in code that I wrote at all.

BTW, now that I switched to the old look I find it much better than tabbed view.

The reason we switched to the tab view was that people were ignoring the "Show only featured packages" checkbox because they didn't understand it / didn't even see it. Having the tabs makes the dichotomy much more obvious. (I prefer it in general, and I think humdinger does too.)

[humdinger]

Replying to waddlesplash:

(I prefer [tabs] in general, and I think humdinger does too.)

I think the tabs do look better as they are not as "in-your-face" as the checkbox that used to be in the middle. OTOH, people were able to miss the "in-your-face" checkbox, so I expect the tabs to fair even less well in that regard...
The tabs have the additional advantage to be extendable with more tabs, if we see a need. (Maybe a dynamically named "Pending/Downloading/Installed" for the changed packages in the curent session.)

[diver]

I'd like to nominate this bug for beta2 milestone.

HaikuDepot, which once was blazing fast, became really sluggish nowadays to the point I stopped using it completely.

However, we don't want to make bad impression on regular users and newcomers of our package management system. #11674 is likely another candidate.

[apl-haiku]

I have investigated this. It seems that there were a couple of areas that could be improved and I was able to get some code clean-up done, but the majority of the computational time taken is in the re-layout of the views of the "prominent" and "all" package lists.

Still to investigate; this has the potential to be a contributing factor to the slow initial loading performance.

Most likely the best (first) path forward would be to implement the FeaturedPackagesView as a single monolithic view rather than as a composite of dynamically laid-out views in order to avoid or simplify the computational cost of the layout-process. This is probably a medium-sized job and low time-risk.

The PackageListView could also be re-written in a more performant way as well, but would be more difficult owing to the inheritance from the more complex BColumnListView. Some compromises may be able to be made in order to reduce the complexity of this task; does it need to be a table?

I have some related improvements in a local branch which I can only put into a PR after PR 2467 is merged. These related improvements make a minor contribution to the performance, but do clean up the code quite a bit around the tab-switching. These changes are "B2 safe".

I suggest that any further and deeper work on this proceeds after B2 is released because it is likely to be disruptive, needs to be well considered and will take time to settle.

[waddlesplash]

I have investigated this. It seems that there were a couple of areas that could be improved and I was able to get some code clean-up done, but the majority of the computational time taken is in the re-layout of the views of the "prominent" and "all" package lists.

I wonder why switching to a tab view made this so much slower vs. the card view, then. I guess the tab view invalidates the child's layout on each tab switch; can we avoid that? That will help restore performance.

[apl-haiku]

It seems that (by manual sampling) most of the time seems to be consumed here;

...
BFont::GetStringWidths(..)
BFont::StringWidth(..)
BTitledColumn::GetPreferredWidth(BField* .., BView* ..)
PackageColumn::GetPreferredWidth(BField* .., BView* ..)
BPrivate::OutlineView::GetColumnPreferredWidth(BColumn* ..)
BColumnListView::PreferredSize()
BViewLayoutItem::PreferredSize()
...

[X512]

Replying to apl-haiku:

BTitledColumn::GetPreferredWidth(BField* .., BView* ..)
PackageColumn::GetPreferredWidth(BField* .., BView* ..)

It is likely full list traverse is done at each insertion making O(n^{2) time complexity.}

[CodeforEvolution]

If StringWidth() is being used a lot, maybe WidthBuffer could be used: ​https://github.com/haiku/haiku/blob/master/src/kits/interface/textview_support/WidthBuffer.cpp

WidthBuffer caches StringWidth() value calculations and avoids extra app_server trips.

[waddlesplash]

The better solution is to not recompute the entire layout every time an item is added, and also not to recompute the layout upon switching tabs.

[apl-haiku]

See the comment trail ​here. I am going to try another approach as suggested there. Short term, I am going to switch to re-factor FeaturedPackagesView first. I have started that process.

[waddlesplash]

Why not just fix ColumnListView to not recompute the preferred column size at every insertion?

[apl-haiku]

Yes I will get back to that; both views have performance problems. I was looking at #15889 which lead me back here on the FeaturedPackagesView class.

[apl-haiku]

I am re-coding FeaturedPackagesView using manual drawing and it is now responding quickly; work progressing on that. This is quite a big change and so if somebody else feels like taking on the work of...

...fix ColumnListView to not recompute the preferred column size at every insertion?

As per comments in ​here -- then that could happen concurrently with what I am doing.

[waddlesplash]

Is that really the best solution? It would seem like making BStringView more performant (possibly by doing caching like BTextView does) would be the better solution?

[apl-haiku]

FeaturedPackagesView -- I think it is the best solution. The auto-layout is great for layout of a window of controls and views and is performant enough, but auto laying out what is essentially a fixed layout thousands of times in a list isn't really making much sense.

[waddlesplash]

So, as it turns out, I was indeed correct that using draw-calls is not at all the best solution, and the problem was indeed in BTabView (or actually BCardView, which is what BTabView uses in layout mode): ​https://review.haiku-os.org/c/haiku/+/2677/2

Switching tabs is basically instantaneous after this, and resizing the window is still very fast. So the draw-calls, and also the fixes to ColumnListView, are not needed, indeed.

[waddlesplash]

Loading packages is still not very fast due to how often the layout is invalidated, but that is a separate ticket. I think there is a way to "batch" invalidations, which is what should be done there.

[apl-haiku]

Yes I agree, but I think all three things would be good to get done here and would make a big difference.

• Change from dynamic to hand-coded layout in FeaturedPackagesView
• Stop tabs-changes from doing relayout
• Stop BColumnListView from readjusting the column widths

I'm working on FeaturedPackagesView, but am moving house (it's complex) and so time is scant, but if you can help with the others things then that would be terrific.

[waddlesplash]

Change from dynamic to hand-coded layout in FeaturedPackagesView

I still maintain this is a bad idea, in general, and is just a hack to get around underlying performance problems that we should solve.

Stop tabs-changes from doing relayout

Yes, that is what my change above does, and it fixes this ticket completely.

Stop BColumnListView from readjusting the column widths

This was just happening due to BCardLayout re-layouts recomputing PreferredSize(), which is what recomputes preferred column sizes. The actual column widths were not changing. We may still want to change BColumnListView here, but at least this is no longer a pressing problem.

[apl-haiku]

Obviously this ticket will be fixed by your change.

I still maintain this is a bad idea, in general, and is just a hack to get around underlying performance problems that we should solve.

In the wider sense, considering other tickets as well -- each View representing a single package undertakes layout independently. If there are 1000 packages in that scrolled View then it is performing that layout 1000 times even though the layout is exactly the same in each case. This automated layout approach is very inefficient and the layout end result is largely static. I don't think that using the automated layout system makes sense this situation at all but I would be interested to hear your approach to how the performance problems with FeaturedPackagesView should be resolved while still using the automated layout system.

Yes, that is what my change above does, and it fixes this ticket completely.

It will fix this specific ticket, but there are other tickets where a solution to the performance problems of BColumnListView will continue to be an issue and I suspect the suggestion from X512 will be good to explore further.

[waddlesplash]

Fixed in hrev54185.