Instagram / Instagram/IGListKit
RFC: Preprocessing API to do expensive work on background before applying batch updates
- Dominant language
- Objective-C
- Stars
- 13.1k
- Forks
- 1.5k
- PR merge metrics
- No merged PRs in 30d
Description
This is a proposal to add an API that makes it easy to do expensive things (e.g. size text, file i/o) on a background queue from within a section controller before an update is applied.
## Current situation
If you have expensive work to do before using your section controller, currently your only options are:
- Create a **very specific** method/helper to warm up caches or w/e before calling `[adapter performUpdatesAnimated:YES completion:nil]` 😢
- Do it on main 😭
We use both of these solutions and still hit 60fps, but it is _not easy_.
## API Design
- Create a new protocol `IGListPreprocessingDelegate`
- Add a new weak property on `IGListSectionController` similar to the [scroll delegate](https://github.com/Instagram/IGListKit/blob/master/Source/IGListSectionController.h#L105) called `preprocessingDelegate`
- Let's section controllers assign `self`
- `nil` by default
### IGListPreprocessingDelegate
The `IGListPreprocessingDelegate` will have 2 **required** methods:
```objc
// Called on a background queue. This is where you would do work, sync or async.
- (void)preprocessWithContext:(IGListPreprocessingContext *)context;
// Method called on main once all work is done.
- (void)preprocessingDidFinishWithContext:(IGListPreprocessingContext *)context;
```
### IGListPreprocessingContext
This is object is similar to the [UIViewControllerContextTransitioning](https://developer.apple.com/reference/uikit/uiviewcontrollercontexttransitioning) (should we name it _IGListContextPreprocessing_?). It's main duties:
- Give the `preprocessingDelegate` info
- The `object` of the section controller
- Container size
- The section index?
- Have an `id value` object to store arbitrary data
- e.g. calculate the `CGSize` of some text, wrap it in `NSValue`, and do `context.value = wrappedSize;`
- Require calling `[context completePreprocessing]` to mark finished
- Allows the method to call async, block-based APIs
## Considerations
### Main thread affinity
Every public `IGListKit` API has `IGAssertMainQueue()` in it and I **don't** think we should remove any of that so we don't get into threading chaos. That means that when in `-preprocessWithContext:` you can't do something like `[self.collectionContext containerSize]`. We should strive to put all necessary background data on each `IGListPreprocessingContext` object.
### Concurrent work
Ideally, preprocessing should all be done concurrently, though we have to wait (non-blocking) until all of the work is finished before continuing. I think we could:
- Create an internal `IGListPreprocessor` (?) object that inits a private, concurrent queue
- When preprocessing begins, create a new `dispatch_group_t`
- For each section controller w/ a non-`nil` `preprocessingDelegate`:
- Create a new context object w/ data (container size, `dispatch_group_t`, object, etc) injected
- Store context in `NSMapTable` where context=>sectionController (use when finished)
- `dispatch_group_enter`
- Call `[sectionController.preprocessingDelegate preprocessWithContext:context]`
- Context is given `dispatch_group_t` and calls `dispatch_group_leave` inside `-[IGListPreprocessingContext complete]`
- Queue `dispatch_group_notify(serviceGroup,dispatch_get_main_queue(),^{...}`
- Block waits for group to be done, then iterate previous `NSMapTable` calling `[sectionController.preprocessingDelegate preprocessingDidFinishWithContext:context]`
### IGListUpdatingDelegate and IGListAdapter APIs
Adding the concurrent step should coalesce other updates (but **NOT** execute updates!) until the preprocess work _and_ the batch updates are all finished. Getting this working will be a little surgical:
- The `IGListUpdatingDelegate` will need a block to execute before actually batch updating
- Similar to the [objectTransitionBlock](https://github.com/Instagram/IGListKit/blob/master/Source/IGListAdapter.m#L266-L272)
- Precisely when to do this is still a little `¯\_(ツ)_/¯`...
- I _think_ that we have to add it [here](https://github.com/Instagram/IGListKit/blob/master/Source/IGListAdapterUpdater.m#L153), wait for a delegate or block to be executed when preprocessing is done, then continue w/ batch updating
## Thoughts?
Curious to hear comments/questions/concerns on this. Specifically, how do you handle expensive sizing/fetching work with IGListKit (or elsewhere)?
Contributor guide
Assessment
This issue has not been assessed yet.