Groupie: GroupUtils.getItem from DiffUtil takes several seconds for large lists

Created on 1 Jun 2020  路  5Comments  路  Source: lisawray/groupie

Describe the bug
In my case, I had 4500 items, which shouldn't be considered as large. I have expandable groups with sections, which cannot be updated asynchronously, so, a section.update() with detectMoves=false freezes the UI for 3 seconds.
Screenshot of profiler:
image

To Reproduce
Run example code for Section.update with about 5000 entries

Expected behavior
Better performance

Library version
2.8.0

bug waiting for owners

Most helpful comment

The problem can actually be easily reproduced on the example sample project, by changing these lines:

for (i in 1..1200) {
    add(UpdatableItem(rainbow200[i % 12], i))
}

I identified two major performance issues :

  • Shuffling the 1200 items takes ~1 minute on my device. DiffCallback is slow because of the use of GroupUtils.getItem in each ot its methods. Fortunately, it looks like it can be fixed by flattening the items only once when the DiffCallback is built (rather than recursively search for items again and again), and using ArrayList.get afterwards on the flattened list (which is fast). It is not perfect (we still have to loop through all the items once before the DiffCallback can actually run) but way faster ( < 0.5s ).
    Commit here: https://github.com/herbeth1u/groupie/commit/8eaa5bc8ced409f387e3b89427bc00d547b7ae7f
    I didn't make a PR because I'm not extremely familiar with the code base, and this could have impacts I'm not aware of. Feel free to use and do whatever you want with it though.
  • Scrolling down through the 1200 items is ok, but when trying to scroll up, I can barely reach the top before a minute. The profiler tells me GroupAdapter.getItem(int position) is one of the main root causes for this; it indeed calls GroupUtils.getItem on the main thread (which is ultimately the same problem as above). I tried to flatten GroupAdapter.groups the same way, so we can simply use ArrayList.get instead, but GroupAdapter.groups is altered in a lot of places, and I wasn't able to make it work.
    Edit : This second issue is actually mentioned in #161 and has been around for quite some time.

It doesn't look like Groupie is very friendly with large lists, but the code could use some optimizations (more or less easy to implement).

All 5 comments

I'm having similar issues, with a similar number of items - ~5k. However i'm doing it with updateAsync so no UI jank, just seriusly slow to update the items.

I have two Sections with headers, one section with 5 items, one with 5k items.

With 1000 items, it takes 0.5s
image

With 2000 it takes 7.5s
image

With 2500 items, it takes 13.45s
image

Wwith 3000 items, it takes 45s
image

I have the following overrides in my item class btw:

  override fun isSameAs(other: Item<*>): Boolean {
        return other is SearchItem
    }

    override fun hasSameContentAs(other: Item<*>): Boolean {
        return other is SearchItem && other.text == text
    }

Without them, its much worse

I hacked around this by just clearing the adapter before updating... this avoid doing the pointless (in my use case) diff

adapter.clear()
adapter.updateAsync(...)

+1 for this issue; using a for loop in GroupUtils.getItem and GroupUtils.getItemCount doesn't help with performance. Couldn't it be possible to use a solution based on groups.get(position) instead, to avoid a nested loop?

The problem can actually be easily reproduced on the example sample project, by changing these lines:

for (i in 1..1200) {
    add(UpdatableItem(rainbow200[i % 12], i))
}

I identified two major performance issues :

  • Shuffling the 1200 items takes ~1 minute on my device. DiffCallback is slow because of the use of GroupUtils.getItem in each ot its methods. Fortunately, it looks like it can be fixed by flattening the items only once when the DiffCallback is built (rather than recursively search for items again and again), and using ArrayList.get afterwards on the flattened list (which is fast). It is not perfect (we still have to loop through all the items once before the DiffCallback can actually run) but way faster ( < 0.5s ).
    Commit here: https://github.com/herbeth1u/groupie/commit/8eaa5bc8ced409f387e3b89427bc00d547b7ae7f
    I didn't make a PR because I'm not extremely familiar with the code base, and this could have impacts I'm not aware of. Feel free to use and do whatever you want with it though.
  • Scrolling down through the 1200 items is ok, but when trying to scroll up, I can barely reach the top before a minute. The profiler tells me GroupAdapter.getItem(int position) is one of the main root causes for this; it indeed calls GroupUtils.getItem on the main thread (which is ultimately the same problem as above). I tried to flatten GroupAdapter.groups the same way, so we can simply use ArrayList.get instead, but GroupAdapter.groups is altered in a lot of places, and I wasn't able to make it work.
    Edit : This second issue is actually mentioned in #161 and has been around for quite some time.

It doesn't look like Groupie is very friendly with large lists, but the code could use some optimizations (more or less easy to implement).

I don't see a better solution other than to replace Section in your code with List<Item> + groupieAdapter.updateAsync(). 馃

Was this page helpful?
0 / 5 - 0 ratings

Related issues

forntoh picture forntoh  路  3Comments

andhie picture andhie  路  3Comments

ValCanBuild picture ValCanBuild  路  6Comments

alvin-nt picture alvin-nt  路  3Comments

punitda picture punitda  路  3Comments