Adds some configuration to tweak how updates are performed: (#145)

- Coalesce updates: when receiving updates from the datasource while an update is being performed, only the last one will be actually executed.
 - Fast updates: allows next performBatchUpdate before completion of the previous one
 - Tracks visible views: keeps track of cells on-screen instead of relying of indexPathsForVisibleItems and indexPathForCell, which are unreliable on fast updates.
This commit is contained in:
Diego Sánchez 2016-06-01 16:33:29 +01:00
parent d87fe40c90
commit ee45cdee43
8 changed files with 214 additions and 66 deletions

View File

@ -36,6 +36,11 @@ extension BaseChatViewController: ChatDataSourceDelegateProtocol {
public func enqueueModelUpdate(updateType updateType: UpdateType) {
let newItems = self.chatDataSource?.chatItems ?? []
if self.updatesConfig.coalesceUpdates {
self.updateQueue.flushQueue()
}
self.updateQueue.addTask({ [weak self] (completion) -> () in
guard let sSelf = self else { return }
@ -87,79 +92,94 @@ extension BaseChatViewController: ChatDataSourceDelegateProtocol {
func updateVisibleCells(changes: CollectionChanges) {
// Datasource should be already updated!
func updateCellIfVisible(atIndexPath cellIndexPath: NSIndexPath, newDataIndexPath: NSIndexPath) {
if let cell = self.collectionView.cellForItemAtIndexPath(cellIndexPath) {
let presenter = self.presenterForIndexPath(newDataIndexPath)
presenter.configureCell(cell, decorationAttributes: self.decorationAttributesForIndexPath(newDataIndexPath))
presenter.cellWillBeShown(cell) // `createModelUpdates` may have created a new presenter instance for existing visible cell so we need to tell it that its cell is visible
}
// Afer performBatchUpdates, indexPathForCell may return a cell refering to the state before the update
// When doing very fast updates this can lead to giving a presenter the wrong cell below in presenter.configureCell(_:, decorationAttributes:)
// For that reason when self.updatesConfig.fastUpdates is enabled, it's also recomended to enable self.updatesConfig.trackVisibleCells
// See more: https://github.com/diegosanchezr/UICollectionViewStressing
// When self.updatesConfig.fastUpdates is disabled we haven't seen this inconsistency so it's recomended to disable `trackVisibleCells`
let cellsToUpdate: [NSIndexPath: UICollectionViewCell]
if self.updatesConfig.trackVisibleCells {
var updatedVisibleCells = updated(collection: self.visibleCells, withChanges: changes)
updatedVisibleCells.forEach({ (indexPath, cell) in
if cell.hidden || cell.superview == nil {
updatedVisibleCells[indexPath] = nil
}
})
self.visibleCells = updatedVisibleCells
cellsToUpdate = updatedVisibleCells
} else {
var visibleCells: [NSIndexPath: UICollectionViewCell] = [:]
self.collectionView.indexPathsForVisibleItems().forEach({ (indexPath) in
if let cell = self.collectionView.cellForItemAtIndexPath(indexPath) {
visibleCells[indexPath] = cell
}
})
cellsToUpdate = updated(collection: visibleCells, withChanges: changes)
}
let visibleIndexPaths = Set(self.collectionView.indexPathsForVisibleItems().filter { (indexPath) -> Bool in
return !changes.insertedIndexPaths.contains(indexPath) && !changes.deletedIndexPaths.contains(indexPath)
})
var updatedIndexPaths = Set<NSIndexPath>()
for move in changes.movedIndexPaths {
updatedIndexPaths.insert(move.indexPathOld)
updateCellIfVisible(atIndexPath: move.indexPathOld, newDataIndexPath: move.indexPathNew)
}
// Update remaining visible cells
let remaining = visibleIndexPaths.subtract(updatedIndexPaths)
for indexPath in remaining {
updateCellIfVisible(atIndexPath: indexPath, newDataIndexPath: indexPath)
cellsToUpdate.forEach { (indexPath, cell) in
let presenter = self.presenterForIndexPath(indexPath)
presenter.configureCell(cell, decorationAttributes: self.decorationAttributesForIndexPath(indexPath))
presenter.cellWillBeShown(cell) // `createModelUpdates` may have created a new presenter instance for existing visible cell so we need to tell it that its cell is visible
}
}
func performBatchUpdates(
updateModelClosure updateModelClosure: () -> Void,
changes: CollectionChanges,
updateType: UpdateType,
completion: () -> Void) {
let shouldScrollToBottom = updateType != .Pagination && self.isScrolledAtBottom()
let (oldReferenceIndexPath, newReferenceIndexPath) = self.referenceIndexPathsToRestoreScrollPositionOnUpdate(itemsBeforeUpdate: self.chatItemCompanionCollection, changes: changes)
let oldRect = self.rectAtIndexPath(oldReferenceIndexPath)
let myCompletion = {
// Found that cells may not match correct index paths here yet! (see comment below)
// Waiting for next loop seems to fix the issue
dispatch_async(dispatch_get_main_queue(), { () -> Void in
completion()
})
}
func performBatchUpdates(updateModelClosure updateModelClosure: () -> Void,
changes: CollectionChanges,
updateType: UpdateType,
completion: () -> Void) {
let shouldScrollToBottom = updateType != .Pagination && self.isScrolledAtBottom()
let (oldReferenceIndexPath, newReferenceIndexPath) = self.referenceIndexPathsToRestoreScrollPositionOnUpdate(itemsBeforeUpdate: self.chatItemCompanionCollection, changes: changes)
let oldRect = self.rectAtIndexPath(oldReferenceIndexPath)
let fastUpdates = self.updatesConfig.fastUpdates
var myCompletionExecuted = false
let myCompletion = {
if myCompletionExecuted { return }
myCompletionExecuted = true
if updateType == .Normal {
dispatch_async(dispatch_get_main_queue(), { () -> Void in
// Reduces inconsistencies before next update: https://github.com/diegosanchezr/UICollectionViewStressing
completion()
})
}
UIView.animateWithDuration(self.constants.updatesAnimationDuration, animations: { () -> Void in
self.collectionView.performBatchUpdates({ () -> Void in
// We want to update visible cells to support easy removal of bubble tail or any other updates that may be needed after a data update
// Collection view state is not constistent after performBatchUpdates. It can happen that we ask a cell for an index path and we still get the old one.
// Visible cells can be either updated in completion block (easier but with delay) or before, taking into account if some cell is gonna be moved
updateModelClosure()
self.updateVisibleCells(changes)
if updateType == .Normal {
self.collectionView.deleteItemsAtIndexPaths(Array(changes.deletedIndexPaths))
self.collectionView.insertItemsAtIndexPaths(Array(changes.insertedIndexPaths))
for move in changes.movedIndexPaths {
self.collectionView.moveItemAtIndexPath(move.indexPathOld, toIndexPath: move.indexPathNew)
}
}) { (finished) -> Void in
myCompletion()
UIView.animateWithDuration(self.constants.updatesAnimationDuration, animations: { () -> Void in
self.collectionView.performBatchUpdates({ () -> Void in
updateModelClosure()
self.updateVisibleCells(changes) // For instace, to support removal of tails
self.collectionView.deleteItemsAtIndexPaths(Array(changes.deletedIndexPaths))
self.collectionView.insertItemsAtIndexPaths(Array(changes.insertedIndexPaths))
for move in changes.movedIndexPaths {
self.collectionView.moveItemAtIndexPath(move.indexPathOld, toIndexPath: move.indexPathNew)
}
})
} else {
updateModelClosure()
self.collectionView.reloadData()
self.collectionView.collectionViewLayout.prepareLayout()
myCompletion()
}
}) { (finished) -> Void in
myCompletion()
}
})
} else {
self.visibleCells = [:]
updateModelClosure()
self.collectionView.reloadData()
self.collectionView.collectionViewLayout.prepareLayout()
}
if shouldScrollToBottom {
self.scrollToBottom(animated: updateType == .Normal)
} else {
let newRect = self.rectAtIndexPath(newReferenceIndexPath)
self.scrollToPreservePosition(oldRefRect: oldRect, newRefRect: newRect)
}
if shouldScrollToBottom {
self.scrollToBottom(animated: updateType == .Normal)
} else {
let newRect = self.rectAtIndexPath(newReferenceIndexPath)
self.scrollToPreservePosition(oldRefRect: oldRect, newRefRect: newRect)
}
if updateType != .Normal || fastUpdates {
myCompletion()
}
}
private func updateModels(newItems newItems: [ChatItemProtocol], oldItems: ChatItemCompanionCollection, updateType: UpdateType, completion: () -> Void) {

View File

@ -45,6 +45,19 @@ extension BaseChatViewController: ChatCollectionViewLayoutDelegate {
self.presentersByCell.removeObjectForKey(cell)
oldPresenterForCell.cellWasHidden(cell)
}
if self.updatesConfig.trackVisibleCells {
if let visibleCell = self.visibleCells[indexPath] where visibleCell === cell {
self.visibleCells[indexPath] = nil
} else {
self.visibleCells.forEach({ (indexPath, storedCell) in
if cell === storedCell {
// Inconsistency found, likely due to very fast updates
self.visibleCells[indexPath] = nil
}
})
}
}
}
public func collectionView(collectionView: UICollectionView, willDisplayCell cell: UICollectionViewCell, forItemAtIndexPath indexPath: NSIndexPath) {
@ -52,6 +65,9 @@ extension BaseChatViewController: ChatCollectionViewLayoutDelegate {
let presenter = self.presenterForIndexPath(indexPath)
self.presentersByCell.setObject(presenter, forKey: cell)
if self.updatesConfig.trackVisibleCells {
self.visibleCells[indexPath] = cell
}
if self.isAdjustingInputContainer {
UIView.performWithoutAnimation({

View File

@ -39,6 +39,14 @@ public class BaseChatViewController: UIViewController, UICollectionViewDataSourc
public var constants = Constants()
public struct UpdatesConfig {
public var trackVisibleCells = false // When updating very fast, UICollectionView.cellForItemAtIndexPath is not reliable, may return cell from the previous state (*)
public var fastUpdates = false // Allows another performBatchUpdates to be called before completion of a previous one (not recommended). When enabling this it's also recommended to enable `trackVisibleCells` as (*) may happen.
public var coalesceUpdates = false // If receiving data source updates too fast, while an update it's being processed, only the last one will be executed
}
public var updatesConfig = UpdatesConfig()
public private(set) var collectionView: UICollectionView!
public final internal(set) var chatItemCompanionCollection: ChatItemCompanionCollection = ReadOnlyOrderedDictionary(items: [])
private var _chatDataSource: ChatDataSourceProtocol?
@ -236,6 +244,7 @@ public class BaseChatViewController: UIViewController, UICollectionViewDataSourc
var presenterFactory: ChatItemPresenterFactoryProtocol!
let presentersByCell = NSMapTable(keyOptions: .WeakMemory, valueOptions: .WeakMemory)
var updateQueue: SerialTaskQueueProtocol = SerialTaskQueue()
var visibleCells: [NSIndexPath: UICollectionViewCell] = [:] // @see UpdatesConfig.trackVisibleCells
/**
- You can use a decorator to:

View File

@ -96,3 +96,26 @@ func generateChanges(oldCollection oldCollection: [UniqueIdentificable], newColl
return CollectionChanges(insertedIndexPaths: insertedIndexPaths, deletedIndexPaths: deletedIndexPaths, movedIndexPaths: movedIndexPaths)
}
func updated<T: Any>(collection collection: [NSIndexPath: T], withChanges changes: CollectionChanges) -> [NSIndexPath: T] {
var result = collection
changes.deletedIndexPaths.forEach { (indexPath) in
result[indexPath] = nil
}
var movedDestinations = Set<NSIndexPath>()
changes.movedIndexPaths.forEach { (move) in
result[move.indexPathNew] = collection[move.indexPathOld]
movedDestinations.insert(move.indexPathNew)
if !movedDestinations.contains(move.indexPathOld) {
result[move.indexPathOld] = nil
}
}
changes.insertedIndexPaths.forEach { (indexPath) in
result[indexPath] = nil
}
return result
}

View File

@ -30,6 +30,7 @@ protocol SerialTaskQueueProtocol {
func addTask(task: TaskClosure)
func start()
func stop()
func flushQueue()
var isEmpty: Bool { get }
}
@ -52,6 +53,10 @@ final class SerialTaskQueue: SerialTaskQueueProtocol {
self.isStopped = true
}
func flushQueue() {
self.tasksQueue.removeAll()
}
var isEmpty: Bool {
return self.tasksQueue.isEmpty
}

View File

@ -129,9 +129,9 @@ class FakeChatItem: ChatItemProtocol {
final class SerialTaskQueueTestHelper: SerialTaskQueueProtocol {
var onAllTasksFinished: (() -> Void)?
private var isBusy = false
private var isStopped = true
private var tasksQueue = [TaskClosure]()
var isBusy = false
var isStopped = true
var tasksQueue = [TaskClosure]()
func addTask(task: TaskClosure) {
self.tasksQueue.append(task)
@ -151,6 +151,10 @@ final class SerialTaskQueueTestHelper: SerialTaskQueueProtocol {
return self.tasksQueue.isEmpty
}
func flushQueue() {
self.tasksQueue.removeAll()
}
private func maybeExecuteNextTask() {
if !self.isStopped && !self.isBusy {
if !self.isEmpty {

View File

@ -207,6 +207,39 @@ class ChatViewControllerTests: XCTestCase {
XCTAssertEqual(900, controller.view.convertRect(controller.chatInputView.bounds, fromView: controller.chatInputView).maxY)
}
func testThat_GivenCoalescingIsEnabled_WhenMultipleUpdatesAreRequested_ThenUpdatesAreCoalesced() {
let controller = TesteableChatViewController()
controller.updatesConfig.coalesceUpdates = true
self.fakeDidAppearAndLayout(controller: controller)
let fakeDataSource = FakeDataSource()
let updateQueue = SerialTaskQueueTestHelper()
controller.updateQueue = updateQueue
controller.setChatDataSource(fakeDataSource, triggeringUpdateType: .None)
controller.chatDataSourceDidUpdate(fakeDataSource) // running
controller.chatDataSourceDidUpdate(fakeDataSource) // discarded
controller.chatDataSourceDidUpdate(fakeDataSource) // discarded
controller.chatDataSourceDidUpdate(fakeDataSource) // queued
XCTAssertEqual(1, updateQueue.tasksQueue.count)
}
func testThat_GivenCoalescingIsDisabled_WhenMultipleUpdatesAreRequested_ThenUpdatesAreQueued() {
let controller = TesteableChatViewController()
self.fakeDidAppearAndLayout(controller: controller)
let fakeDataSource = FakeDataSource()
let updateQueue = SerialTaskQueueTestHelper()
controller.updateQueue = updateQueue
updateQueue.start()
controller.setChatDataSource(fakeDataSource, triggeringUpdateType: .None)
controller.chatDataSourceDidUpdate(fakeDataSource) // running
controller.chatDataSourceDidUpdate(fakeDataSource) // queued
controller.chatDataSourceDidUpdate(fakeDataSource) // queued
controller.chatDataSourceDidUpdate(fakeDataSource) // queued
XCTAssertEqual(3, updateQueue.tasksQueue.count)
}
// MARK: helpers

View File

@ -83,6 +83,44 @@ class CollectionChangesTests: XCTestCase {
XCTAssertEqual(changes.insertedIndexPaths, [NSIndexPath(forItem: 0, inSection: 0)])
XCTAssertEqual(Set(changes.movedIndexPaths), [Move(0, to: 2), Move(2, to: 1)])
}
func testThatAppliesChangesToCollection() {
// (0, 1, 2, 3, 4) -> (2, 3, new, 4)
let indexPath0 = NSIndexPath(forItem: 0, inSection: 0)
let indexPath1 = NSIndexPath(forItem: 1, inSection: 0)
let indexPath2 = NSIndexPath(forItem: 2, inSection: 0)
let indexPath3 = NSIndexPath(forItem: 3, inSection: 0)
let indexPath4 = NSIndexPath(forItem: 4, inSection: 0)
let collection = [
indexPath0: 0,
indexPath1: 1,
indexPath2: 2,
indexPath3: 3,
indexPath4: 4,
]
let deletions = Set([indexPath0, indexPath1])
let insertions = Set([indexPath2])
let movements = [
CollectionChangeMove(indexPathOld: indexPath2, indexPathNew: indexPath0),
CollectionChangeMove(indexPathOld: indexPath3, indexPathNew: indexPath1),
CollectionChangeMove(indexPathOld: indexPath4, indexPathNew: indexPath3)
]
let changes = CollectionChanges(insertedIndexPaths: insertions, deletedIndexPaths: deletions, movedIndexPaths:movements)
let result = updated(collection: collection, withChanges: changes)
let expected = [
indexPath0: 2,
indexPath1: 3,
// indexPath2: new
indexPath3: 4,
]
XCTAssertEqual(result, expected)
}
}
func Item(uid: String) -> UniqueIdentificable {