Summary:
Adding a fix to the `IGListAdapterUpdater` that requests the `UICollectionView` to perform updates on until just-before we update. This way if the `UICollectionView` is changed between update-queue and execution (b/c updates are async), we guarantee the update is performed on the correct view.
See the accompanying unit test that fails w/out the fix enabled.
Differential Revision: D7889908
fbshipit-source-id: 7178677f34951a1e42986b0289fc4abc708d6946
Summary:
Inspired by recent performance investigations in Instagram, adding an experimental feature to defer requesting objects from the `IGListAdapterDataSource` until //just before// diffing is executed. If //n// updates are coalesced into one, this results in just a single request for objects from the data source.
This is a **breaking change** to the public API, but since writing custom `IGListUpdatingDelegate`s is discouraged, I'm less worried about saving this for a major version launch.
Reviewed By: manicakes
Differential Revision: D7744518
fbshipit-source-id: f0001263cddda383e3dedd1d350a83d2cfb8362a
Summary:
Issue fixed: #1141
This is a set of minimum fixes for Xcode 9.3 rather than #1142. Much smaller diffs 😃
The changes here were done by Xcode's FixIt. See also https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/Strings/Articles/formatSpecifiers.html#//apple_ref/doc/uid/TP40004265-SW5.
> Platform Dependencies
>
> OS X uses several data types—NSInteger, NSUInteger,CGFloat, and CFIndex—to provide a consistent means of representing values in 32- and 64-bit environments. In a 32-bit environment, NSInteger and NSUInteger are defined as int and unsigned int, respectively. In 64-bit environments, NSInteger and NSUInteger are defined as long and unsigned long, respectively. To avoid the need to use different printf-style type specifiers depending on the platform, you can use the specifiers shown in Table 3. Note that in some cases you may have to cast the value.
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/1143
Differential Revision: D7551617
Pulled By: rnystrom
fbshipit-source-id: b27ebe2b1a1c93ebfd1218dc38b7621fefee2e6b
Summary: Prefix all private methods with an underscore. A private method is defined as any method not exposed via an interface or protocol. This will improve the readability of the code and hopefully reduce bugs.
Reviewed By: rnystrom
Differential Revision: D7421346
fbshipit-source-id: 536472d57ea7fd8990fe50f3e950907ca57b8e6d
Summary: Found a ~15% perf improvement on just insert/delete by removing copy. Saw traces from retaining the `NSMapTable` in profile.
Reviewed By: manicakes
Differential Revision: D6982523
fbshipit-source-id: 28c1539e06ecf3fe580bcccfecfc4915d4309714
Summary:
In an attempt to best other diffing libraries, I noticed that pure insert/delete diffing results would be almost 5x slower than changes between existing arrays. This is pretty much a benchmarking enhancement, but improves diffing performance by ~5x when the from-array or to-array is empty.
```
// OLD only inserts
avg: 0.007469, min: 0.006998, max: 0.016550, p50: 0.007254, p75: 0.007712, p90: 0.007899, p95: 0.008345, p99: 0.016550
// NEW
avg: 0.001392, min: 0.001256, max: 0.006772, p50: 0.001289, p75: 0.001348, p90: 0.001533, p95: 0.001614, p99: 0.006772
```
```
// OLD only deletes
avg: 0.005821, min: 0.005669, max: 0.006511, p50: 0.005766, p75: 0.005852, p90: 0.006030, p95: 0.006204, p99: 0.006511
// NEW
avg: 0.001184, min: 0.001096, max: 0.001673, p50: 0.001123, p75: 0.001212, p90: 0.001378, p95: 0.001467, p99: 0.001673
```
Note the average time improvements (seconds).
Benchmarking done on a 4s w/ this project:
https://pxl.cl/bLBB
Reviewed By: manicakes
Differential Revision: D6968683
fbshipit-source-id: 0d8e058f0aaa9ce756ca69326527d04504ac6429
Summary: Tiny, measurable perf gain by removing any retains on these params.
Reviewed By: manicakes
Differential Revision: D6968685
fbshipit-source-id: f0ef1ecac6367661d125ea85dc1c9989bf2e9659
Summary: Should also just be a c function instead of a block. Much simpler design.
Reviewed By: manicakes
Differential Revision: D6968682
fbshipit-source-id: f3bac6c5e0f93deec46449eb7abcb42f5e4e6071
Summary: This is much simpler as a static function instead of a block. Also taking an object vs plucking from the array will lend itself to better performance and reusability later.
Reviewed By: manicakes
Differential Revision: D6968684
fbshipit-source-id: 6166473feb20937ce6a1337b4b99a5a240778f56
Summary: Looking at crash logs, the new high-firing crash has 2 item inserts in it. Test deduping the insert. Testing b/c I want to make sure there aren't any weird side effects.
Differential Revision: D6605474
fbshipit-source-id: 522120074aed2ed4995104443d48d8254ddb4fec
Summary:
Followup to make sure that object type mismatches can't happen, even if identifiers collide (which is discouraged). Add assert when duplicates are detected.
Patched some unit tests while I'm in here.
Reviewed By: calimarkus
Differential Revision: D6439094
fbshipit-source-id: d669c01734e5ce9483e851051f548d9960b3087c
Summary:
… source setup too
Issue fixed: #815
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/993
Reviewed By: manicakes
Differential Revision: D6388270
Pulled By: rnystrom
fbshipit-source-id: e5e7e047bad5f21b81b562ebd586f7f5036325ff
Summary:
* Issue: [IGListAdapter visibleSectionControllers] calls [UICollectionViewLayout layoutAttributesForElementsInRect:] which can be expensive. And since visibleSectionControllers is called in scrollViewDidScroll, it can add up and take a toll on scroll performance. On an iPhone 7, it's in 2-10% of the main-thread samples (using Time Profiler).
* Fix: IGListDisplayHandler conveniently keeps track of visible IGListSectionControllers, so let's just use that. Now, [IGListAdapter visibleSectionControllers] drops to ~0.3% of samples.
Reviewed By: rnystrom
Differential Revision: D6351474
fbshipit-source-id: 66914db95e08498927bfbceda4d2e9d58cbc7530
Summary:
… source setup too
Issue fixed: #815
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/993
Reviewed By: jeremycohen
Differential Revision: D6264886
Pulled By: rnystrom
fbshipit-source-id: 5159b7989f1d17fd0a7522d986ed6abf012da77f
Summary:
Issue fixed: #700
- [X] All tests pass. Demo project builds and runs.
- [X] I added tests, an experiment, or detailed why my change isn't tested.
- [X] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [X] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/916
Reviewed By: manicakes
Differential Revision: D5872117
Pulled By: rnystrom
fbshipit-source-id: 420190566c0a822834d8a3b64202f6d807e6a487
Summary:
Adding this API as a standard in preparation for a reloadData fallback test, avoiding massive batch updates stalling the main thread.
Depends on D5392536
Reviewed By: jeremycohen
Differential Revision: D5392713
fbshipit-source-id: 9447bad4b76194ccf26d74c5111624b2fba02efa
Summary:
Experimenting with a new change. We have observed instances of very large (3k+) lists stalling, even on modern devices. This is especially noticeable if the models being diffed are:
- Immutable
- `performUpdates:` often and models are alloc/init'd each time
- The `isEqualToDiffableObject:` is sort of expensive (many `-[NSString isEqualToString:]` across thousands of models or something)
Instead of just rolling this out, I plan on experimenting with results and seeing how much of a performance and stability boost we gain w/ this. Things to measure:
- Scroll performance
- CPU stalls
- WatchDog kills on older devices
Closes https://github.com/Instagram/IGListKit/pull/841
Reviewed By: amonshiz
Differential Revision: D5364127
Pulled By: rnystrom
fbshipit-source-id: 31d50d2e4b3c7c73584d6ec521a9047efd83f315
Summary:
Issue fixed: #773
- [ ] All tests pass. Demo project builds and runs.
- [ ] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [ ] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/776
Differential Revision: D5129343
Pulled By: rnystrom
fbshipit-source-id: 127126545f246e5431636642f203c8150b6a547b
Summary:
This adds `NS_SWIFT_NAME` annotations to all public API's to provide cleaner integration into Swift:
- Removes the need to prefix classes in Swift code, instead rely on Swift module name spacing
- Adds more argument labels to C function API's like `IGListDiff([], [], .equality)` => `ListDiff(oldArray: [], newArray: [], option: .equality)`
While this is a large API change it should be as easy as:
- Find and replace `(IGList)([^K])` to `List$2` in Xcode with a scope set to Swift
- Build and follow compiler's auto fix corrections for C API's or any missed renames
I have not updated the documentation to reflect this yet, I am totally willing to do so but before I sink that amount of time into it I wanted to see if the Instagram team is even open to this change!
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
- [ ] I have updated the documentation
Closes https://github.com/Instagram/IGListKit/pull/593
Reviewed By: jessesquires
Differential Revision: D5028039
Pulled By: rnystrom
fbshipit-source-id: b473d874a1f9574e56b2ebaabd5b73d1b57d4bab
Summary:
Got a little time on the plane to mess around with this idea. Very much want comments on this! Some questions:
- We should probably compile out parts of this using `#if DEBUG`.
- Should I compile out all of the description methods?
- Maybe I wrap the entire debug files w/ `#if DEBUG ... #endif` so none of it is loaded?
- ryanolsonk I used [FLEX](https://github.com/Flipboard/FLEX/blob/master/Classes/Utility/FLEXHeapEnumerator.m) for searching the heap, lmk if you see any problems w/ this
- Its lifted w/ only a few changes for my specific needs (including comments 😂)
- Chalk us up for using [another bit](https://github.com/Instagram/IGListKit/blob/master/Source/Internal/IGListAdapterProxy.m#L77-L82) of your code
- Added a basic test so that the basics of this work and coverage doesn't tank
- Anything else I should test?
- Is there more data we should be dumping?
- I have to track batch update state on the updater in order to dump it. If this seems dan
Closes https://github.com/Instagram/IGListKit/pull/617
Reviewed By: jessesquires
Differential Revision: D4929426
Pulled By: rnystrom
fbshipit-source-id: fcba0d6f0b7766485440f208cf70ad39dfc7a42e
Summary:
a415ef5552 exposed a bug in `UICollectionView` where its state gets corrupted when deleting the same index path more than once in a single batch update block. This resulted crashes like
```
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x4)
Closes https://github.com/Instagram/IGListKit/pull/657
Reviewed By: jessesquires
Differential Revision: D4913790
Pulled By: rnystrom
fbshipit-source-id: 8f6fcdd2e2438da309fc64ca0ac111b9a0980149
Summary:
Issue fixed: #655
- [x] All tests pass. Demo project builds and runs.
- [ ] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
I've also addressed a few cases where there was no empty line between the `param`s and `return`.
Closes https://github.com/Instagram/IGListKit/pull/665
Differential Revision: D4906215
Pulled By: jessesquires
fbshipit-source-id: 5693c85eb548644b0b0e2571eea8ff4d9f706b12
Summary:
Original comment:
If you insert or delete into the same index twice **and** update your data source to reflect those changes, we will squash the insert/delete into a single update because we're using `NSSet` internally.
This becomes very apparent when multiple updates are coalesced.
Issue fixed: #483
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
Closes https://github.com/Instagram/IGListKit/pull/634
Differential Revision: D4860479
Pulled By: jessesquires
fbshipit-source-id: 3aa271d90fca21b11201f62cefa8d7fbcef6930f
Summary:
Issue fixed: #430
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
Closes https://github.com/Instagram/IGListKit/pull/577
Differential Revision: D4768612
Pulled By: rnystrom
fbshipit-source-id: 6a2024101411302446cc2b7843fa175cd43a1562
Summary:
We constantly have random bugs pop up when mutations are made outside of a batch update. This change restricts the mutation API to a batch context object (which is just an `IGListAdapter`) so they can only be done **inside an update block**.
- Fixed open source project
- Confirmed open source examples build
- Updated all of Instagram.app to use this API
- Changelog breaking changes entry
Fixes#392
Reviewed By: jessesquires
Differential Revision: D4754129
fbshipit-source-id: 11d32a0fac3e50c9edbb01e92a8a0c7b8a43cf2d
Summary:
Adding an API to do item-level (cell) moves on the collection view. This complicates things a little bit because of all the issues that moving sections have while in batch updates (e.g. simultaneous animation UICV bugs). Thankfully we use pretty strict types so the compiler does most of the work for us.
Closes#145
- [x] Tests build and pass
- [x] Add `IGListBatchUpdateData` tests to check moves during
- [x] ~~Moving within a reloaded section (no op)~~ can't reload sections
- [x] Moving within a deleted section (no op)
- [x] Moving within a moved section (convert section ops to delete+insert)
- [x] Moving an index path that is also reloaded (convert to delete+insert path)
- [x] Add move unit tests to `IGListAdapterUpdater`
- [x] Add move unit tests to `IGListReloadDataUpdater` (mostly for code coverage...)
- [x] Add move unit tests to `IGListStackedSectionController`
- [x] Add `CHANGELOG.md` entry for 3.0.0
- [x] Test moving without batch
Closes https://github.com/Instagram/IGListKit/pull/418
Reviewed By: jessesquires
Differential Revision: D4521732
Pulled By: rnystrom
fbshipit-source-id: 99a46d1cbb0cc1f857a62ff6ca257aff6e8b7f25
Summary:
Closed#402
- [x] All tests pass. Demo project builds and runs.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
Closes https://github.com/Instagram/IGListKit/pull/453
Differential Revision: D4479611
Pulled By: rnystrom
fbshipit-source-id: 4c93ce30613e45c19478f9f5d0e59eeeb0142766
Summary:
- [x] All tests pass. Demo project builds and runs.
No breaking changes that I am aware of.
Closes https://github.com/Instagram/IGListKit/pull/440
Differential Revision: D4448496
Pulled By: rnystrom
fbshipit-source-id: 9ad9bc3734b605ceab25bf6cdf993568aa6561c1
Summary:
Updated documentation to fix#408
- [X] All tests pass. Demo project builds and runs.
- [X] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [X] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/413
Differential Revision: D4415035
Pulled By: rnystrom
fbshipit-source-id: 2c477c515c6f1346ce6db48d4ba865099cf197cd
Summary:
So the main Podspec file now has two Subspecs, `Diffing` and for lack of better name `Default`. `Diffing` is purely the files within the common directories (used for Diffing, if in the future we support MacOS NSCollectionView then this may need changing). `Default` depends on `Diffing` but also adds all the non-common files.
To use it via CocoaPods, nothing changes. You `import IGListKit` and you will only have access to the files in the pod you use. (So if you use `IGListKit/Diffing` in your Podfile, then you will only get access to the diffing files). If you do a manual installation, or I assume via Carthage, then you will need to import the correct header file (either `IGListDiffKit.h` or `IGListKit.h` depending on what you want).
Turns out CocoaPods creates it's own umbrella header (unless you tell it otherwise) meaning our ones are unused by it.
Just to confirm `IGListKit` in your Podfile will give you everything, only if you add `/Diffing` will you "opt-out" o
Closes https://github.com/Instagram/IGListKit/pull/368
Differential Revision: D4367438
Pulled By: jessesquires
fbshipit-source-id: 272318ca551e7e8c4177ca3ca501fde23bd9705a
Summary:
- Added IGListCompatibility.h file, and imported it in the files where we need the target check.
- Ordered the project by type
In "IGListKit.h" we still have the conditional check for `#if TARGET_OS_EMBEDDED || TARGET_OS_SIMULATOR` - Did we want to make a macro or something for this check?
This is the most Obj-C I've ever done... just felt that was a worthy comment.
Closes#364
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [x] I added an entry to the `CHANGELOG.md` for any breaking changes, enhancements, or bug fixes.
- [x] I have reviewed the [contributing guide](https://github.com/Instagram/IGListKit/blob/master/.github/CONTRIBUTING.md)
Closes https://github.com/Instagram/IGListKit/pull/369
Differential Revision: D4366344
Pulled By: jessesquires
fbshipit-source-id: f0a6168b8965ded22a641814797fbdec92ca1c36
Summary:
- Add missing common files
- Fix podspec for common files
- Pod install all examples
Reviewed By: rnystrom
Differential Revision: D4350871
fbshipit-source-id: 708ebe6ce66caed75af5b2ac18920dcf45567d16
Summary:
Moving files to `Common` dir so changes are more automated with Buck and easier to manage in the long run. Ran `pod install` on test and example targets. Updated buck file to include `Common` so diffing lib still works.
Part of #270
Reviewed By: jessesquires
Differential Revision: D4346722
fbshipit-source-id: 366d89f6dd571b158b5fccd542080c51517ed341
