mirror of
https://github.com/Instagram/IGListKit
synced 2026-05-04 14:09:28 +00:00
0c257795e4
Summary: ## Context Merging multiple batch updates of the same section is inherently unsafe, because each block can mutate the underlaying list and lead to insert/deletes that operate on different before/after lists. The long term fix is to get rid of item level batch updates (insert/delete calls) and use diffing like for sections, but that's a much larger project. Right now, we have a crash spiking because we're reloading the same cell multiple times. Each time `IGListDiff(...)` returns a pair of insert/delete for the same index (lets say 0). Once these updates are merge together, they look like this: ``` Item inserts = 0, 0 Item deletes = 0, 0 ``` We dedupe deletes in `IGListBatchUpdateData`, but don't do anything for inserts, so it looks liket this: ``` Item inserts = 0, 0 Item deletes = 0 ``` So we create an insert/delete unbalance and can crash. Tbh, not sure how this hasn't been a bigger issue in the past. D21822502 tried to remove all duplicate inserts, but that caused other types of crashes, because multiple inserts are allowed. ## This diff * Lets keep the net insert/delete count the same by keeping track of the number of deletes we remove and apply the same to inserts. * Add some tests * Add an experiment flag to kill this if needed to be safe. IG only: Note that I'm making the default true because the crash is happening pretty at start-up. ## Follow up Over time, the batch update clean up logic was slowly added to multiple place (IGListBatchUpdateData, IGListAdapterUpdaterHelpers, IGListIndexPathResult), which is making it hard to follow. A first step would be to move it in 1 or 2 places and avoid redundant logic (like getting unique deletes). However, the real fix is getting rid of batch updates all together. Differential Revision: D61719884 fbshipit-source-id: eda7c264c8239a6a106dbe0256fe777b38fae335
95 lines
2.7 KiB
Objective-C
95 lines
2.7 KiB
Objective-C
/*
|
|
* Copyright (c) Meta Platforms, Inc. and affiliates.
|
|
*
|
|
* This source code is licensed under the MIT license found in the
|
|
* LICENSE file in the root directory of this source tree.
|
|
*/
|
|
|
|
#import <Foundation/Foundation.h>
|
|
|
|
#import "IGListMacros.h"
|
|
#import "IGListMoveIndex.h"
|
|
#import "IGListMoveIndexPath.h"
|
|
|
|
NS_ASSUME_NONNULL_BEGIN
|
|
|
|
/**
|
|
An instance of `IGListBatchUpdateData` takes section indexes and item index paths
|
|
and performs cleanup on init in order to perform a crash-free
|
|
update via `-[UICollectionView performBatchUpdates:completion:]`.
|
|
*/
|
|
IGLK_SUBCLASSING_RESTRICTED
|
|
NS_SWIFT_NAME(ListBatchUpdateData)
|
|
@interface IGListBatchUpdateData : NSObject
|
|
|
|
/**
|
|
Section insert indexes.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSIndexSet *insertSections;
|
|
|
|
/**
|
|
Section delete indexes.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSIndexSet *deleteSections;
|
|
|
|
/**
|
|
Section moves.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSSet<IGListMoveIndex *> *moveSections;
|
|
|
|
/**
|
|
Item insert index paths.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSArray<NSIndexPath *> *insertIndexPaths;
|
|
|
|
/**
|
|
Item delete index paths.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSArray<NSIndexPath *> *deleteIndexPaths;
|
|
|
|
/**
|
|
Item update index paths.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSArray<NSIndexPath *> *updateIndexPaths;
|
|
|
|
/**
|
|
Item moves.
|
|
*/
|
|
@property (nonatomic, strong, readonly) NSArray<IGListMoveIndexPath *> *moveIndexPaths;
|
|
|
|
/**
|
|
Creates a new batch update object with section and item operations.
|
|
|
|
@param insertSections Section indexes to insert.
|
|
@param deleteSections Section indexes to delete.
|
|
@param moveSections Section moves.
|
|
@param insertIndexPaths Item index paths to insert.
|
|
@param deleteIndexPaths Item index paths to delete.
|
|
@param updateIndexPaths Item index paths to update.
|
|
@param moveIndexPaths Item index paths to move.
|
|
@param enableNetItemCountFix Fix item count balance issue when removing duplicate deletes
|
|
|
|
@return A new batch update object.
|
|
*/
|
|
- (instancetype)initWithInsertSections:(NSIndexSet *)insertSections
|
|
deleteSections:(NSIndexSet *)deleteSections
|
|
moveSections:(NSSet<IGListMoveIndex *> *)moveSections
|
|
insertIndexPaths:(NSArray<NSIndexPath *> *)insertIndexPaths
|
|
deleteIndexPaths:(NSArray<NSIndexPath *> *)deleteIndexPaths
|
|
updateIndexPaths:(NSArray<NSIndexPath *> *)updateIndexPaths
|
|
moveIndexPaths:(NSArray<IGListMoveIndexPath *> *)moveIndexPaths
|
|
enableNetItemCountFix:(BOOL)enableNetItemCountFix NS_DESIGNATED_INITIALIZER;
|
|
|
|
/**
|
|
:nodoc:
|
|
*/
|
|
- (instancetype)init NS_UNAVAILABLE;
|
|
|
|
/**
|
|
:nodoc:
|
|
*/
|
|
+ (instancetype)new NS_UNAVAILABLE;
|
|
|
|
@end
|
|
|
|
NS_ASSUME_NONNULL_END
|