composed-swift / ComposedUI

A Swift framework for building User Interfaces with the Composed framework.
Other
12 stars 3 forks source link

Add collection view batch updates #15

Closed JosephDuffy closed 3 years ago

JosephDuffy commented 3 years ago

The idea of this is that without breaking the API the collection view will batch updates.

Inserting a large number of sections is the main area of performance loss we are currently encountering, because the sections are inserted individually and not batched. This change alone has reduce the initial load time of one our screens (which has 100-150 sections added at once) from 30-45 seconds down to less than a second (at least it is not noticeable).

I had created https://github.com/composed-swift/Composed/pull/17 to try and address this, which has the advantage that it would apply to other view types (e.g. UITableView), but I believe does not offer the same performance improvements and it is restricted to a single ComposedSectionProvider.

This is a draft to collect feedback; as you can see there are some TODOs but I think there's enough implemented to provide an overview of the changes that would be required to implement this.

This does not currently work; there are situations that cause the collection NSInternalInconsistencyException', reason: 'Invalid update error. I have some failing tests that demonstrate what the result should be.

JosephDuffy commented 3 years ago

Ok so its obviously a lot more complex and likely this is necessary. I'm never a big fan of index tracking since its very easy to break.

I'm not enjoying it and I'm worried it's fragile, but the (pretty large and growing) test suite makes me feel a little better about it.

Could we improve this with perhaps a type that maintains an index. Then include a few accessors like:

stack.push()
stack.pop()
stack.isEmpty

EDIT: I'm obviously referring to the += and -= balance calls in the mapping.

I thought that tracking some sort of index (I was using the index provided by the mapping) would be useful/required, but the issue is that we don't know the order change could occur so I don't think there's a single index that can be tracked that helps.

At first I was going to create a Change type that contains the data of the change, but always with a var section: Int property to make some of the updates easier, e.g.:

allChanges.map { change in
    guard change.section > removedSection else { return change }

    var change = change
    change.section -= 1
    return change
}

But this isn't actually that useful and could cause more confusion because of differences with how changes are handled (see how sections removes modify other section removes for example)

I have been thinking about some sort of Changeset type that contains this logic though. It would allow for reuse with UITableView and would be a single unit that can be tested, rather than the current kind of hacky testing. The current tests actually apply the changes to a UICollectionView though so they are useful for catching mistakes in the expected results because the collection view will trigger an exception.

JosephDuffy commented 3 years ago

Moving to main repo with merged libraries: https://github.com/composed-swift/Composed/pull/28