IGListKit-integrated-scrolling slightly choppier than stock counterpart

[christianselig]

In the ASDKgram example project, if you scroll in the "ASDK" then the "ListKit" tab, you can perceive a small performance drop in the ListKit version. The difference isn't enormous, but it's enough that it's ever so slightly "jumpy" at times (to be fair it's still great, just not as buttery smooth as the primary tab).

I posted two time profiles, one of each tab (attached) in the Slack, and @appleguy was kind enough to look at them and stated:

so, something is definitely not working optimally - for some reason the collection view sizeForItem: is hitting actual layout calculations rather than cached / layout-ahead

adding,

There have been changes in the Collection stack recently, and I can’t say for sure what it is. The non-IGListKit trace seems to lack this problem (with just a very quick look, maybe not definitive) so it could be that something in listkit is triggering a layout invalidation or moving ahead a call that normally waits

IGListKit is delightful, and the integration alongside Texture is sublime and life changing, but this small nitpick is ever so slightly unfortunate, so I've created an issue as requested. :)

Time Profiles:
listkit-vs-stock-texture-profiles.zip

[Adlai-Holler]

Interesting! This can happen when the size range used during the initial (background) measurement doesn't match the size range used during -layoutSubviews. We should investigate why the two don't match here. Inside [ASCollectionView sizeForElement:] look at the event log for the cell node and see what size range was used to measure it previously, what actual size was returned, and then compare that to the size range & actual size that are generated in that method.

[christianselig]

@Adlai-Holler How exactly do I read the event log for the cell to figure out the previous measurements? The specific cell's event log in sizeForElement reads as follows:

<__NSArrayM 0x17044d800>(
<(Main) t= 14.244: asyncTraitCollectionDidChange: { userInterfaceIdiom = Phone; containerSize = {414, 736}; horizontalSizeClass = Compact; verticalSizeClass = Regular; forceTouchCapability = Available }>,
<(Main) t= 14.430: interfaceStateDidChange: { MeasureLayout | Preload | Display }, old: { MeasureLayout | Preload | Display | Visible }>,
<(Main) t= 14.581: exitHierarchy>,
<(Main) t= 14.583: interfaceStateDidChange: { MeasureLayout | Preload }, old: { MeasureLayout | Preload | Display }>,
<(Main) t= 14.854: interfaceStateDidChange: { MeasureLayout }, old: { MeasureLayout | Preload }>
)

With the call stack for the last log being:

<_NSCallStackArray 0x17425ca70>(
0   Sample                              0x00000001001fd5a8 -[ASDisplayNode setInterfaceState:] + 2032,
1   Sample                              0x00000001001fccf0 __46-[ASDisplayNode recursivelySetInterfaceState:]_block_invoke + 68,
2   Sample                              0x0000000100209938 ASDisplayNodePerformBlockOnEveryNode + 1240,
3   Sample                              0x00000001001fcc74 -[ASDisplayNode recursivelySetInterfaceState:] + 188,
4   Sample                              0x0000000100273c3c -[ASRangeController _updateVisibleNodeIndexPaths] + 5816,
5   Sample                              0x0000000100272050 -[ASRangeController updateIfNeeded] + 96,
6   Sample                              0x000000010019feb8 -[ASCollectionView layoutSubviews] + 780,
7   UIKit                               0x0000000197a2b158 <redacted> + 1200,
8   QuartzCore                          0x0000000194c1b274 <redacted> + 148,
9   Sample                              0x00000001001508c4 -[_ASDisplayLayer layoutSublayers] + 368,
10  QuartzCore                          0x0000000194c0fde8 <redacted> + 292,
11  QuartzCore                          0x0000000194c0fca8 <redacted> + 32,
12  QuartzCore                          0x0000000194b8b360 <redacted> + 252,
13  QuartzCore                          0x0000000194bb23c0 <redacted> + 504,
14  QuartzCore                          0x0000000194b0e554 <redacted> + 600,
15  IOKit                               0x0000000191b6bdd4 IODispatchCalloutFromCFMessage + 372,
16  CoreFoundation                      0x0000000191895958 <redacted> + 180,
17  CoreFoundation                      0x00000001918adadc <redacted> + 56,
18  CoreFoundation                      0x00000001918ad27c <redacted> + 436,
19  CoreFoundation                      0x00000001918aad90 <redacted> + 1752,
20  CoreFoundation                      0x00000001917dad94 CFRunLoopRunSpecific + 424,
21  GraphicsServices                    0x0000000193244074 GSEventRunModal + 100,
22  UIKit                               0x0000000197a93130 UIApplicationMain + 208,
23  Sample                              0x0000000100094414 main + 124,
24  libdyld.dylib                       0x00000001907e959c <redacted> + 4
)

The size range it returns in sizeForElement is:

(ASSizeRange) = (min = (width = 0, height = 0), max = (width = 414, height = 1.7976931348623157E+308))

With the actual size being returned at:

(width = 414, height = 618)

Anything more I can to help shed light on what's occurring?

[christianselig]

@Adlai-Holler Hmm, are you able to offer any more advice? I'm running into this issue in other facets of my app now.

[ajanuar]

Hi, @christianselig, I am experiencing the same problem. How do you solve it?

[christianselig]

@ajanuar I essentially migrated away from IGListKit + Texture built in methods and rewrote the parts I like of IGListKit so they'd just work on top of Texture rather than trying to smush them together after the fact, if that makes sense. I basically rewrote SectionController so that it accepts a nodeBlock, and behind the scenes it's just doing your normal Texture stuff.

[ajanuar]

@christianselig
thank you! I found out in ASSectionController we can overwrite method of sizeRange. If we can provide the exact size of the nodeBlock, scrolling is better.

The drawback is we need to calculate the size.

[christianselig]

@ajanuar Ah yeah, that's one way to do it.

[nguyenhuy]

@christianselig I haven't read much into this discussion, but it seems like you all provided all useful information regarding this problem and how to mitigate it. I'm gonna close this ticket, but please feel free to re-open it if there is still an issue or action item on the framework side. Thank you all for this open and helpful discussion!

[appleguy]

Agreed, we want to make sure this integration is working properly. Where there any issues in the Texture framework integration?

…

On Mon, May 21, 2018 at 4:08 AM Huy Nguyen ***@***.***> wrote: @christianselig <https://github.com/christianselig> I haven't read much into this discussion, but it seems like you all provided all useful information regarding this problem and how to mitigate it. I'm gonna close this ticket, but please feel free to re-open it if there is still an issue or action item on the framework side. Thank you all for this open and helpful discussion! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#63 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAigAwhQPhHBfW9Z-GBPwN6jBJp6EvKjks5t0qAagaJpZM4NFkEJ> .

[christianselig]

@appleguy What do you mean exactly? From the initial Instruments trace it seems that it's recalculating layouts unnecessarily per what you said:

so, something is definitely not working optimally - for some reason the collection view sizeForItem: is hitting actual layout calculations rather than cached / layout-ahead

So I assume that's something to do with the integration between the two frameworks.

[hebertialmeida]

Is there any suggestion on how to improve this? I have been testing IGListKit on my project but I also have noticed the same performance drop. Perhaps @christianselig would you be able to share here how did you solve that?