Optimise the package filtering process

[oco]

After profiling HaikuDepot, i have found a potential optimization in the filtering process.

Here is the corresponding patch as, unfortunately, i am currently unable to login in gerrit anymore.

Optimize the package filtering process

• Avoid convert package name to lower case each time the filter is changed

[apl-haiku]

Hello @oco! Thanks for the patch. I am unsure if this will work because the _TextContains(..) is used in other methods in the same class and would have the impact of making the "publisher" and "title", "summary" and "description" case-sensitive.

I am actually currently performing a large refactor of the data and processing internals of HaikuDepot. I haven't gotten to the filtering yet, but do intend to look into the performance of the filtering.

[waddlesplash]

Won't the package name already be in lowercase anyway? So we don't need to store it separately.

_TextContains seems like the wrong place to put a ToLower transform for just this reason, and the transform should be put elsewhere.

BString also has an IFindFirst method which is much more efficient than this algorithm anyway.

[apl-haiku]

Thanks @waddlesplash; when I get to the filters, I'll switch to use IFindFirst.

[waddlesplash]

Do we need to wait for your refactor? We can just apply this or a similar patch now, and whenever you get around to submitting the full refactor we can just replace it with that.

[apl-haiku]

Yes; that's totally fine too - do you want to do that or do you want me to open a pr later today?

[waddlesplash]

Committed the change in hrev58646~2. Looking at the profiler, I also found some other easy performance wins in view invalidation and fixed those at the same time.

However, there's still a big one remaining: we invalidate every single row unconditionally, it appears. I came up with a change that fixes this case too:

diff --git a/src/kits/interface/ColumnListView.cpp b/src/kits/interface/ColumnListView.cpp
index 9175b83b36..8f823a73d2 100644
--- a/src/kits/interface/ColumnListView.cpp
+++ b/src/kits/interface/ColumnListView.cpp
@@ -4309,26 +4309,30 @@ OutlineView::ExpandOrCollapse(BRow* parentRow, bool expand)
 
        parentRow->fIsExpanded = expand;
 
+       if (parentRow->fChildList == NULL)
+               return;
+

But this causes the list to redraw incorrectly, because it seems the incremental-invalidation code in the row insertion logic does not take the alternating backgrounds into account. So that will need to be fixed in order for this to be used.

[waddlesplash]

Committed an alternative optimization that helps that case in hrev58647.

It looks like we are now spending over half the remaining time in PackageRow constructor. That seems like those objects should be cached and not reconstructed every time we rebuild the full list. But that sounds like it would be part of your data model refactors so I will leave that to you.

[apl-haiku]

Hi @waddlesplash; Thanks for those commits. I'll rebase those onto my branch soon.

But that sounds like it would be part of your data model refactors so I will leave that to you.

Yes this whole invalidation, one-by-one row changes, lock-contention, immutable data models and so on is very much the area I'm making quite big changes in -- so yes please it would be best not to work in this area or it will become difficult.

I am putting together a "big bang" change unfortunately but I can't see another way to deal with these changes.